[llvm] r252862 - [FunctionAttrs] Identify norecurse functions

James Molloy via llvm-commits llvm-commits at lists.llvm.org
Sat Dec 19 10:10:26 PST 2015 

Not that I can think of*. I was half considering that when first
implementing but the finalisation method seemed (a bit) cleaner. If It
makes your life easier and conforms to the pass contract I'm happy.

* this gmail predictive reply feature is getting fairly accurate...
On Sat, 19 Dec 2015 at 18:00, Chandler Carruth <chandlerc at gmail.com> wrote:

> Any particular reason to not use a separate module pass? It won't be able
> to use a cache of results, but that doesn't seem hard to compute from first
> principals, and it shouldn't change the big-O...
>
> On Sat, Dec 19, 2015, 07:10 James Molloy <james at jamesmolloy.co.uk> wrote:
>
>> Hey Chandler,
>>
>> Thanks for raising this. Myself and Mehdi both felt it was ugly, but we
>> couldn't think of a better way of doing this.
>>
>> The primary consumer of this attribute is GlobalOpt. It uses this to
>> localize global variables. Because GlobalOpt is a module pass, I expect
>> (and do observe, in practice) that the finalization on this CGSCC pass is
>> called before GlobalOpt::runOnModule().
>>
>> I welcome a better solution!
>>
>> Cheers,
>>
>> James
>>
>> On Sat, 19 Dec 2015 at 01:29 Chandler Carruth via llvm-commits <
>> llvm-commits at lists.llvm.org> wrote:
>>
>>> Hey James,
>>>
>>> Sorry I'm so late to the party here, but I have a pretty serious concern
>>> with how you're using the pass manager here...
>>>
>>> On Thu, Nov 12, 2015 at 12:55 AM James Molloy via llvm-commits <
>>> llvm-commits at lists.llvm.org> wrote:
>>>
>>>>
>>>> +bool FunctionAttrs::doFinalization(CallGraph &CG) {
>>>> +  bool Changed = false;
>>>> +  // When iterating over SCCs we visit functions in a bottom-up
>>>> fashion. Some of
>>>> +  // the rules we have for identifying norecurse functions work best
>>>> with a
>>>> +  // top-down walk, so look again at all the functions we previously
>>>> marked as
>>>> +  // worth revisiting, in top-down order.
>>>> +  for (auto &F : reverse(Revisit))
>>>> +    if (F)
>>>> +      Changed |=
>>>> addNoRecurseAttrsTopDownOnly(cast<Function>((Value*)F));
>>>>    return Changed;
>>>>  }
>>>>
>>>
>>> I don't think this is a good model for getting the top-down behavior you
>>> want.
>>>
>>> When do you observe this getting called? I would expect it to get called
>>> *after* all inlining has already happened, and most of the rest of the
>>> CGSCC pass manager as well. Is this what you intend?
>>>
>>> I think the finalization and initialization should really be reserved
>>> for setup and teardown of the pass, not for transforming the IR to the
>>> extent possible. Sometimes we introduce declarations, but not really for
>>> mutating the IR per-se.
>>>
>>> For example, I can no longer easily convert this pass to the new pass
>>> manager as there no analog there at all.
>>>
>>> I think the correct way to structure this is to have a separate module
>>> pass that does the top-down walk of the CG... But I'd like to better
>>> understand the use case you have in mind for this top-down propagation.
>>> What is benefitting the most from these attributes being inferred? What
>>> passes need to have visibility into it?
>>>
>>> Thanks,
>>> -Chandler
>>>
>>>
>>>>
>>>> Modified:
>>>> llvm/trunk/test/Analysis/TypeBasedAliasAnalysis/functionattrs.ll
>>>> URL:
>>>> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Analysis/TypeBasedAliasAnalysis/functionattrs.ll?rev=252862&r1=252861&r2=252862&view=diff
>>>>
>>>> ==============================================================================
>>>> --- llvm/trunk/test/Analysis/TypeBasedAliasAnalysis/functionattrs.ll
>>>> (original)
>>>> +++ llvm/trunk/test/Analysis/TypeBasedAliasAnalysis/functionattrs.ll
>>>> Thu Nov 12 02:53:04 2015
>>>> @@ -30,7 +30,7 @@ define void @test1_yes(i32* %p) nounwind
>>>>    ret void
>>>>  }
>>>>
>>>> -; CHECK: define void @test1_no(i32* %p) #1 {
>>>> +; CHECK: define void @test1_no(i32* %p) #3 {
>>>>  define void @test1_no(i32* %p) nounwind {
>>>>    call void @callee(i32* %p), !tbaa !2
>>>>    ret void
>>>> @@ -72,9 +72,11 @@ define i32 @test3_no(i8* %p) nounwind {
>>>>  declare void @callee(i32* %p) nounwind
>>>>  declare void @llvm.memcpy.p0i8.p0i8.i64(i8*, i8*, i64, i32, i1)
>>>> nounwind
>>>>
>>>> -; CHECK: attributes #0 = { nounwind readnone }
>>>> -; CHECK: attributes #1 = { nounwind }
>>>> +; CHECK: attributes #0 = { norecurse nounwind readnone }
>>>> +; CHECK: attributes #1 = { norecurse nounwind }
>>>>  ; CHECK: attributes #2 = { nounwind readonly }
>>>> +; CHECK: attributes #3 = { nounwind }
>>>> +; CHECK: attributes #4 = { nounwind argmemonly }
>>>>
>>>>  ; Root note.
>>>>  !0 = !{ }
>>>>
>>>> Modified:
>>>> llvm/trunk/test/Transforms/FunctionAttrs/2008-09-03-ReadNone.ll
>>>> URL:
>>>> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/FunctionAttrs/2008-09-03-ReadNone.ll?rev=252862&r1=252861&r2=252862&view=diff
>>>>
>>>> ==============================================================================
>>>> --- llvm/trunk/test/Transforms/FunctionAttrs/2008-09-03-ReadNone.ll
>>>> (original)
>>>> +++ llvm/trunk/test/Transforms/FunctionAttrs/2008-09-03-ReadNone.ll Thu
>>>> Nov 12 02:53:04 2015
>>>> @@ -10,15 +10,16 @@ define i32 @f() {
>>>>         ret i32 %tmp
>>>>  }
>>>>
>>>> -; CHECK: define i32 @g() #0
>>>> +; CHECK: define i32 @g() #1
>>>>  define i32 @g() readonly {
>>>>         ret i32 0
>>>>  }
>>>>
>>>> -; CHECK: define i32 @h() #0
>>>> +; CHECK: define i32 @h() #1
>>>>  define i32 @h() readnone {
>>>>         %tmp = load i32, i32* @x                ; <i32> [#uses=1]
>>>>         ret i32 %tmp
>>>>  }
>>>>
>>>>  ; CHECK: attributes #0 = { readnone }
>>>> +; CHECK: attributes #1 = { norecurse readnone }
>>>>
>>>> Modified:
>>>> llvm/trunk/test/Transforms/FunctionAttrs/2010-10-30-volatile.ll
>>>> URL:
>>>> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/FunctionAttrs/2010-10-30-volatile.ll?rev=252862&r1=252861&r2=252862&view=diff
>>>>
>>>> ==============================================================================
>>>> --- llvm/trunk/test/Transforms/FunctionAttrs/2010-10-30-volatile.ll
>>>> (original)
>>>> +++ llvm/trunk/test/Transforms/FunctionAttrs/2010-10-30-volatile.ll Thu
>>>> Nov 12 02:53:04 2015
>>>> @@ -4,7 +4,9 @@
>>>>  @g = constant i32 1
>>>>
>>>>  define void @foo() {
>>>> -; CHECK: void @foo() {
>>>> +; CHECK: void @foo() #0 {
>>>>    %tmp = load volatile i32, i32* @g
>>>>    ret void
>>>>  }
>>>> +
>>>> +; CHECK: attributes #0 = { norecurse }
>>>>
>>>> Modified: llvm/trunk/test/Transforms/FunctionAttrs/atomic.ll
>>>> URL:
>>>> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/FunctionAttrs/atomic.ll?rev=252862&r1=252861&r2=252862&view=diff
>>>>
>>>> ==============================================================================
>>>> --- llvm/trunk/test/Transforms/FunctionAttrs/atomic.ll (original)
>>>> +++ llvm/trunk/test/Transforms/FunctionAttrs/atomic.ll Thu Nov 12
>>>> 02:53:04 2015
>>>> @@ -19,5 +19,5 @@ entry:
>>>>    ret i32 %r
>>>>  }
>>>>
>>>> -; CHECK: attributes #0 = { readnone ssp uwtable }
>>>> -; CHECK: attributes #1 = { ssp uwtable }
>>>> +; CHECK: attributes #0 = { norecurse readnone ssp uwtable }
>>>> +; CHECK: attributes #1 = { norecurse ssp uwtable }
>>>>
>>>> Added: llvm/trunk/test/Transforms/FunctionAttrs/norecurse.ll
>>>> URL:
>>>> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/FunctionAttrs/norecurse.ll?rev=252862&view=auto
>>>>
>>>> ==============================================================================
>>>> --- llvm/trunk/test/Transforms/FunctionAttrs/norecurse.ll (added)
>>>> +++ llvm/trunk/test/Transforms/FunctionAttrs/norecurse.ll Thu Nov 12
>>>> 02:53:04 2015
>>>> @@ -0,0 +1,57 @@
>>>> +; RUN: opt < %s -basicaa -functionattrs -S | FileCheck %s
>>>> +
>>>> +; CHECK: define i32 @leaf() #0
>>>> +define i32 @leaf() {
>>>> +  ret i32 1
>>>> +}
>>>> +
>>>> +; CHECK: define i32 @self_rec() #1
>>>> +define i32 @self_rec() {
>>>> +  %a = call i32 @self_rec()
>>>> +  ret i32 4
>>>> +}
>>>> +
>>>> +; CHECK: define i32 @indirect_rec() #1
>>>> +define i32 @indirect_rec() {
>>>> +  %a = call i32 @indirect_rec2()
>>>> +  ret i32 %a
>>>> +}
>>>> +; CHECK: define i32 @indirect_rec2() #1
>>>> +define i32 @indirect_rec2() {
>>>> +  %a = call i32 @indirect_rec()
>>>> +  ret i32 %a
>>>> +}
>>>> +
>>>> +; CHECK: define i32 @extern() #1
>>>> +define i32 @extern() {
>>>> +  %a = call i32 @k()
>>>> +  ret i32 %a
>>>> +}
>>>> +declare i32 @k() readnone
>>>> +
>>>> +; CHECK: define internal i32 @called_by_norecurse() #0
>>>> +define internal i32 @called_by_norecurse() {
>>>> +  %a = call i32 @k()
>>>> +  ret i32 %a
>>>> +}
>>>> +define void @m() norecurse {
>>>> +  %a = call i32 @called_by_norecurse()
>>>> +  ret void
>>>> +}
>>>> +
>>>> +; CHECK: define internal i32 @called_by_norecurse_indirectly() #0
>>>> +define internal i32 @called_by_norecurse_indirectly() {
>>>> +  %a = call i32 @k()
>>>> +  ret i32 %a
>>>> +}
>>>> +define internal void @o() {
>>>> +  %a = call i32 @called_by_norecurse_indirectly()
>>>> +  ret void
>>>> +}
>>>> +define void @p() norecurse {
>>>> +  call void @o()
>>>> +  ret void
>>>> +}
>>>> +
>>>> +; CHECK: attributes #0 = { norecurse readnone }
>>>> +; CHECK: attributes #1 = { readnone }
>>>>
>>>> Modified: llvm/trunk/test/Transforms/FunctionAttrs/optnone.ll
>>>> URL:
>>>> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/FunctionAttrs/optnone.ll?rev=252862&r1=252861&r2=252862&view=diff
>>>>
>>>> ==============================================================================
>>>> --- llvm/trunk/test/Transforms/FunctionAttrs/optnone.ll (original)
>>>> +++ llvm/trunk/test/Transforms/FunctionAttrs/optnone.ll Thu Nov 12
>>>> 02:53:04 2015
>>>> @@ -16,9 +16,11 @@ define void @test_optnone(i8* %p) noinli
>>>>
>>>>  declare i8 @strlen(i8*) noinline optnone
>>>>  ; CHECK-LABEL: @strlen
>>>> -; CHECK: (i8*) #1
>>>> +; CHECK: (i8*) #2
>>>>
>>>>  ; CHECK-LABEL: attributes #0
>>>> -; CHECK: = { readnone }
>>>> +; CHECK: = { norecurse readnone }
>>>>  ; CHECK-LABEL: attributes #1
>>>> +; CHECK: = { noinline norecurse optnone }
>>>> +; CHECK-LABEL: attributes #2
>>>>  ; CHECK: = { noinline optnone }
>>>>
>>>>
>>>> _______________________________________________
>>>> llvm-commits mailing list
>>>> llvm-commits at lists.llvm.org
>>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>>>>
>>> _______________________________________________
>>> llvm-commits mailing list
>>> llvm-commits at lists.llvm.org
>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>>>
>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20151219/300a3276/attachment.html>

More information about the llvm-commits mailing list