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

[Johannes Doerfert via Phabricator via llvm-commits]

jdoerfert added a comment.

In D94633#2498642 <https://reviews.llvm.org/D94633#2498642>, @nikic wrote:

> 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.

Fair. As I said, we can enable it in a restricted, lightweight mode first. Could you remind me which bugs?

>> 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
>   }

So, I run this through the Attributor: https://opt.godbolt.org/z/TqsK86
Not what we were looking for, so here is the non-trivial version: https://opt.godbolt.org/z/rbeedM

> 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...

I don't disagree. My concern is that we should add more sophisitcated the logic in the Attributor instead. We had some initial patches wrt. finite loops, but I don't think they went in.


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

https://reviews.llvm.org/D94633