[PATCH] D94633: [FunctionAttrs] Infer willreturn for functions without loops

[Nikita Popov via Phabricator via llvm-commits]

nikic added a comment.

In D94633#2498420 <https://reviews.llvm.org/D94633#2498420>, @jdoerfert wrote:

> I still think we should check to enable a light-attributor run in the beginning of the pipeline.

I was hoping to get the willreturn-related bugs fixed in time for LLVM 12, so sticking to the existing infrastructure seems like the safer bet :) It probably makes more sense to enable the attributor after branching rather than before.

> The logic seems sound, could be improved since this does initialize the fixpoint computation with a pessimistic starting value. One comment below, the rest is OK for merging.

I thought about this, but I don't think that we can use an optimistic fixpoint here without some much more sophisticated analysis. Just naively assuming "willreturn until proven otherwise" will incorrectly mark infinite recursion as willreturn:

  ; Not willreturn.
  define void @willreturn_recursion() {
    tail call void @willreturn_recursion()
    ret void
  }

Without mustprogress, I believe that function is well-defined (or at least with `musttail` it's well-defined for sure). Inferring willreturn for a non-trivial SCC (without mustprogress) would require proving that the recursion is finite. And as the current code doesn't even handle finite loops, this is a bit out of scope...



================
Comment at: lib/Transforms/IPO/FunctionAttrs.cpp:1443
+  if (!Backedges.empty())
+    return false;
+
----------------
jdoerfert wrote:
> I never used this function. Does it handle irreducible control flow properly? Maybe we want a test just to be sure.
I believe it will just pick some possible choice of backedges for the irreducible case, which is good enough for this purpose. I've added a test.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D94633/new/

https://reviews.llvm.org/D94633