[llvm] r252862 - [FunctionAttrs] Identify norecurse functions

Chandler Carruth via llvm-commits llvm-commits at lists.llvm.org
Sat Dec 19 14:05:36 PST 2015 

Chatted a bit with Mehdi. Seems like there is no specific performance
concern, so I'm gonna split it out and make it a proper top-down
propagation (which seems really useful anyways). I'm hoping to add a
top-down CGSCC pass manager at some point, but no need to wait for that to
arrive.

We also chatted a bit about norecurse generally and it sounds like there is
still a lot of TBD stuff with this attribute and so I'm going to try to
push it through for some of the basics. For example, currently we won't
mark functions that call strlen as norecurse which seems... silly. =]

On Sat, Dec 19, 2015 at 1:32 PM Chandler Carruth <chandlerc at gmail.com>
wrote:

> There is no easy way to do that. I think making that work would be very,
> very hard. What is the benefit? It doesn't seem to be terribly important to
> me, but maybe I'm missing something?
>
> On Sat, Dec 19, 2015 at 1:30 PM Mehdi Amini <mehdi.amini at apple.com> wrote:
>
>> About the “cache of results”: could the the bottom-up CGSCC Pass do what
>> it currently does and store the “Revisit” in another "analysis pass" that
>> the new ModulePass would then use to perform the top-down propagation?
>>
>> —
>> Mehdi
>>
>> On Dec 19, 2015, at 8:00 AM, Chandler Carruth via llvm-commits <
>> llvm-commits at lists.llvm.org> 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
>>>>
>>> _______________________________________________
>> 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/89c95feb/attachment-0001.html>

More information about the llvm-commits mailing list