[llvm] r252862 - [FunctionAttrs] Identify norecurse functions

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

Cool, you should have several patches inbound then.

On Sat, Dec 19, 2015, 10:10 James Molloy <james at jamesmolloy.co.uk> wrote:

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

More information about the llvm-commits mailing list