[PATCH] D14769: [FunctionAttrs] Calls to libcalls don't cause a function to potentially recurse

[Duncan Exon Smith via llvm-commits]

Calls to exit() can recurse because of atexit() handlers.  The same might be true of other libcalls?

Also, other noreturn functions can recurse; it's a weird path but I don't think it's malformed:
--
void bar() noreturn;
int foo() {
static bool Return = false;
if (Return) return 5;
Return = true;
bar();
}

void bar() {
sink(foo());
exit();
}
--

> On Nov 18, 2015, at 3:51 AM, James Molloy <james.molloy at arm.com> wrote:
> 
> jmolloy created this revision.
> jmolloy added reviewers: joker.eph, manmanren, dexonsmith.
> jmolloy added a subscriber: llvm-commits.
> jmolloy set the repository for this revision to rL LLVM.
> 
> If a function calls a library function, that shouldn't be treated as potentially causing a cycle in the CFG. Additionally calls to noreturn functions are norecurse by definition.
> 
> Repository:
>  rL LLVM
> 
> http://reviews.llvm.org/D14769
> 
> Files:
>  lib/Transforms/IPO/FunctionAttrs.cpp
>  test/Transforms/FunctionAttrs/norecurse.ll
> 
> Index: test/Transforms/FunctionAttrs/norecurse.ll
> ===================================================================
> --- test/Transforms/FunctionAttrs/norecurse.ll
> +++ test/Transforms/FunctionAttrs/norecurse.ll
> @@ -1,5 +1,7 @@
> ; RUN: opt < %s -basicaa -functionattrs -S | FileCheck %s
> 
> +target triple = "x86_64-linux-gnu"
> +
> ; CHECK: define i32 @leaf() #0
> define i32 @leaf() {
>   ret i32 1
> @@ -53,5 +55,24 @@
>   ret void
> }
> 
> +; CHECK: define i32 @calls_exit() #2
> +define i32 @calls_exit() {
> +  call void @exit(i32 2)
> +  ret i32 5
> +}
> +
> +declare void @exit(i32) noreturn
> +
> +; CHECK: define i32 @calls_only_libcalls() #2
> +define i32 @calls_only_libcalls() {
> +  %a = call double @sin(double 0.0)
> +  call void @fclose(i8* null)
> +  ret i32 2
> +}
> +
> +declare double @sin(double)
> +declare void @fclose(i8*)
> +
> ; CHECK: attributes #0 = { norecurse readnone }
> ; CHECK: attributes #1 = { readnone }
> +; CHECK: attributes #2 = { norecurse }
> \ No newline at end of file
> Index: lib/Transforms/IPO/FunctionAttrs.cpp
> ===================================================================
> --- lib/Transforms/IPO/FunctionAttrs.cpp
> +++ lib/Transforms/IPO/FunctionAttrs.cpp
> @@ -1795,6 +1795,7 @@
> }
> 
> static bool addNoRecurseAttrs(const CallGraphSCC &SCC,
> +                              const TargetLibraryInfo &TLI,
>                               SmallVectorImpl<WeakVH> &Revisit) {
>   // Try and identify functions that do not recurse.
> 
> @@ -1804,16 +1805,29 @@
> 
>   const CallGraphNode *CGN = *SCC.begin();
>   Function *F = CGN->getFunction();
> -  if (!F || F->isDeclaration() || F->doesNotRecurse())
> +  if (!F || 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.
> +  // A noreturn function by definition cannot recurse.
> +  if (F->doesNotReturn())
> +    return setDoesNotRecurse(*F);
> +
> +  if (F->isDeclaration())
> +    return false;
> +
> +  auto IsLibCall = [&](Function *F) {
> +    LibFunc::Func LF;
> +    return TLI.getLibFunc(F->getName(), LF) && TLI.has(LF);
> +  };
> +
> +  // If all of the calls in F are identifiable and are to norecurse functions or
> +  // are libcalls, 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) {
> +                  [&](const CallGraphNode::CallRecord &CR) {
>                     Function *F = CR.second->getFunction();
> -                    return F && F->doesNotRecurse();
> +                    return F && (F->doesNotRecurse() || IsLibCall(F));
>                   }))
>     // Function calls a potentially recursive function.
>     return setDoesNotRecurse(*F);
> @@ -1890,8 +1904,8 @@
>     Changed |= addNoAliasAttrs(SCCNodes);
>     Changed |= addNonNullAttrs(SCCNodes, *TLI);
>   }
> -  
> -  Changed |= addNoRecurseAttrs(SCC, Revisit);
> +
> +  Changed |= addNoRecurseAttrs(SCC, *TLI, Revisit);
>   return Changed;
> }
> 
> 
> 
> <D14769.40490.patch>