Thanks Duncan. Committed in r236372.
Steven
> On May 1, 2015, at 5:30 PM, Duncan P. N. Exon Smith <dexonsm...@apple.com>
> wrote:
>
>
>> On 2015-Apr-30, at 13:04, Steven Wu <steve...@apple.com> 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 <steve...@apple.com> 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 <steve...@apple.com>
>>> 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
>>>> <dexonsm...@apple.com> wrote:
>>>>
>>>>
>>>>> On 2015 Mar 16, at 10:34, Steven Wu <steve...@apple.com> 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
>>>>>> <dexonsm...@apple.com> wrote:
>>>>>>
>>>>>>
>>>>>>> On 2015-Mar-02, at 14:56, Steven Wu <steve...@apple.com> 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
>>> cfe-commits@cs.uiuc.edu
>>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>>
>
_______________________________________________
cfe-commits mailing list
cfe-commits@cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
