[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

1434

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

1437

Changed = true; ;)

[fhahn]

llvm/lib/Transforms/IPO/FunctionAttrs.cpp

1437

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

1437

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

1437

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

1437

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/include/llvm/IR/Function.h

public:
 
   /// Determine if the function is required to make forward progress.
   bool mustProgress() const {
     return hasFnAttribute(Attribute::MustProgress) ||
            hasFnAttribute(Attribute::WillReturn);
   }
   void setMustProgress() { addFnAttr(Attribute::MustProgress); }
 
+  /// Determine if the function will return.
+  bool willReturn() const { return hasFnAttribute(Attribute::WillReturn); }
+  void setWillReturn() { addFnAttr(Attribute::WillReturn); }
+
   /// True if the ABI mandates (or the user requested) that this
   /// function be in a unwind table.
   bool hasUWTable() const {
     return hasFnAttribute(Attribute::UWTable);
   }
   void setHasUWTable() {
     addFnAttr(Attribute::UWTable);
   }

llvm/lib/Transforms/IPO/FunctionAttrs.cpp

 STATISTIC(NumReturned, "Number of arguments marked returned");
 STATISTIC(NumReadNoneArg, "Number of arguments marked readnone");
 STATISTIC(NumReadOnlyArg, "Number of arguments marked readonly");
 STATISTIC(NumNoAlias, "Number of function returns marked noalias");
 STATISTIC(NumNonNullReturn, "Number of function returns marked nonnull");
 STATISTIC(NumNoRecurse, "Number of functions marked as norecurse");
 STATISTIC(NumNoUnwind, "Number of functions marked as nounwind");
 STATISTIC(NumNoFree, "Number of functions marked as nofree");
+STATISTIC(NumWillReturn, "Number of functions marked as willreturn");
 
 static cl::opt<bool> EnableNonnullArgPropagation(
     "enable-nonnull-arg-prop", cl::init(true), cl::Hidden,
     cl::desc("Try to propagate nonnull argument attributes from callsites to "
              "caller functions."));
 
 static cl::opt<bool> DisableNoUnwindInference(
     "disable-nounwind-inference", cl::Hidden,
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() || F->willReturn())
+      continue;
+
+    F->setWillReturn();
+    NumWillReturn++;
+    Changed = true;
+  }
+
+  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: {{.*}} noreturn {{.*}} 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