Steven Wu <steve...@apple.com> writes: > Hi all > > Currently, -save-temp option doesn’t work well with ObjCARC, sanitizer > and profiling instrumentation. It will drop all ObjCARC optimizations, > runs sanitizer pass very early in the pipeline and profiling > instrumentation will happen twice (which is a no-op second time). They > are because of a combination of two issues: > 1. Language options are not parsed and get dropped when the input is > LLVM IR. Thus no ObjcARC pass and Sanitizer pass are run from bitcode > input. > 2. We use -disable-llvm-optzns to get pristine LLVM IR generated from > front-end but the flag is not as strong as expected. It will run the > instrumentation passes like sanitizer and profiling. > > In this patch, I added a new flag -disable-llvm-passes to disable > every pass from optimization pipeline including instrumentations and > use it in -save-temp. It should be useful for debugging clang CodeGen > as well. I also change the clang front-end to parse ObjcARC and > sanitizer flag in all conditions. I didn’t touch -disable-llvm-optzns > because I am not sure if the flag is supposed to disable > instrumentation passes and if someone is relying on something like: > clang -sanitizer=address -disable-llvm-optzns > Tightening up -disable-llvm-optzns might break them.
This seems pretty reasonable. LGTM with a couple of minor comments. > Thanks > > Steven > > From 57369fb85529c943c394cfe3bf5bc35dcdbd2c59 Mon Sep 17 00:00:00 2001 > From: Steven Wu <steve...@apple.com> > Date: Mon, 13 Jul 2015 11:12:28 -0700 > Subject: [PATCH] Fix -save-temp when using objc-arc, sanitizer and profiling > > Currently, -save-temp will cause ObjCARC optimization to be dropped, > sanitizer pass to run early in the pipeline, and profiling > instrumentation to run twice. > Fix the issue by properly disable all passes in the optimization > pipeline when generating bitcode output and parse some of the Language > Options even when the input is bitcode so the passes can be setup > correctly. > --- > include/clang/Driver/CC1Options.td | 3 +++ > include/clang/Frontend/CodeGenOptions.def | 3 +++ > lib/CodeGen/BackendUtil.cpp | 3 +++ > lib/Driver/Tools.cpp | 11 ++++------ > lib/Frontend/CompilerInvocation.cpp | 6 ++++++ > test/CodeGen/sanitize-address-field-padding.cpp | 1 + > test/CodeGenObjC/arc.ll | 27 > +++++++++++++++++++++++++ > test/Driver/save-temps.c | 8 ++++---- > 8 files changed, 51 insertions(+), 11 deletions(-) > create mode 100644 test/CodeGenObjC/arc.ll > > diff --git a/include/clang/Driver/CC1Options.td > b/include/clang/Driver/CC1Options.td > index 60cc6ec..f7bb6f3 100644 > --- a/include/clang/Driver/CC1Options.td > +++ b/include/clang/Driver/CC1Options.td > @@ -153,6 +153,9 @@ def disable_llvm_optzns : Flag<["-"], > "disable-llvm-optzns">, > HelpText<"Don't run LLVM optimization passes">; > def disable_llvm_verifier : Flag<["-"], "disable-llvm-verifier">, > HelpText<"Don't run the LLVM IR verifier pass">; > +def disable_llvm_passes : Flag<["-"], "disable-llvm-passes">, > + HelpText<"Don't run any LLVM passes including instrumentation " > + "in order to get the IR generated by frontend as it is">; This wording's kind of confusing. I'd say something like "Emit pristine IR from the frontend by not running any LLVM passes at all". Similarly for the CODEGENOPT below. > def disable_red_zone : Flag<["-"], "disable-red-zone">, > HelpText<"Do not emit code that uses the red zone.">; > def dwarf_column_info : Flag<["-"], "dwarf-column-info">, > diff --git a/include/clang/Frontend/CodeGenOptions.def > b/include/clang/Frontend/CodeGenOptions.def > index 803d023..74378b4 100644 > --- a/include/clang/Frontend/CodeGenOptions.def > +++ b/include/clang/Frontend/CodeGenOptions.def > @@ -48,6 +48,9 @@ CODEGENOPT(DisableLLVMOpts , 1, 0) ///< Don't run any > optimizations, for use i > ///< getting .bc files that correspond > to the > ///< internal state before > optimizations are > ///< done. > +CODEGENOPT(DisableLLVMPasses , 1, 0) ///< Don't run any optimizations > + ///< inclduing instrumetation to get the > + ///< IR generated by frontend as it it. > CODEGENOPT(DisableRedZone , 1, 0) ///< Set when -mno-red-zone is enabled. > CODEGENOPT(DisableTailCalls , 1, 0) ///< Do not emit tail calls. > CODEGENOPT(EmitDeclMetadata , 1, 0) ///< Emit special metadata indicating > what > diff --git a/lib/CodeGen/BackendUtil.cpp b/lib/CodeGen/BackendUtil.cpp > index 5ff760c..0c870ee 100644 > --- a/lib/CodeGen/BackendUtil.cpp > +++ b/lib/CodeGen/BackendUtil.cpp > @@ -272,6 +272,9 @@ static void addSymbolRewriterPass(const CodeGenOptions > &Opts, > } > > void EmitAssemblyHelper::CreatePasses() { > + if (CodeGenOpts.DisableLLVMPasses) > + return; > + > unsigned OptLevel = CodeGenOpts.OptimizationLevel; > CodeGenOptions::InliningMethod Inlining = CodeGenOpts.getInlining(); > > diff --git a/lib/Driver/Tools.cpp b/lib/Driver/Tools.cpp > index 0757355..698a842 100644 > --- a/lib/Driver/Tools.cpp > +++ b/lib/Driver/Tools.cpp > @@ -4785,7 +4785,6 @@ void Clang::ConstructJob(Compilation &C, const > JobAction &JA, > // Forward -Xclang arguments to -cc1, and -mllvm arguments to the LLVM > option > // parser. > Args.AddAllArgValues(CmdArgs, options::OPT_Xclang); > - bool OptDisabled = false; > for (const Arg *A : Args.filtered(options::OPT_mllvm)) { > A->claim(); > > @@ -4793,17 +4792,15 @@ void Clang::ConstructJob(Compilation &C, const > JobAction &JA, > // it and developers have been trained to spell it with -mllvm. > if (StringRef(A->getValue(0)) == "-disable-llvm-optzns") { > CmdArgs.push_back("-disable-llvm-optzns"); > - OptDisabled = true; > } else > A->render(Args, CmdArgs); > } > > // With -save-temps, we want to save the unoptimized bitcode output from > the > - // CompileJobAction, so disable optimizations if they are not already > - // disabled. > - if (C.getDriver().isSaveTempsEnabled() && !OptDisabled && > - isa<CompileJobAction>(JA)) > - CmdArgs.push_back("-disable-llvm-optzns"); > + // CompileJobAction, use -disable-llvm-passes to get pristine IR generated > + // by the frontend. > + if (C.getDriver().isSaveTempsEnabled() && isa<CompileJobAction>(JA)) > + CmdArgs.push_back("-disable-llvm-passes"); > > if (Output.getType() == types::TY_Dependencies) { > // Handled with other dependency code. > diff --git a/lib/Frontend/CompilerInvocation.cpp > b/lib/Frontend/CompilerInvocation.cpp > index baee119..fed2717 100644 > --- a/lib/Frontend/CompilerInvocation.cpp > +++ b/lib/Frontend/CompilerInvocation.cpp > @@ -426,6 +426,7 @@ static bool ParseCodeGenArgs(CodeGenOptions &Opts, > ArgList &Args, InputKind IK, > Opts.EmitLLVMUseLists = A->getOption().getID() == OPT_emit_llvm_uselists; > > Opts.DisableLLVMOpts = Args.hasArg(OPT_disable_llvm_optzns); > + Opts.DisableLLVMPasses = Args.hasArg(OPT_disable_llvm_passes); > Opts.DisableRedZone = Args.hasArg(OPT_disable_red_zone); > Opts.ForbidGuardVariables = Args.hasArg(OPT_fforbid_guard_variables); > Opts.UseRegisterSizedBitfieldAccess = Args.hasArg( > @@ -1887,6 +1888,11 @@ bool > CompilerInvocation::CreateFromArgs(CompilerInvocation &Res, > ParseLangArgs(*Res.getLangOpts(), Args, DashX, Diags); > if (Res.getFrontendOpts().ProgramAction == frontend::RewriteObjC) > Res.getLangOpts()->ObjCExceptions = 1; > + } else { > + if (Args.hasArg(OPT_fobjc_arc)) > + Res.getLangOpts()->ObjCAutoRefCount = 1; > + parseSanitizerKinds("-fsanitize=", > Args.getAllArgValues(OPT_fsanitize_EQ), > + Diags, Res.getLangOpts()->Sanitize); This deserves a comment. I'd probably also reverse the order of the if/else conditions to avoid the double negative in the else case. > } > // FIXME: ParsePreprocessorArgs uses the FileManager to read the contents > of > // PCH file and find the original header name. Remove the need to do that > in > diff --git a/test/CodeGen/sanitize-address-field-padding.cpp > b/test/CodeGen/sanitize-address-field-padding.cpp > index d4eea1b..7bd368f 100644 > --- a/test/CodeGen/sanitize-address-field-padding.cpp > +++ b/test/CodeGen/sanitize-address-field-padding.cpp > @@ -5,6 +5,7 @@ > // RUN: %clang_cc1 -triple x86_64-unknown-unknown -fsanitize=address > -fsanitize-address-field-padding=1 -fsanitize-blacklist=%t.type.blacklist > -Rsanitize-address -emit-llvm -o - %s -O1 -mconstructor-aliases 2>&1 | > FileCheck %s --check-prefix=WITH_CTOR_ALIASES > // RUN: %clang_cc1 -triple x86_64-unknown-unknown -fsanitize=address > -fsanitize-address-field-padding=1 -fsanitize-blacklist=%t.file.blacklist > -Rsanitize-address -emit-llvm -o - %s 2>&1 | FileCheck %s > --check-prefix=FILE_BLACKLIST > // RUN: %clang_cc1 -fsanitize=address -emit-llvm -o - %s 2>&1 | FileCheck %s > --check-prefix=NO_PADDING > +// RUN: %clang -save-temps -fsanitize=address -emit-llvm -S -o - %s 2>&1 | > FileCheck %s --check-prefix=NO_PADDING > // > > // The reasons to ignore a particular class are not set in stone and will > change. > diff --git a/test/CodeGenObjC/arc.ll b/test/CodeGenObjC/arc.ll > new file mode 100644 > index 0000000..caafcff > --- /dev/null > +++ b/test/CodeGenObjC/arc.ll > @@ -0,0 +1,27 @@ > +; RUN: %clang_cc1 -Os -emit-llvm -fobjc-arc -o - %s | FileCheck %s > + > +target triple = "x86_64-apple-darwin10" > + > +declare i8* @objc_retain(i8*) > +declare void @objc_release(i8*) > + > +; CHECK-LABEL: define void @test( > +; CHECK-NOT: @objc_ > +; CHECK: } > +define void @test(i8* %x, i1* %p) nounwind { > +entry: > + br label %loop > + > +loop: > + call i8* @objc_retain(i8* %x) > + %q = load i1, i1* %p > + br i1 %q, label %loop.more, label %exit > + > +loop.more: > + call void @objc_release(i8* %x) > + br label %loop > + > +exit: > + call void @objc_release(i8* %x) > + ret void > +} > diff --git a/test/Driver/save-temps.c b/test/Driver/save-temps.c > index 277a901..c974d15 100644 > --- a/test/Driver/save-temps.c > +++ b/test/Driver/save-temps.c > @@ -2,7 +2,7 @@ > // RUN: | FileCheck %s > // CHECK: "-o" "save-temps.i" > // CHECK: "-emit-llvm-uselists" > -// CHECK: "-disable-llvm-optzns" > +// CHECK: "-disable-llvm-passes" > // CHECK: "-o" "save-temps.bc" > // CHECK: "-o" "save-temps.s" > // CHECK: "-o" "save-temps.o" > @@ -14,7 +14,7 @@ > // RUN: | FileCheck %s -check-prefix=CWD > // CWD: "-o" "save-temps.i" > // CWD: "-emit-llvm-uselists" > -// CWD: "-disable-llvm-optzns" > +// CWD: "-disable-llvm-passes" > // CWD: "-o" "save-temps.bc" > // CWD: "-o" "save-temps.s" > // CWD: "-o" "save-temps.o" > @@ -63,7 +63,7 @@ > // RUN: %clang -target x86_64-apple-darwin -save-temps=obj -o obj/dir/a.out > -arch x86_64 %s -### 2>&1 \ > // RUN: | FileCheck %s -check-prefix=CHECK-OBJ > // CHECK-OBJ: "-o" "obj/dir{{/|\\\\}}save-temps.i" > -// CHECK-OBJ: "-disable-llvm-optzns" > +// CHECK-OBJ: "-disable-llvm-passes" > // CHECK-OBJ: "-o" "obj/dir{{/|\\\\}}save-temps.bc" > // CHECK-OBJ: "-o" "obj/dir{{/|\\\\}}save-temps.s" > // CHECK-OBJ: "-o" "obj/dir{{/|\\\\}}save-temps.o" > @@ -72,7 +72,7 @@ > // RUN: %clang -target x86_64-apple-darwin -save-temps=obj -arch x86_64 %s > -### 2>&1 \ > // RUN: | FileCheck %s -check-prefix=CHECK-OBJ-NOO > // CHECK-OBJ-NOO: "-o" "save-temps.i" > -// CHECK-OBJ-NOO: "-disable-llvm-optzns" > +// CHECK-OBJ-NOO: "-disable-llvm-passes" > // CHECK-OBJ-NOO: "-o" "save-temps.bc" > // CHECK-OBJ-NOO: "-o" "save-temps.s" > // CHECK-OBJ-NOO: "-o" "save-temps.o" _______________________________________________ cfe-commits mailing list cfe-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits