[FunctionAttrs] Derive willreturn for fns with readonly` & `mustprogress`.

[fhahn]

Summary

Similar to D94125, derive willreturn for functions that are readonly and
mustprogress in FunctionAttrs.

To quote the reasoning from D94125:

Since D86233 we have `mustprogress` which, in combination with
`readonly`, implies `willreturn`. The idea is that every side-effect
has to be modeled as a "write". Consequently, `readonly` means there
is no side-effect, and `mustprogress` guarantees that we cannot "loop"
forever without side-effect.

[jdoerfert]

I left some comments that need to be addressed, not necessarily how I proposed it but somehow. LGTM otherwise.

llvm/lib/Transforms/IPO/FunctionAttrs.cpp

1433

|| F->hasFnAttr(Attribute::WillReturn)

1436

Changed = true; ;)

[fhahn]

llvm/lib/Transforms/IPO/FunctionAttrs.cpp

1436

Changed = true; ;)

yeah.

I guess a comment here might be helpful as well. It is possible that the function has been marked as noreturn earlier, e.g. because it does not have any returns. In such cases, will return seems a better choice, if possible.

[nikic]

llvm/lib/Transforms/IPO/FunctionAttrs.cpp

1436

Is it possible to keep both, or does that cause verifier errors? A function that always unwinds should be able to be both noreturn and willreturn without invoking UB.

[jdoerfert]

llvm/lib/Transforms/IPO/FunctionAttrs.cpp

1436

Good point. Both seem sensible, assuming exit and abort are considered "returning". I'd argue they unwind the stack like an exception (that is potentially not captured) so we should be good.

[fhahn]

llvm/lib/Transforms/IPO/FunctionAttrs.cpp

1436

Sounds good to me, I'll update the code to not drop it; I don't think there's a verifier for this. The only thing is that at first glance it might be surprising to have a function both willreturn and noreturn

[fhahn]

Address comments, also add NumWillReturn stats counter and Function::willReturn()/Function::setWillReturn.

[nikic]

LGTM as well.

llvm/test/Transforms/FunctionAttrs/willreturn.ll

4–5

illreturn :)

[fhahn]

Thanks for taking a look!

llvm/test/Transforms/FunctionAttrs/willreturn.ll

4–5

thanks, uploaded too soon, fixed in committed version.

llvm/lib/Transforms/IPO/FunctionAttrs.cpp

if (none_of(*F, basicBlockCanReturn)) {
       F->setDoesNotReturn();
       Changed = true;
     }
   }
 
   return Changed;
 }
 
+// Set the willreturn function attribute if possible.
+static bool addWillReturn(const SCCNodeSet &SCCNodes) {
+  bool Changed = false;
+
+  for (Function *F : SCCNodes) {
+    if (!F || !F->onlyReadsMemory() || !F->mustProgress())
+      continue;
+
+    F->addFnAttr(Attribute::WillReturn);
+    F->removeFnAttr(Attribute::NoReturn);
+  }
+
+  return Changed;
+}
+
 static SCCNodesResult createSCCNodeSet(ArrayRef<Function *> Functions) {
   SCCNodesResult Res;
   Res.HasUnknownCall = false;
   for (Function *F : Functions) {
     if (!F || F->hasOptNone() || F->hasFnAttribute(Attribute::Naked)) {
       // Treat any function we're trying not to optimize as if it were an
       // indirect call and omit it from the node set used below.
       Res.HasUnknownCall = true;
static bool deriveAttrsInPostOrder(ArrayRef<Function *> Functions,
   if (Nodes.SCCNodes.empty())
     return Changed;
 
   Changed |= addArgumentReturnedAttrs(Nodes.SCCNodes);
   Changed |= addReadAttrs(Nodes.SCCNodes, AARGetter);
   Changed |= addArgumentAttrs(Nodes.SCCNodes);
   Changed |= inferConvergent(Nodes.SCCNodes);
   Changed |= addNoReturnAttrs(Nodes.SCCNodes);
+  Changed |= addWillReturn(Nodes.SCCNodes);
 
   // If we have no external nodes participating in the SCC, we can deduce some
   // more precise attributes as well.
   if (!Nodes.HasUnknownCall) {
     Changed |= addNoAliasAttrs(Nodes.SCCNodes);
     Changed |= addNonNullAttrs(Nodes.SCCNodes);
     Changed |= inferAttrsFromFunctionBodies(Nodes.SCCNodes);
     Changed |= addNoRecurseAttrs(Nodes.SCCNodes);

llvm/test/Transforms/FunctionAttrs/willreturn.ll

 ; RUN: opt -function-attrs -S %s | FileCheck %s
 
-; TODO
 define void @mustprogress_readnone() mustprogress {
-; CHECK-NOT: Function Attrs: {{.*}} willreturn
-; CHECK:     define void @mustprogress_readnone()
+; CHECK:      Function Attrs: {{.*}} readnone willreturn
+; CHECK-NEXT: define void @mustprogress_readnone()
 ;
 entry:
   br label %while.body
 
 while.body:
   br label %while.body
 }
 
-; TODO
 define i32 @mustprogress_load(i32* %ptr) mustprogress {
-; CHECK-NOT: Function Attrs: {{.*}} willreturn
-; CHECK:     define i32 @mustprogress_load(
+; CHECK:      Function Attrs: {{.*}} readonly willreturn
+; CHECK-NEXT: define i32 @mustprogress_load(
 ;
 entry:
   %r = load i32, i32* %ptr
   ret i32 %r
 }
 
 define void @mustprogress_store(i32* %ptr) mustprogress {
 ; CHECK-NOT: Function Attrs: {{.*}} willreturn
 ; CHECK: define void @mustprogress_store(
 ;
 entry:
   store i32 0, i32* %ptr
   ret void
 }
 
 declare void @unknown_fn()
 
 define void @mustprogress_call_unknown_fn() mustprogress {
 ; CHECK-NOT: Function Attrs: {{.*}} willreturn
-; CHECK: define void @mustprogress_call_unknown_fn(
+; CHECK:     define void @mustprogress_call_unknown_fn(
 ;
   call void @unknown_fn()
   ret void
 }
 
-; TODO
 define i32 @mustprogress_call_known_functions(i32* %ptr) mustprogress {
-; CHECK-NOT: Function Attrs: {{.*}} willreturn
-; CHECK:     define i32 @mustprogress_call_known_functions(
+; CHECK:      Function Attrs: {{.*}} readonly willreturn
+; CHECK-NEXT: define i32 @mustprogress_call_known_functions(
 ;
   call void @mustprogress_readnone()
   %r = call i32 @mustprogress_load(i32* %ptr)
   ret i32 %r
 }
 
 declare i32 @__gxx_personality_v0(...)
 
-; TODO
 define i64 @mustprogress_mayunwind() mustprogress personality i8* bitcast (i32 (...)* @__gxx_personality_v0 to i8*) {
-; CHECK-NOT: Function Attrs: {{.*}} willreturn
-; CHECK:     define i64 @mustprogress_mayunwind(
+; CHECK:      Function Attrs: {{.*}} readnone willreturn
+; CHECK-NEXT: define i64 @mustprogress_mayunwind(
 ;
   %a = invoke i64 @fn_noread()
           to label %A unwind label %B
 A:
   ret i64 10
 
 B:
   %val = landingpad { i8*, i32 }
            catch i8* null
   ret i64 0
 }
 
 declare i64 @fn_noread() readnone