[llvm] r252862 - [FunctionAttrs] Identify norecurse functions

Sat Dec 19 10:00:35 PST 2015

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/517394e3/attachment.html>