[llvm] r252871 - Revert "Revert "[FunctionAttrs] Identify norecurse functions""
James Molloy via llvm-commits
llvm-commits at lists.llvm.org
Thu Nov 12 08:32:25 PST 2015
Hi David,
I should have been more clear. I'm pretty sure I know what happened - I put
the new attribute in correctly sorted order in the attribute printer where
it wasn't before in a previous version of the patch. What I'm not sure
about is how the test as committed ended up out of sync with the functional
code. I was doing quite a bit of cherry-picking and rebasing, and I must
have screwed up somehow.
I'm ensuring I re-build and re-run all tests when I "git am" into my
git-svn tree in future to avoid repeats.
That was what caused the (single) LLVM regression test failure. The Clang
ones were because I didn't have clang in the tree I was testing with, and
they needed updating.
Cheers,
James
On Thu, 12 Nov 2015 at 16:24 David Blaikie <dblaikie at gmail.com> wrote:
> On Thu, Nov 12, 2015 at 8:17 AM, James Molloy <james at jamesmolloy.co.uk>
> wrote:
>
>> Hi David,
>>
>> Understood. I'll make my un-revert commit messages more detailed in
>> future.
>>
>> 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.
>>
>
> 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)
>
>
>>
>> James
>>
>> On Thu, 12 Nov 2015 at 16:14 David Blaikie via llvm-commits <
>> llvm-commits at lists.llvm.org> wrote:
>>
>>> On Thu, Nov 12, 2015 at 2:55 AM, James Molloy via llvm-commits <
>>> llvm-commits at lists.llvm.org> wrote:
>>>
>>>> Author: jamesm
>>>> Date: Thu Nov 12 04:55:20 2015
>>>> New Revision: 252871
>>>>
>>>> URL: http://llvm.org/viewvc/llvm-project?rev=252871&view=rev
>>>> Log:
>>>> Revert "Revert "[FunctionAttrs] Identify norecurse functions""
>>>>
>>>> This reapplies this patch, with test fixes.
>>>>
>>>
>>> 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.
>>>
>>>
>>>>
>>>> Added:
>>>> llvm/trunk/test/Transforms/FunctionAttrs/norecurse.ll
>>>> Modified:
>>>> llvm/trunk/lib/Transforms/IPO/FunctionAttrs.cpp
>>>> llvm/trunk/test/Analysis/TypeBasedAliasAnalysis/functionattrs.ll
>>>> llvm/trunk/test/Transforms/FunctionAttrs/2008-09-03-ReadNone.ll
>>>> llvm/trunk/test/Transforms/FunctionAttrs/2010-10-30-volatile.ll
>>>> llvm/trunk/test/Transforms/FunctionAttrs/atomic.ll
>>>> llvm/trunk/test/Transforms/FunctionAttrs/optnone.ll
>>>>
>>>> Modified: llvm/trunk/lib/Transforms/IPO/FunctionAttrs.cpp
>>>> URL:
>>>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/IPO/FunctionAttrs.cpp?rev=252871&r1=252870&r2=252871&view=diff
>>>>
>>>> ==============================================================================
>>>> --- llvm/trunk/lib/Transforms/IPO/FunctionAttrs.cpp (original)
>>>> +++ llvm/trunk/lib/Transforms/IPO/FunctionAttrs.cpp Thu Nov 12 04:55:20
>>>> 2015
>>>> @@ -50,6 +50,7 @@ STATISTIC(NumReadOnlyArg, "Number of arg
>>>> STATISTIC(NumNoAlias, "Number of function returns marked noalias");
>>>> STATISTIC(NumNonNullReturn, "Number of function returns marked
>>>> nonnull");
>>>> STATISTIC(NumAnnotated, "Number of attributes added to library
>>>> functions");
>>>> +STATISTIC(NumNoRecurse, "Number of functions marked as norecurse");
>>>>
>>>> namespace {
>>>> typedef SmallSetVector<Function *, 8> SCCNodeSet;
>>>> @@ -63,7 +64,12 @@ struct FunctionAttrs : public CallGraphS
>>>> }
>>>>
>>>> bool runOnSCC(CallGraphSCC &SCC) override;
>>>> -
>>>> + bool doInitialization(CallGraph &CG) override {
>>>> + Revisit.clear();
>>>> + return false;
>>>> + }
>>>> + bool doFinalization(CallGraph &CG) override;
>>>> +
>>>> void getAnalysisUsage(AnalysisUsage &AU) const override {
>>>> AU.setPreservesCFG();
>>>> AU.addRequired<AssumptionCacheTracker>();
>>>> @@ -73,6 +79,7 @@ struct FunctionAttrs : public CallGraphS
>>>>
>>>> private:
>>>> TargetLibraryInfo *TLI;
>>>> + SmallVector<WeakVH,16> Revisit;
>>>> };
>>>> }
>>>>
>>>> @@ -976,6 +983,14 @@ static void setDoesNotAlias(Function &F,
>>>> }
>>>> }
>>>>
>>>> +static bool setDoesNotRecurse(Function &F) {
>>>> + if (F.doesNotRecurse())
>>>> + return false;
>>>> + F.setDoesNotRecurse();
>>>> + ++NumNoRecurse;
>>>> + return true;
>>>> +}
>>>> +
>>>> /// Analyze the name and prototype of the given function and set any
>>>> applicable
>>>> /// attributes.
>>>> ///
>>>> @@ -1779,6 +1794,55 @@ static bool inferPrototypeAttributes(Fun
>>>> return true;
>>>> }
>>>>
>>>> +static bool addNoRecurseAttrs(const CallGraphSCC &SCC,
>>>> + SmallVectorImpl<WeakVH> &Revisit) {
>>>> + // Try and identify functions that do not recurse.
>>>> +
>>>> + // If the SCC contains multiple nodes we know for sure there is
>>>> recursion.
>>>> + if (!SCC.isSingular())
>>>> + return false;
>>>> +
>>>> + const CallGraphNode *CGN = *SCC.begin();
>>>> + Function *F = CGN->getFunction();
>>>> + if (!F || F->isDeclaration() || F->doesNotRecurse())
>>>> + return false;
>>>> +
>>>> + // If all of the calls in F are identifiable and are to norecurse
>>>> functions, F
>>>> + // is norecurse. This check also detects self-recursion as F is not
>>>> currently
>>>> + // marked norecurse, so any called from F to F will not be marked
>>>> norecurse.
>>>> + if (std::all_of(CGN->begin(), CGN->end(),
>>>> + [](const CallGraphNode::CallRecord &CR) {
>>>> + Function *F = CR.second->getFunction();
>>>> + return F && F->doesNotRecurse();
>>>> + }))
>>>> + // Function calls a potentially recursive function.
>>>> + return setDoesNotRecurse(*F);
>>>> +
>>>> + // We know that F is not obviously recursive, but we haven't been
>>>> able to
>>>> + // prove that it doesn't actually recurse. Add it to the Revisit
>>>> list to try
>>>> + // again top-down later.
>>>> + Revisit.push_back(F);
>>>> + return false;
>>>> +}
>>>> +
>>>> +static bool addNoRecurseAttrsTopDownOnly(Function *F) {
>>>> + // If F is internal and all uses are in norecurse functions, then F
>>>> is also
>>>> + // norecurse.
>>>> + if (F->doesNotRecurse())
>>>> + return false;
>>>> + if (F->hasInternalLinkage()) {
>>>> + for (auto *U : F->users())
>>>> + if (auto *I = dyn_cast<Instruction>(U)) {
>>>> + if (!I->getParent()->getParent()->doesNotRecurse())
>>>> + return false;
>>>> + } else {
>>>> + return false;
>>>> + }
>>>> + return setDoesNotRecurse(*F);
>>>> + }
>>>> + return false;
>>>> +}
>>>> +
>>>> bool FunctionAttrs::runOnSCC(CallGraphSCC &SCC) {
>>>> TLI = &getAnalysis<TargetLibraryInfoWrapperPass>().getTLI();
>>>> bool Changed = false;
>>>> @@ -1826,6 +1890,19 @@ bool FunctionAttrs::runOnSCC(CallGraphSC
>>>> Changed |= addNoAliasAttrs(SCCNodes);
>>>> Changed |= addNonNullAttrs(SCCNodes, *TLI);
>>>> }
>>>> +
>>>> + Changed |= addNoRecurseAttrs(SCC, Revisit);
>>>> + return Changed;
>>>> +}
>>>>
>>>> +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;
>>>> }
>>>>
>>>> Modified:
>>>> llvm/trunk/test/Analysis/TypeBasedAliasAnalysis/functionattrs.ll
>>>> URL:
>>>> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Analysis/TypeBasedAliasAnalysis/functionattrs.ll?rev=252871&r1=252870&r2=252871&view=diff
>>>>
>>>> ==============================================================================
>>>> --- llvm/trunk/test/Analysis/TypeBasedAliasAnalysis/functionattrs.ll
>>>> (original)
>>>> +++ llvm/trunk/test/Analysis/TypeBasedAliasAnalysis/functionattrs.ll
>>>> Thu Nov 12 04:55:20 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 = { argmemonly nounwind }
>>>>
>>>> ; 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=252871&r1=252870&r2=252871&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 04:55:20 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=252871&r1=252870&r2=252871&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 04:55:20 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=252871&r1=252870&r2=252871&view=diff
>>>>
>>>> ==============================================================================
>>>> --- llvm/trunk/test/Transforms/FunctionAttrs/atomic.ll (original)
>>>> +++ llvm/trunk/test/Transforms/FunctionAttrs/atomic.ll Thu Nov 12
>>>> 04:55:20 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=252871&view=auto
>>>>
>>>> ==============================================================================
>>>> --- llvm/trunk/test/Transforms/FunctionAttrs/norecurse.ll (added)
>>>> +++ llvm/trunk/test/Transforms/FunctionAttrs/norecurse.ll Thu Nov 12
>>>> 04:55:20 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=252871&r1=252870&r2=252871&view=diff
>>>>
>>>> ==============================================================================
>>>> --- llvm/trunk/test/Transforms/FunctionAttrs/optnone.ll (original)
>>>> +++ llvm/trunk/test/Transforms/FunctionAttrs/optnone.ll Thu Nov 12
>>>> 04:55:20 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/20151112/9bb1cb27/attachment.html>
More information about the llvm-commits
mailing list