[PATCH] D94125: [Attributor] Derive `willreturn` based on `mustprogress`

[Florian Hahn via Phabricator via llvm-commits]

fhahn added a comment.

The logic in general seems fine to me, but I'm not too familiar with the attributor internals,  so it would be good for someone more familiar to take a look.

BTW, I put up a similar patch for `-function-attrs` D94502 <https://reviews.llvm.org/D94502>



================
Comment at: llvm/lib/Transforms/IPO/AttributorAttributes.cpp:2298
+
+    if ((!getAnchorScope() || !getAnchorScope()->doesNotAccessMemory()) &&
+        (!getAssociatedFunction() ||
----------------
jdoerfert wrote:
> fhahn wrote:
> > Do we have to also ensure that the function is `nounwind`? In case of an exception, we would return to the closest exception handler and not return to a point in the existing call stack.
> No, they are by design orthogonal. Willreturn "allows" to "come back through unwind. Basically, you will go back and unwind through the call of the function, that means you "return" to the call. So `willreturn` does not imply `nounwind` and `willreturn` does not guarantee there is no exception either.
> Here is the lang ref:
> 
> 
> > This function attribute indicates that a call of this function will either exhibit undefined behavior or comes back and continues execution at a point in the existing call stack that includes the current invocation. Annotated functions may still raise an exception, i.a., nounwind is not implied. If an invocation of an annotated function does not return control back to a point in the call stack, the behavior is undefined.
> 
> 
Ah right, that makes sense. Not sure, but IMO the current wording could be a bit improved, but I am not really sure how.


================
Comment at: llvm/lib/Transforms/IPO/AttributorAttributes.cpp:2300
+        (!getAssociatedFunction() ||
+         !getAssociatedFunction()->doesNotAccessMemory())) {
+
----------------
jdoerfert wrote:
> I think I can even remove this condition.
I assume the `MemAA.*ReadOnly` helpers also return true if the function does not access memory? Then it would probably be simpler to just drop the check.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94125