[llvm] r252862 - [FunctionAttrs] Identify norecurse functions
James Molloy via llvm-commits
llvm-commits at lists.llvm.org
Sat Dec 19 07:09:55 PST 2015
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/11919a60/attachment.html>
More information about the llvm-commits
mailing list