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 ++ .../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 a4694f46a29fc..082a21f6a4f73 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 9227c1282d1ca..99ff105ea89f9 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 baba30d5bf698..027435d7c5996 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 @@ class CGBuilderInserter 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 @@ class CGBuilderTy : public CGBuilderBaseTy { } }; -#undef PreserveNames - } // end namespace CodeGen } // end namespace clang diff --git a/clang/lib/CodeGen/CGCall.cpp b/clang/lib/CodeGen/CGCall.cpp index 19f8c7cfd247b..7fb301c8c3680 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 4a0f149b96971..ccda7f4d1aafe 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 0e118b8f6f70c..ac0322303a2a8 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 a1211aa71ae6c..bd59332a274a0 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 72e7caa76a401..f51e1f6778838 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 bf845b299f2a8..dee425eb88b99 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 0e3d334b27453..1e7f38915ccb9 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 f8c89dfff5483..5e55285f3f1c3 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 afff3b41d833c..217bdb92c6f68 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 57fa8ef5109b7..af2dd5cbec2fb 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 b92098f45a4e6..7910734d6f16e 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 a63efe5d780a3..ed52fd05238e9 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 0000000000000..49cb7d2fc0587 --- /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 473a57cda27d2..d6340ef2c10b6 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();