<p dir="ltr">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...</p>
<br><div class="gmail_quote"><div dir="ltr">On Sat, Dec 19, 2015, 07:10 James Molloy <<a href="mailto:james@jamesmolloy.co.uk">james@jamesmolloy.co.uk</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">Hey Chandler,<div><br></div><div>Thanks for raising this. Myself and Mehdi both felt it was ugly, but we couldn't think of a better way of doing this.</div><div><br></div><div>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().</div><div><br></div><div>I welcome a better solution!</div><div><br></div><div>Cheers,</div><div><br></div><div>James</div></div><br><div class="gmail_quote"><div dir="ltr">On Sat, 19 Dec 2015 at 01:29 Chandler Carruth via llvm-commits <<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">Hey James,<div><br></div><div>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...</div><br><div class="gmail_quote"></div></div><div dir="ltr"><div class="gmail_quote"><div dir="ltr">On Thu, Nov 12, 2015 at 12:55 AM James Molloy via llvm-commits <<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><br>
+bool FunctionAttrs::doFinalization(CallGraph &CG) {<br>
+ bool Changed = false;<br>
+ // When iterating over SCCs we visit functions in a bottom-up fashion. Some of<br>
+ // the rules we have for identifying norecurse functions work best with a<br>
+ // top-down walk, so look again at all the functions we previously marked as<br>
+ // worth revisiting, in top-down order.<br>
+ for (auto &F : reverse(Revisit))<br>
+ if (F)<br>
+ Changed |= addNoRecurseAttrsTopDownOnly(cast<Function>((Value*)F));<br>
return Changed;<br>
}<br></blockquote><div><br></div></div></div><div dir="ltr"><div class="gmail_quote"><div>I don't think this is a good model for getting the top-down behavior you want.</div><div><br></div><div>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?</div><div><br></div><div>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.</div><div><br></div><div>For example, I can no longer easily convert this pass to the new pass manager as there no analog there at all.</div><div><br></div><div>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?</div><div><br></div><div>Thanks,</div><div>-Chandler</div></div></div><div dir="ltr"><div class="gmail_quote"><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
Modified: llvm/trunk/test/Analysis/TypeBasedAliasAnalysis/functionattrs.ll<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Analysis/TypeBasedAliasAnalysis/functionattrs.ll?rev=252862&r1=252861&r2=252862&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Analysis/TypeBasedAliasAnalysis/functionattrs.ll?rev=252862&r1=252861&r2=252862&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/test/Analysis/TypeBasedAliasAnalysis/functionattrs.ll (original)<br>
+++ llvm/trunk/test/Analysis/TypeBasedAliasAnalysis/functionattrs.ll Thu Nov 12 02:53:04 2015<br>
@@ -30,7 +30,7 @@ define void @test1_yes(i32* %p) nounwind<br>
ret void<br>
}<br>
<br>
-; CHECK: define void @test1_no(i32* %p) #1 {<br>
+; CHECK: define void @test1_no(i32* %p) #3 {<br>
define void @test1_no(i32* %p) nounwind {<br>
call void @callee(i32* %p), !tbaa !2<br>
ret void<br>
@@ -72,9 +72,11 @@ define i32 @test3_no(i8* %p) nounwind {<br>
declare void @callee(i32* %p) nounwind<br>
declare void @llvm.memcpy.p0i8.p0i8.i64(i8*, i8*, i64, i32, i1) nounwind<br>
<br>
-; CHECK: attributes #0 = { nounwind readnone }<br>
-; CHECK: attributes #1 = { nounwind }<br>
+; CHECK: attributes #0 = { norecurse nounwind readnone }<br>
+; CHECK: attributes #1 = { norecurse nounwind }<br>
; CHECK: attributes #2 = { nounwind readonly }<br>
+; CHECK: attributes #3 = { nounwind }<br>
+; CHECK: attributes #4 = { nounwind argmemonly }<br>
<br>
; Root note.<br>
!0 = !{ }<br>
<br>
Modified: llvm/trunk/test/Transforms/FunctionAttrs/2008-09-03-ReadNone.ll<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/FunctionAttrs/2008-09-03-ReadNone.ll?rev=252862&r1=252861&r2=252862&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/FunctionAttrs/2008-09-03-ReadNone.ll?rev=252862&r1=252861&r2=252862&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/test/Transforms/FunctionAttrs/2008-09-03-ReadNone.ll (original)<br>
+++ llvm/trunk/test/Transforms/FunctionAttrs/2008-09-03-ReadNone.ll Thu Nov 12 02:53:04 2015<br>
@@ -10,15 +10,16 @@ define i32 @f() {<br>
ret i32 %tmp<br>
}<br>
<br>
-; CHECK: define i32 @g() #0<br>
+; CHECK: define i32 @g() #1<br>
define i32 @g() readonly {<br>
ret i32 0<br>
}<br>
<br>
-; CHECK: define i32 @h() #0<br>
+; CHECK: define i32 @h() #1<br>
define i32 @h() readnone {<br>
%tmp = load i32, i32* @x ; <i32> [#uses=1]<br>
ret i32 %tmp<br>
}<br>
<br>
; CHECK: attributes #0 = { readnone }<br>
+; CHECK: attributes #1 = { norecurse readnone }<br>
<br>
Modified: llvm/trunk/test/Transforms/FunctionAttrs/2010-10-30-volatile.ll<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/FunctionAttrs/2010-10-30-volatile.ll?rev=252862&r1=252861&r2=252862&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/FunctionAttrs/2010-10-30-volatile.ll?rev=252862&r1=252861&r2=252862&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/test/Transforms/FunctionAttrs/2010-10-30-volatile.ll (original)<br>
+++ llvm/trunk/test/Transforms/FunctionAttrs/2010-10-30-volatile.ll Thu Nov 12 02:53:04 2015<br>
@@ -4,7 +4,9 @@<br>
@g = constant i32 1<br>
<br>
define void @foo() {<br>
-; CHECK: void @foo() {<br>
+; CHECK: void @foo() #0 {<br>
%tmp = load volatile i32, i32* @g<br>
ret void<br>
}<br>
+<br>
+; CHECK: attributes #0 = { norecurse }<br>
<br>
Modified: llvm/trunk/test/Transforms/FunctionAttrs/atomic.ll<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/FunctionAttrs/atomic.ll?rev=252862&r1=252861&r2=252862&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/FunctionAttrs/atomic.ll?rev=252862&r1=252861&r2=252862&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/test/Transforms/FunctionAttrs/atomic.ll (original)<br>
+++ llvm/trunk/test/Transforms/FunctionAttrs/atomic.ll Thu Nov 12 02:53:04 2015<br>
@@ -19,5 +19,5 @@ entry:<br>
ret i32 %r<br>
}<br>
<br>
-; CHECK: attributes #0 = { readnone ssp uwtable }<br>
-; CHECK: attributes #1 = { ssp uwtable }<br>
+; CHECK: attributes #0 = { norecurse readnone ssp uwtable }<br>
+; CHECK: attributes #1 = { norecurse ssp uwtable }<br>
<br>
Added: llvm/trunk/test/Transforms/FunctionAttrs/norecurse.ll<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/FunctionAttrs/norecurse.ll?rev=252862&view=auto" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/FunctionAttrs/norecurse.ll?rev=252862&view=auto</a><br>
==============================================================================<br>
--- llvm/trunk/test/Transforms/FunctionAttrs/norecurse.ll (added)<br>
+++ llvm/trunk/test/Transforms/FunctionAttrs/norecurse.ll Thu Nov 12 02:53:04 2015<br>
@@ -0,0 +1,57 @@<br>
+; RUN: opt < %s -basicaa -functionattrs -S | FileCheck %s<br>
+<br>
+; CHECK: define i32 @leaf() #0<br>
+define i32 @leaf() {<br>
+ ret i32 1<br>
+}<br>
+<br>
+; CHECK: define i32 @self_rec() #1<br>
+define i32 @self_rec() {<br>
+ %a = call i32 @self_rec()<br>
+ ret i32 4<br>
+}<br>
+<br>
+; CHECK: define i32 @indirect_rec() #1<br>
+define i32 @indirect_rec() {<br>
+ %a = call i32 @indirect_rec2()<br>
+ ret i32 %a<br>
+}<br>
+; CHECK: define i32 @indirect_rec2() #1<br>
+define i32 @indirect_rec2() {<br>
+ %a = call i32 @indirect_rec()<br>
+ ret i32 %a<br>
+}<br>
+<br>
+; CHECK: define i32 @extern() #1<br>
+define i32 @extern() {<br>
+ %a = call i32 @k()<br>
+ ret i32 %a<br>
+}<br>
+declare i32 @k() readnone<br>
+<br>
+; CHECK: define internal i32 @called_by_norecurse() #0<br>
+define internal i32 @called_by_norecurse() {<br>
+ %a = call i32 @k()<br>
+ ret i32 %a<br>
+}<br>
+define void @m() norecurse {<br>
+ %a = call i32 @called_by_norecurse()<br>
+ ret void<br>
+}<br>
+<br>
+; CHECK: define internal i32 @called_by_norecurse_indirectly() #0<br>
+define internal i32 @called_by_norecurse_indirectly() {<br>
+ %a = call i32 @k()<br>
+ ret i32 %a<br>
+}<br>
+define internal void @o() {<br>
+ %a = call i32 @called_by_norecurse_indirectly()<br>
+ ret void<br>
+}<br>
+define void @p() norecurse {<br>
+ call void @o()<br>
+ ret void<br>
+}<br>
+<br>
+; CHECK: attributes #0 = { norecurse readnone }<br>
+; CHECK: attributes #1 = { readnone }<br>
<br>
Modified: llvm/trunk/test/Transforms/FunctionAttrs/optnone.ll<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/FunctionAttrs/optnone.ll?rev=252862&r1=252861&r2=252862&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/FunctionAttrs/optnone.ll?rev=252862&r1=252861&r2=252862&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/test/Transforms/FunctionAttrs/optnone.ll (original)<br>
+++ llvm/trunk/test/Transforms/FunctionAttrs/optnone.ll Thu Nov 12 02:53:04 2015<br>
@@ -16,9 +16,11 @@ define void @test_optnone(i8* %p) noinli<br>
<br>
declare i8 @strlen(i8*) noinline optnone<br>
; CHECK-LABEL: @strlen<br>
-; CHECK: (i8*) #1<br>
+; CHECK: (i8*) #2<br>
<br>
; CHECK-LABEL: attributes #0<br>
-; CHECK: = { readnone }<br>
+; CHECK: = { norecurse readnone }<br>
; CHECK-LABEL: attributes #1<br>
+; CHECK: = { noinline norecurse optnone }<br>
+; CHECK-LABEL: attributes #2<br>
; CHECK: = { noinline optnone }<br>
<br>
<br>
_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a><br>
</blockquote></div></div>
_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a><br>
</blockquote></div>
</blockquote></div>