<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Thu, Nov 12, 2015 at 8:17 AM, James Molloy <span dir="ltr"><<a href="mailto:james@jamesmolloy.co.uk" target="_blank">james@jamesmolloy.co.uk</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">Hi David,<div><br></div><div>Understood. I'll make my un-revert commit messages more detailed in future.</div><div><br></div><div>For the record, the problem was that the expected ordering of attribute names when printed had changed (affecting function-attrs.ll). I'm still not quite sure why this happened.</div></div></blockquote><div><br></div><div>Are you investigating that? Working around unknowns is always a bit concerning, though sometimes the right call for sure. (the only concern I'd have here is if it's non-deterministic output order - but it doesn't sound like that's the case)</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><span class="HOEnZb"><font color="#888888"><div><br></div><div>James</div></font></span></div><div class="HOEnZb"><div class="h5"><br><div class="gmail_quote"><div dir="ltr">On Thu, 12 Nov 2015 at 16:14 David Blaikie 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"><div class="gmail_extra"><div class="gmail_quote">On Thu, Nov 12, 2015 at 2:55 AM, James Molloy via llvm-commits <span dir="ltr"><<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: jamesm<br>
Date: Thu Nov 12 04:55:20 2015<br>
New Revision: 252871<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=252871&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=252871&view=rev</a><br>
Log:<br>
Revert "Revert "[FunctionAttrs] Identify norecurse functions""<br>
<br>
This reapplies this patch, with test fixes.<br></blockquote><div><br></div></div></div></div><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div>It'd be helpful to detail the fixes (& whatever extra testing was involved (which buildbot scenarios failed, etc) in finding them) so that those changes can get extra scrutiny by anyone who's already reviewed the original patch & just wants to check the new changes.</div></div></div></div><div dir="ltr"><div class="gmail_extra"><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>
Added:<br>
llvm/trunk/test/Transforms/FunctionAttrs/norecurse.ll<br>
Modified:<br>
llvm/trunk/lib/Transforms/IPO/FunctionAttrs.cpp<br>
llvm/trunk/test/Analysis/TypeBasedAliasAnalysis/functionattrs.ll<br>
llvm/trunk/test/Transforms/FunctionAttrs/2008-09-03-ReadNone.ll<br>
llvm/trunk/test/Transforms/FunctionAttrs/2010-10-30-volatile.ll<br>
llvm/trunk/test/Transforms/FunctionAttrs/atomic.ll<br>
llvm/trunk/test/Transforms/FunctionAttrs/optnone.ll<br>
<br>
Modified: llvm/trunk/lib/Transforms/IPO/FunctionAttrs.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/IPO/FunctionAttrs.cpp?rev=252871&r1=252870&r2=252871&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/IPO/FunctionAttrs.cpp?rev=252871&r1=252870&r2=252871&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/lib/Transforms/IPO/FunctionAttrs.cpp (original)<br>
+++ llvm/trunk/lib/Transforms/IPO/FunctionAttrs.cpp Thu Nov 12 04:55:20 2015<br>
@@ -50,6 +50,7 @@ STATISTIC(NumReadOnlyArg, "Number of arg<br>
STATISTIC(NumNoAlias, "Number of function returns marked noalias");<br>
STATISTIC(NumNonNullReturn, "Number of function returns marked nonnull");<br>
STATISTIC(NumAnnotated, "Number of attributes added to library functions");<br>
+STATISTIC(NumNoRecurse, "Number of functions marked as norecurse");<br>
<br>
namespace {<br>
typedef SmallSetVector<Function *, 8> SCCNodeSet;<br>
@@ -63,7 +64,12 @@ struct FunctionAttrs : public CallGraphS<br>
}<br>
<br>
bool runOnSCC(CallGraphSCC &SCC) override;<br>
-<br>
+ bool doInitialization(CallGraph &CG) override {<br>
+ Revisit.clear();<br>
+ return false;<br>
+ }<br>
+ bool doFinalization(CallGraph &CG) override;<br>
+<br>
void getAnalysisUsage(AnalysisUsage &AU) const override {<br>
AU.setPreservesCFG();<br>
AU.addRequired<AssumptionCacheTracker>();<br>
@@ -73,6 +79,7 @@ struct FunctionAttrs : public CallGraphS<br>
<br>
private:<br>
TargetLibraryInfo *TLI;<br>
+ SmallVector<WeakVH,16> Revisit;<br>
};<br>
}<br>
<br>
@@ -976,6 +983,14 @@ static void setDoesNotAlias(Function &F,<br>
}<br>
}<br>
<br>
+static bool setDoesNotRecurse(Function &F) {<br>
+ if (F.doesNotRecurse())<br>
+ return false;<br>
+ F.setDoesNotRecurse();<br>
+ ++NumNoRecurse;<br>
+ return true;<br>
+}<br>
+<br>
/// Analyze the name and prototype of the given function and set any applicable<br>
/// attributes.<br>
///<br>
@@ -1779,6 +1794,55 @@ static bool inferPrototypeAttributes(Fun<br>
return true;<br>
}<br>
<br>
+static bool addNoRecurseAttrs(const CallGraphSCC &SCC,<br>
+ SmallVectorImpl<WeakVH> &Revisit) {<br>
+ // Try and identify functions that do not recurse.<br>
+<br>
+ // If the SCC contains multiple nodes we know for sure there is recursion.<br>
+ if (!SCC.isSingular())<br>
+ return false;<br>
+<br>
+ const CallGraphNode *CGN = *SCC.begin();<br>
+ Function *F = CGN->getFunction();<br>
+ if (!F || F->isDeclaration() || F->doesNotRecurse())<br>
+ return false;<br>
+<br>
+ // If all of the calls in F are identifiable and are to norecurse functions, F<br>
+ // is norecurse. This check also detects self-recursion as F is not currently<br>
+ // marked norecurse, so any called from F to F will not be marked norecurse.<br>
+ if (std::all_of(CGN->begin(), CGN->end(),<br>
+ [](const CallGraphNode::CallRecord &CR) {<br>
+ Function *F = CR.second->getFunction();<br>
+ return F && F->doesNotRecurse();<br>
+ }))<br>
+ // Function calls a potentially recursive function.<br>
+ return setDoesNotRecurse(*F);<br>
+<br>
+ // We know that F is not obviously recursive, but we haven't been able to<br>
+ // prove that it doesn't actually recurse. Add it to the Revisit list to try<br>
+ // again top-down later.<br>
+ Revisit.push_back(F);<br>
+ return false;<br>
+}<br>
+<br>
+static bool addNoRecurseAttrsTopDownOnly(Function *F) {<br>
+ // If F is internal and all uses are in norecurse functions, then F is also<br>
+ // norecurse.<br>
+ if (F->doesNotRecurse())<br>
+ return false;<br>
+ if (F->hasInternalLinkage()) {<br>
+ for (auto *U : F->users())<br>
+ if (auto *I = dyn_cast<Instruction>(U)) {<br>
+ if (!I->getParent()->getParent()->doesNotRecurse())<br>
+ return false;<br>
+ } else {<br>
+ return false;<br>
+ }<br>
+ return setDoesNotRecurse(*F);<br>
+ }<br>
+ return false;<br>
+}<br>
+<br>
bool FunctionAttrs::runOnSCC(CallGraphSCC &SCC) {<br>
TLI = &getAnalysis<TargetLibraryInfoWrapperPass>().getTLI();<br>
bool Changed = false;<br>
@@ -1826,6 +1890,19 @@ bool FunctionAttrs::runOnSCC(CallGraphSC<br>
Changed |= addNoAliasAttrs(SCCNodes);<br>
Changed |= addNonNullAttrs(SCCNodes, *TLI);<br>
}<br>
+<br>
+ Changed |= addNoRecurseAttrs(SCC, Revisit);<br>
+ return Changed;<br>
+}<br>
<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>
<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=252871&r1=252870&r2=252871&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Analysis/TypeBasedAliasAnalysis/functionattrs.ll?rev=252871&r1=252870&r2=252871&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/test/Analysis/TypeBasedAliasAnalysis/functionattrs.ll (original)<br>
+++ llvm/trunk/test/Analysis/TypeBasedAliasAnalysis/functionattrs.ll Thu Nov 12 04:55:20 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 = { argmemonly nounwind }<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=252871&r1=252870&r2=252871&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/FunctionAttrs/2008-09-03-ReadNone.ll?rev=252871&r1=252870&r2=252871&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 04:55:20 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=252871&r1=252870&r2=252871&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/FunctionAttrs/2010-10-30-volatile.ll?rev=252871&r1=252870&r2=252871&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 04:55:20 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=252871&r1=252870&r2=252871&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/FunctionAttrs/atomic.ll?rev=252871&r1=252870&r2=252871&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/test/Transforms/FunctionAttrs/atomic.ll (original)<br>
+++ llvm/trunk/test/Transforms/FunctionAttrs/atomic.ll Thu Nov 12 04:55:20 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=252871&view=auto" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/FunctionAttrs/norecurse.ll?rev=252871&view=auto</a><br>
==============================================================================<br>
--- llvm/trunk/test/Transforms/FunctionAttrs/norecurse.ll (added)<br>
+++ llvm/trunk/test/Transforms/FunctionAttrs/norecurse.ll Thu Nov 12 04:55:20 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=252871&r1=252870&r2=252871&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/FunctionAttrs/optnone.ll?rev=252871&r1=252870&r2=252871&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/test/Transforms/FunctionAttrs/optnone.ll (original)<br>
+++ llvm/trunk/test/Transforms/FunctionAttrs/optnone.ll Thu Nov 12 04:55:20 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></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>
</div></div></blockquote></div><br></div></div>