[llvm] r252871 - Revert "Revert "[FunctionAttrs] Identify norecurse functions""

David Blaikie via llvm-commits llvm-commits at lists.llvm.org
Thu Nov 12 08:24:36 PST 2015


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/791146d6/attachment.html>


More information about the llvm-commits mailing list