From 557c20a886d941ef830437d631b575c0c584586b Mon Sep 17 00:00:00 2001 From: Mehdi Amini Date: Sun, 13 Mar 2016 21:05:23 +0000 Subject: [PATCH] Remove compile time PreserveName in favor of a runtime cc1 -discard-value-names option Summary: This flag is enabled by default in the driver when NDEBUG is set. It is forwarded on the LLVMContext to discard all value names (but GlobalValue) for performance purpose. This an improved version of D18024 Reviewers: echristo, chandlerc Subscribers: cfe-commits Differential Revision: http://reviews.llvm.org/D18127 From: Mehdi Amini llvm-svn: 263394 --- clang/include/clang/Driver/CC1Options.td | 2 ++ clang/include/clang/Frontend/CodeGenOptions.def | 1 + clang/lib/CodeGen/CGBuilder.h | 19 ++++--------------- clang/lib/CodeGen/CGCall.cpp | 2 +- clang/lib/CodeGen/CGExpr.cpp | 2 -- clang/lib/CodeGen/CodeGenFunction.cpp | 20 +++----------------- clang/lib/CodeGen/ModuleBuilder.cpp | 5 +++-- clang/lib/Driver/Tools.cpp | 2 ++ clang/lib/Frontend/CompilerInvocation.cpp | 1 + clang/test/CodeGen/mips-byval-arg.c | 4 ++-- clang/test/CodeGen/mips-vector-arg.c | 4 ++-- clang/test/CodeGen/mips-zero-sized-struct.c | 12 ++++++------ clang/test/CodeGen/mips64-class-return.cpp | 2 +- clang/test/CodeGen/mips64-padding-arg.c | 6 +++--- clang/test/CodeGenCXX/debug-info-class.cpp | 6 +++--- clang/test/CodeGenCXX/discard-name-values.cpp | 10 ++++++++++ clang/test/CodeGenCXX/stack-reuse.cpp | 4 ++-- 17 files changed, 46 insertions(+), 56 deletions(-) create mode 100644 clang/test/CodeGenCXX/discard-name-values.cpp diff --git a/clang/include/clang/Driver/CC1Options.td b/clang/include/clang/Driver/CC1Options.td index a4694f4..082a21f 100644 --- a/clang/include/clang/Driver/CC1Options.td +++ b/clang/include/clang/Driver/CC1Options.td @@ -367,6 +367,8 @@ def code_completion_brief_comments : Flag<["-"], "code-completion-brief-comments HelpText<"Include brief documentation comments in code-completion results.">; def disable_free : Flag<["-"], "disable-free">, HelpText<"Disable freeing of memory on exit">; +def discard_value_names : Flag<["-"], "discard-value-names">, + HelpText<"Discard value names in LLVM IR">; def load : Separate<["-"], "load">, MetaVarName<"">, HelpText<"Load the named plugin (dynamic shared object)">; def plugin : Separate<["-"], "plugin">, MetaVarName<"">, diff --git a/clang/include/clang/Frontend/CodeGenOptions.def b/clang/include/clang/Frontend/CodeGenOptions.def index 9227c12..99ff105 100644 --- a/clang/include/clang/Frontend/CodeGenOptions.def +++ b/clang/include/clang/Frontend/CodeGenOptions.def @@ -43,6 +43,7 @@ CODEGENOPT(DataSections , 1, 0) ///< Set when -fdata-sections is enabled. CODEGENOPT(UniqueSectionNames, 1, 1) ///< Set for -funique-section-names. CODEGENOPT(DisableFPElim , 1, 0) ///< Set when -fomit-frame-pointer is enabled. CODEGENOPT(DisableFree , 1, 0) ///< Don't free memory. +CODEGENOPT(DiscardValueNames , 1, 0) ///< Discard Value Names from the IR (LLVMContext flag) CODEGENOPT(DisableGCov , 1, 0) ///< Don't run the GCov pass, for testing. CODEGENOPT(DisableLLVMOpts , 1, 0) ///< Don't run any optimizations, for use in ///< getting .bc files that correspond to the diff --git a/clang/lib/CodeGen/CGBuilder.h b/clang/lib/CodeGen/CGBuilder.h index baba30d..027435d 100644 --- a/clang/lib/CodeGen/CGBuilder.h +++ b/clang/lib/CodeGen/CGBuilder.h @@ -23,9 +23,7 @@ class CodeGenFunction; /// \brief This is an IRBuilder insertion helper that forwards to /// CodeGenFunction::InsertHelper, which adds necessary metadata to /// instructions. -template -class CGBuilderInserter - : protected llvm::IRBuilderDefaultInserter { +class CGBuilderInserter : protected llvm::IRBuilderDefaultInserter { public: CGBuilderInserter() = default; explicit CGBuilderInserter(CodeGenFunction *CGF) : CGF(CGF) {} @@ -39,17 +37,10 @@ private: CodeGenFunction *CGF = nullptr; }; -// Don't preserve names on values in an optimized build. -#ifdef NDEBUG -#define PreserveNames false -#else -#define PreserveNames true -#endif - -typedef CGBuilderInserter CGBuilderInserterTy; +typedef CGBuilderInserter CGBuilderInserterTy; -typedef llvm::IRBuilder CGBuilderBaseTy; +typedef llvm::IRBuilder + CGBuilderBaseTy; class CGBuilderTy : public CGBuilderBaseTy { /// Storing a reference to the type cache here makes it a lot easier @@ -305,8 +296,6 @@ public: } }; -#undef PreserveNames - } // end namespace CodeGen } // end namespace clang diff --git a/clang/lib/CodeGen/CGCall.cpp b/clang/lib/CodeGen/CGCall.cpp index 19f8c7c..7fb301c 100644 --- a/clang/lib/CodeGen/CGCall.cpp +++ b/clang/lib/CodeGen/CGCall.cpp @@ -3840,7 +3840,7 @@ RValue CodeGenFunction::EmitCall(const CGFunctionInfo &CallInfo, } llvm::Instruction *CI = CS.getInstruction(); - if (Builder.isNamePreserving() && !CI->getType()->isVoidTy()) + if (!CI->getType()->isVoidTy()) CI->setName("call"); // Emit any writebacks immediately. Arguably this should happen diff --git a/clang/lib/CodeGen/CGExpr.cpp b/clang/lib/CodeGen/CGExpr.cpp index 4a0f149..ccda7f4 100644 --- a/clang/lib/CodeGen/CGExpr.cpp +++ b/clang/lib/CodeGen/CGExpr.cpp @@ -66,8 +66,6 @@ Address CodeGenFunction::CreateTempAlloca(llvm::Type *Ty, CharUnits Align, /// block. llvm::AllocaInst *CodeGenFunction::CreateTempAlloca(llvm::Type *Ty, const Twine &Name) { - if (!Builder.isNamePreserving()) - return new llvm::AllocaInst(Ty, nullptr, "", AllocaInsertPt); return new llvm::AllocaInst(Ty, nullptr, Name, AllocaInsertPt); } diff --git a/clang/lib/CodeGen/CodeGenFunction.cpp b/clang/lib/CodeGen/CodeGenFunction.cpp index 0e118b8..ac03223 100644 --- a/clang/lib/CodeGen/CodeGenFunction.cpp +++ b/clang/lib/CodeGen/CodeGenFunction.cpp @@ -747,9 +747,7 @@ void CodeGenFunction::StartFunction(GlobalDecl GD, // later. Don't create this with the builder, because we don't want it // folded. llvm::Value *Undef = llvm::UndefValue::get(Int32Ty); - AllocaInsertPt = new llvm::BitCastInst(Undef, Int32Ty, "", EntryBB); - if (Builder.isNamePreserving()) - AllocaInsertPt->setName("allocapt"); + AllocaInsertPt = new llvm::BitCastInst(Undef, Int32Ty, "allocapt", EntryBB); ReturnBlock = getJumpDestInCurrentScope("return"); @@ -1862,26 +1860,14 @@ void CodeGenFunction::InsertHelper(llvm::Instruction *I, CGM.getSanitizerMetadata()->disableSanitizerForInstruction(I); } -template -void CGBuilderInserter::InsertHelper( +void CGBuilderInserter::InsertHelper( llvm::Instruction *I, const llvm::Twine &Name, llvm::BasicBlock *BB, llvm::BasicBlock::iterator InsertPt) const { - llvm::IRBuilderDefaultInserter::InsertHelper(I, Name, BB, - InsertPt); + llvm::IRBuilderDefaultInserter::InsertHelper(I, Name, BB, InsertPt); if (CGF) CGF->InsertHelper(I, Name, BB, InsertPt); } -#ifdef NDEBUG -#define PreserveNames false -#else -#define PreserveNames true -#endif -template void CGBuilderInserter::InsertHelper( - llvm::Instruction *I, const llvm::Twine &Name, llvm::BasicBlock *BB, - llvm::BasicBlock::iterator InsertPt) const; -#undef PreserveNames - static bool hasRequiredFeatures(const SmallVectorImpl &ReqFeatures, CodeGenModule &CGM, const FunctionDecl *FD, std::string &FirstMissing) { diff --git a/clang/lib/CodeGen/ModuleBuilder.cpp b/clang/lib/CodeGen/ModuleBuilder.cpp index a1211aa..bd59332 100644 --- a/clang/lib/CodeGen/ModuleBuilder.cpp +++ b/clang/lib/CodeGen/ModuleBuilder.cpp @@ -64,8 +64,9 @@ namespace { CoverageSourceInfo *CoverageInfo = nullptr) : Diags(diags), Ctx(nullptr), HeaderSearchOpts(HSO), PreprocessorOpts(PPO), CodeGenOpts(CGO), HandlingTopLevelDecls(0), - CoverageInfo(CoverageInfo), - M(new llvm::Module(ModuleName, C)) {} + CoverageInfo(CoverageInfo), M(new llvm::Module(ModuleName, C)) { + C.setDiscardValueNames(CGO.DiscardValueNames); + } ~CodeGeneratorImpl() override { // There should normally not be any leftover inline method definitions. diff --git a/clang/lib/Driver/Tools.cpp b/clang/lib/Driver/Tools.cpp index 72e7caa..f51e1f6 100644 --- a/clang/lib/Driver/Tools.cpp +++ b/clang/lib/Driver/Tools.cpp @@ -3712,6 +3712,8 @@ void Clang::ConstructJob(Compilation &C, const JobAction &JA, // Disable the verification pass in -asserts builds. #ifdef NDEBUG CmdArgs.push_back("-disable-llvm-verifier"); + // Discard LLVM value names in -asserts builds. + CmdArgs.push_back("-discard-value-names"); #endif // Set the main file name, so that debug info works even with diff --git a/clang/lib/Frontend/CompilerInvocation.cpp b/clang/lib/Frontend/CompilerInvocation.cpp index bf845b2..dee425e 100644 --- a/clang/lib/Frontend/CompilerInvocation.cpp +++ b/clang/lib/Frontend/CompilerInvocation.cpp @@ -541,6 +541,7 @@ static bool ParseCodeGenArgs(CodeGenOptions &Opts, ArgList &Args, InputKind IK, Opts.DisableFPElim = (Args.hasArg(OPT_mdisable_fp_elim) || Args.hasArg(OPT_pg)); Opts.DisableFree = Args.hasArg(OPT_disable_free); + Opts.DiscardValueNames = Args.hasArg(OPT_discard_value_names); Opts.DisableTailCalls = Args.hasArg(OPT_mdisable_tail_calls); Opts.FloatABI = Args.getLastArgValue(OPT_mfloat_abi); if (Arg *A = Args.getLastArg(OPT_meabi)) { diff --git a/clang/test/CodeGen/mips-byval-arg.c b/clang/test/CodeGen/mips-byval-arg.c index 0e3d334..1e7f389 100644 --- a/clang/test/CodeGen/mips-byval-arg.c +++ b/clang/test/CodeGen/mips-byval-arg.c @@ -1,5 +1,5 @@ -// RUN: %clang -target mipsel-unknown-linux -O3 -S -o - -emit-llvm %s | FileCheck %s -check-prefix=O32 -// RUN: %clang -target mips64el-unknown-linux -O3 -S -mabi=n64 -o - -emit-llvm %s | FileCheck %s -check-prefix=N64 +// RUN: %clang_cc1 -triple mipsel-unknown-linux -O3 -S -o - -emit-llvm %s | FileCheck %s -check-prefix=O32 +// RUN: %clang_cc1 -triple mips64el-unknown-linux -O3 -S -target-abi n64 -o - -emit-llvm %s | FileCheck %s -check-prefix=N64 typedef struct { float f[3]; diff --git a/clang/test/CodeGen/mips-vector-arg.c b/clang/test/CodeGen/mips-vector-arg.c index f8c89df..5e55285 100644 --- a/clang/test/CodeGen/mips-vector-arg.c +++ b/clang/test/CodeGen/mips-vector-arg.c @@ -1,5 +1,5 @@ -// RUN: %clang -target mipsel-unknown-linux -O3 -S -o - -emit-llvm %s | FileCheck %s -check-prefix=O32 -// RUN: %clang -target mips64el-unknown-linux -O3 -S -mabi=n64 -o - -emit-llvm %s | FileCheck %s -check-prefix=N64 +// RUN: %clang_cc1 -triple mipsel-unknown-linux -O3 -S -o - -emit-llvm %s | FileCheck %s -check-prefix=O32 +// RUN: %clang_cc1 -triple mips64el-unknown-linux -O3 -S -target-abi n64 -o - -emit-llvm %s | FileCheck %s -check-prefix=N64 // check that // 1. vector arguments are passed in integer registers diff --git a/clang/test/CodeGen/mips-zero-sized-struct.c b/clang/test/CodeGen/mips-zero-sized-struct.c index afff3b4..217bdb9 100644 --- a/clang/test/CodeGen/mips-zero-sized-struct.c +++ b/clang/test/CodeGen/mips-zero-sized-struct.c @@ -1,9 +1,9 @@ -// RUN: %clang -target mips-unknown-linux-gnu -S -emit-llvm -o - %s | FileCheck -check-prefix=O32 %s -// RUN: %clang -target mipsel-unknown-linux-gnu -S -emit-llvm -o - %s | FileCheck -check-prefix=O32 %s -// RUN: %clang -target mips64-unknown-linux-gnu -S -emit-llvm -o - %s -mabi=n32 | FileCheck -check-prefix=N32 %s -// RUN: %clang -target mips64el-unknown-linux-gnu -S -emit-llvm -o - %s -mabi=n32 | FileCheck -check-prefix=N32 %s -// RUN: %clang -target mips64-unknown-linux-gnu -S -emit-llvm -o - %s | FileCheck -check-prefix=N64 %s -// RUN: %clang -target mips64el-unknown-linux-gnu -S -emit-llvm -o - %s | FileCheck -check-prefix=N64 %s +// RUN: %clang_cc1 -triple mips-unknown-linux-gnu -S -emit-llvm -o - %s | FileCheck -check-prefix=O32 %s +// RUN: %clang_cc1 -triple mipsel-unknown-linux-gnu -S -emit-llvm -o - %s | FileCheck -check-prefix=O32 %s +// RUN: %clang_cc1 -triple mips64-unknown-linux-gnu -S -emit-llvm -o - %s -target-abi n32 | FileCheck -check-prefix=N32 %s +// RUN: %clang_cc1 -triple mips64el-unknown-linux-gnu -S -emit-llvm -o - %s -target-abi n32 | FileCheck -check-prefix=N32 %s +// RUN: %clang_cc1 -triple mips64-unknown-linux-gnu -S -emit-llvm -o - %s | FileCheck -check-prefix=N64 %s +// RUN: %clang_cc1 -triple mips64el-unknown-linux-gnu -S -emit-llvm -o - %s | FileCheck -check-prefix=N64 %s // O32: define void @fn28(%struct.T2* noalias sret %agg.result, i8 signext %arg0) // N32: define void @fn28(i8 signext %arg0) diff --git a/clang/test/CodeGen/mips64-class-return.cpp b/clang/test/CodeGen/mips64-class-return.cpp index 57fa8ef..af2dd5c 100644 --- a/clang/test/CodeGen/mips64-class-return.cpp +++ b/clang/test/CodeGen/mips64-class-return.cpp @@ -1,4 +1,4 @@ -// RUN: %clang -target mips64el-unknown-linux -O3 -S -mabi=n64 -o - -emit-llvm %s | FileCheck %s +// RUN: %clang_cc1 -triple mips64el-unknown-linux -O3 -S -target-abi n64 -o - -emit-llvm %s | FileCheck %s class B0 { double d; diff --git a/clang/test/CodeGen/mips64-padding-arg.c b/clang/test/CodeGen/mips64-padding-arg.c index b92098f..7910734 100644 --- a/clang/test/CodeGen/mips64-padding-arg.c +++ b/clang/test/CodeGen/mips64-padding-arg.c @@ -1,6 +1,6 @@ -// RUN: %clang -target mipsel-unknown-linux -O3 -S -o - -emit-llvm %s | FileCheck %s -check-prefix=O32 -// RUN: %clang -target mips64el-unknown-linux -O3 -S -mabi=n64 -o - -emit-llvm %s | FileCheck %s -check-prefix=N64 -// RUN: %clang -target mipsel-unknown-linux -mfp64 -O3 -S -o - -emit-llvm %s | FileCheck %s -check-prefix=O32 +// RUN: %clang_cc1 -triple mipsel-unknown-linux -O3 -S -o - -emit-llvm %s | FileCheck %s -check-prefix=O32 +// RUN: %clang_cc1 -triple mips64el-unknown-linux -O3 -S -target-abi n64 -o - -emit-llvm %s | FileCheck %s -check-prefix=N64 +// RUN: %clang_cc1 -triple mipsel-unknown-linux -target-feature "+fp64" -O3 -S -o - -emit-llvm %s | FileCheck %s -check-prefix=O32 typedef struct { double d; diff --git a/clang/test/CodeGenCXX/debug-info-class.cpp b/clang/test/CodeGenCXX/debug-info-class.cpp index a63efe5..ed52fd0 100644 --- a/clang/test/CodeGenCXX/debug-info-class.cpp +++ b/clang/test/CodeGenCXX/debug-info-class.cpp @@ -83,9 +83,9 @@ int main(int argc, char **argv) { return 0; } -// RUN: %clang -target x86_64-unknown_unknown -emit-llvm -g -S %s -o - | FileCheck %s -// RUN: %clang -target i686-cygwin -emit-llvm -g -S %s -o - | FileCheck %s -// RUN: %clang -target armv7l-unknown-linux-gnueabihf -emit-llvm -g -S %s -o - | FileCheck %s +// RUN: %clang_cc1 -triple x86_64-unknown_unknown -emit-llvm -debug-info-kind=limited -fexceptions %s -o - | FileCheck %s +// RUN: %clang_cc1 -triple i686-cygwin -emit-llvm -debug-info-kind=limited -fexceptions %s -o - | FileCheck %s +// RUN: %clang_cc1 -triple armv7l-unknown-linux-gnueabihf -emit-llvm -debug-info-kind=limited -fexceptions %s -o - | FileCheck %s // CHECK: invoke {{.+}} @_ZN1BD1Ev(%class.B* %b) // CHECK-NEXT: unwind label %{{.+}}, !dbg ![[EXCEPTLOC:.*]] diff --git a/clang/test/CodeGenCXX/discard-name-values.cpp b/clang/test/CodeGenCXX/discard-name-values.cpp new file mode 100644 index 0000000..49cb7d2 --- /dev/null +++ b/clang/test/CodeGenCXX/discard-name-values.cpp @@ -0,0 +1,10 @@ +// RUN: %clang_cc1 -emit-llvm -triple=armv7-apple-darwin -emit-llvm -std=c++11 %s -o - -O1 | FileCheck %s +// RUN: %clang_cc1 -emit-llvm -triple=armv7-apple-darwin -emit-llvm -std=c++11 %s -o - -O1 -discard-value-names | FileCheck %s --check-prefix=DISCARDVALUE + +int foo(int bar) { + return bar; +} + +// CHECK: ret i32 %bar +// DISCARDVALUE: ret i32 %0 + diff --git a/clang/test/CodeGenCXX/stack-reuse.cpp b/clang/test/CodeGenCXX/stack-reuse.cpp index 473a57c..d6340ef 100644 --- a/clang/test/CodeGenCXX/stack-reuse.cpp +++ b/clang/test/CodeGenCXX/stack-reuse.cpp @@ -1,4 +1,4 @@ -// RUN: %clang -target armv7l-unknown-linux-gnueabihf -S %s -o - -emit-llvm -O1 -disable-llvm-optzns | FileCheck %s +// RUN: %clang_cc1 -triple armv7-unknown-linux-gnueabihf %s -o - -emit-llvm -O1 | FileCheck %s // Stack should be reused when possible, no need to allocate two separate slots // if they have disjoint lifetime. @@ -21,7 +21,7 @@ struct Combiner { S_large a, b; Combiner(S_large); - Combiner f(); + Combiner f(); }; extern S_small foo_small(); -- 2.7.4