> On 2015-Apr-30, at 13:04, Steven Wu <[email protected]> wrote: > > ping. > > Can I pull the trigger to always run the pass? Here is a patch with some > tweaks from the previous one because we don’t actually want to run the pass > with -O0.
Yup, LGTM. Sorry for the delay. > > diff --git a/lib/CodeGen/BackendUtil.cpp b/lib/CodeGen/BackendUtil.cpp > index 7bc351a..177deb1 100644 > --- a/lib/CodeGen/BackendUtil.cpp > +++ b/lib/CodeGen/BackendUtil.cpp > @@ -570,8 +570,7 @@ bool EmitAssemblyHelper::AddEmitPasses(BackendAction > Action, > // Add ObjC ARC final-cleanup optimizations. This is done as part of the > // "codegen" passes so that it isn't run multiple times when there is > // inlining happening. > - if (LangOpts.ObjCAutoRefCount && > - CodeGenOpts.OptimizationLevel > 0) > + if (CodeGenOpts.OptimizationLevel > 0) > PM->add(createObjCARCContractPass()); > > if (TM->addPassesToEmitFile(*PM, OS, CGFT, > > Thanks > > Steven > >> On Mar 16, 2015, at 2:19 PM, Steven Wu <[email protected]> wrote: >> >> No patch for the previous email. I really wanted to move the ObjCConstract >> pass to libLLVMCodeGen but the pass is very tightly coupled with other parts >> of ObjC-ARC. I will leave that as future work. >> I will just fix clang to run the pass before they can be decoupled. Here is >> the patch: >> >> From fca14e93b929b6aa5e22ad64040ad8b9e3965364 Mon Sep 17 00:00:00 2001 >> From: Steven Wu <[email protected]> >> Date: Mon, 16 Mar 2015 14:08:42 -0700 >> Subject: [PATCH] Always run ObjCARCContract in clang to match LTO >> >> ObjCARCContract is mandatory for ObjC code with ARC. It only runs when >> -fobj-arc flag is specified. Since -fobj-arc is a frontend flag, there are >> no way to run the ObjCARCContract pass from bitcode input in clang. >> Since ObjCARCContract checks the presence of ARC function in the module >> level, it has little overhead for code not using ObjCARC (not observable >> under -O0), it is now enable all the time to match the behavior in LTO. >> --- >> lib/CodeGen/BackendUtil.cpp | 4 +--- >> 1 file changed, 1 insertion(+), 3 deletions(-) >> >> diff --git a/lib/CodeGen/BackendUtil.cpp b/lib/CodeGen/BackendUtil.cpp >> index d3d5bcf..09f1e2a 100644 >> --- a/lib/CodeGen/BackendUtil.cpp >> +++ b/lib/CodeGen/BackendUtil.cpp >> @@ -566,9 +566,7 @@ bool EmitAssemblyHelper::AddEmitPasses(BackendAction >> Action, >> // Add ObjC ARC final-cleanup optimizations. This is done as part of the >> // "codegen" passes so that it isn't run multiple times when there is >> // inlining happening. >> - if (LangOpts.ObjCAutoRefCount && >> - CodeGenOpts.OptimizationLevel > 0) >> - PM->add(createObjCARCContractPass()); >> + PM->add(createObjCARCContractPass()); >> >> if (TM->addPassesToEmitFile(*PM, OS, CGFT, >> /*DisableVerify=*/!CodeGenOpts.VerifyModule)) { >> -- >> 2.3.0 (Apple Git-54) >> >> Steven >> >> >>> On Mar 16, 2015, at 11:12 AM, Duncan P. N. Exon Smith >>> <[email protected]> wrote: >>> >>> >>>> On 2015 Mar 16, at 10:34, Steven Wu <[email protected]> wrote: >>>> >>>> After a detailed looked at ObjCContract pass, it already has a module >>>> level check to decide whether the pass should be run or not. It checks the >>>> presence of ARC function at module level. The overhead of this check is >>>> very low and it doesn’t hurt to run it all the time. So there are three >>>> options now: >>>> 1. Function attribute: the advantage is that we can run ObjCContract pass >>>> only on function instead of the entire module. The disadvantage is that it >>>> introduces complication in the bicode and we have to run upgrade pass for >>>> old bitcode by checking all the presence of ARC function. >>>> 2. Module flag: it is really not helpful to create a module flag after I >>>> find there is almost no overhead to run ObjCARCContract pass on a module >>>> without Objc-ARC. It is also tricky to get the consistence behavior after >>>> the upgrade of bitcode by only checking the module flag. >>>> 3. Always run the pass to match the behavior in LTO: this seems the >>>> simplest solution. I also run full LNT test-suite to prove that there are >>>> no overhead to run the pass on the code without ObjC-ARC. I force >>>> ObjCARCContract pass to run at -O0 on the code and there are no observable >>>> overhead even at -O0. Results attached. >>>> >>>> Steven >>>> <no-objc-arc-O0.json><objc-arc-O0.json> >>> >>> (Did you mean to attach a patch to this email?) >>> >>> #3 makes sense to me given how cheap the checks are. I hadn't >>> realized they were on the module. >>> >>>> >>>> >>>>> On Mar 3, 2015, at 1:20 PM, Duncan P. N. Exon Smith >>>>> <[email protected]> wrote: >>>>> >>>>> >>>>>> On 2015-Mar-02, at 14:56, Steven Wu <[email protected]> wrote: >>>>>> >>>>>> Fix the code according to Duncan's feedback. Attempt to add a testcase. >>>>>> Not sure if there is better to check if the pass is run other than using >>>>>> -debug-pass=Arguments. >>>>> >>>>> Steven and I talked offline. All the passes that depend on this flag >>>>> are function passes. Since it's possible to switch on a per-function >>>>> basis, this should be a function attribute (not a module flag). >>>>> >>>>>> http://reviews.llvm.org/D7799 >>>>>> >>>>>> Files: >>>>>> lib/CodeGen/BackendUtil.cpp >>>>>> lib/CodeGen/CGObjCMac.cpp >>>>>> test/CodeGen/arc-passes.m >>>>>> >>>>>> Index: lib/CodeGen/BackendUtil.cpp >>>>>> =================================================================== >>>>>> --- lib/CodeGen/BackendUtil.cpp >>>>>> +++ lib/CodeGen/BackendUtil.cpp >>>>>> @@ -569,7 +569,7 @@ >>>>>> // Add ObjC ARC final-cleanup optimizations. This is done as part of the >>>>>> // "codegen" passes so that it isn't run multiple times when there is >>>>>> // inlining happening. >>>>>> - if (LangOpts.ObjCAutoRefCount && >>>>>> + if (TheModule->getModuleFlag("Objective-C ARC") && >>>>>> CodeGenOpts.OptimizationLevel > 0) >>>>>> PM->add(createObjCARCContractPass()); >>>>>> >>>>>> Index: lib/CodeGen/CGObjCMac.cpp >>>>>> =================================================================== >>>>>> --- lib/CodeGen/CGObjCMac.cpp >>>>>> +++ lib/CodeGen/CGObjCMac.cpp >>>>>> @@ -4284,6 +4284,12 @@ >>>>>> } >>>>>> } >>>>>> >>>>>> + // Add ObjCAutoRefCount as a module flag >>>>>> + if (CGM.getLangOpts().ObjCAutoRefCount) { >>>>>> + Mod.addModuleFlag(llvm::Module::Error, >>>>>> + "Objective-C ARC", 1u); >>>>>> + } >>>>>> + >>>>>> // Indicate whether we're compiling this to run on a simulator. >>>>>> const llvm::Triple &Triple = CGM.getTarget().getTriple(); >>>>>> if (Triple.isiOS() && >>>>>> Index: test/CodeGen/arc-passes.m >>>>>> =================================================================== >>>>>> --- /dev/null >>>>>> +++ test/CodeGen/arc-passes.m >>>>>> @@ -0,0 +1,12 @@ >>>>>> +// RUN: %clang_cc1 -triple x86_64-apple-darwin10 -fobjc-arc -emit-llvm >>>>>> %s -o - | FileCheck %s >>>>>> +// RUN: %clang_cc1 -triple x86_64-apple-darwin10 -emit-llvm %s -o - | >>>>>> FileCheck %s -check-prefix=NO-ARC >>>>>> +// RUN: %clang_cc1 -triple x86_64-apple-darwin10 -fobjc-arc -emit-obj >>>>>> -O3 %s -o /dev/null -mllvm -debug-pass=Arguments 2>&1 | FileCheck %s >>>>>> -check-prefix=ARC-PASS >>>>>> +// RUN: %clang_cc1 -triple x86_64-apple-darwin10 -emit-obj -O3 %s -o >>>>>> /dev/null -mllvm -debug-pass=Arguments 2>&1 | FileCheck %s >>>>>> -check-prefix=NO-ARC-PASS >>>>>> + >>>>>> +// CHECK: !{i32 1, !"Objective-C ARC", i32 1} >>>>>> +// NO-ARC-NOT: !{i32 1, !"Objective-C ARC", i32 1} >>>>>> +// ARC-PASS: -objc-arc-contract >>>>>> +// NO-ARC-PASS-NOT: -objc-arc-contract >>>>>> + >>>>>> +void test() { >>>>>> +} >>>>>> >>>>>> EMAIL PREFERENCES >>>>>> http://reviews.llvm.org/settings/panel/emailpreferences/ >>>>>> <D7799.21048.patch> >>>>> >>>> >>> >> >> >> _______________________________________________ >> cfe-commits mailing list >> [email protected] >> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits > _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
