[llvm] r252862 - [FunctionAttrs] Identify norecurse functions
Chandler Carruth via llvm-commits
llvm-commits at lists.llvm.org
Fri Dec 18 17:29:22 PST 2015
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
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20151219/e0332acc/attachment.html>
More information about the llvm-commits
mailing list