On Wed, Jan 15, 2025 at 10:05:35AM +0100, Richard Biener wrote: > > When we have return somefn (whatever); where somefn is normally tail > > callable and IPA-VRP determines somefn returns a singleton range, VRP > > just changes the IL to > > somefn (whatever); > > return 42; > > (or whatever the value in that range is). The introduction of IPA-VRP > > return value tracking then effectively regresses the tail call optimization. > > This is even more important if the call is [[gnu::musttail]]. > > > > So, the following patch queries IPA-VRP whether a function returns singleton > > range and if so and the value returned is identical to that, marks the > > call as [tail call] anyway. If expansion decides it can't use the tail > > call, we'll still expand the return 42; or similar statement, and if it > > decides it can use the tail call, that part will be ignored and we'll emit > > normal tail call. > > Interesting idea. I'd have said that for [[gnu::musttail]] we want to > disable the IPA transform instead?
The patch isn't just about [[gnu::musttail]], that is mentioned primarily because it is then an error rather than just missed optimization. The patch fixes a regression for the non-musttail cases, where before GCC 14 we used to tail call and we no longer do. The maybe_error_musttail call in there does nothing if call isn't musttail (except print a note into the dump file). > But I can see that when the user > wrote > > somefn (whatever); > return 42; > > the handling would enable tail-calling, but your patch only handles > it in the [[musttail]] failure path. No, the patch handles that too. > Anyway, I think we want to guard IPA transforms on [[gnu::musttail]] > return value which should be more robust? The question is what. I think not computing the return range for that case is a bad idea, we want to propagate it to the caller. One thing I've considered was extending the range in one of the directions (which?) so that it isn't singleton, but there could be harm caused if we pick the wrong one. So maybe just guard the spot which replaces the use of SSA_NAME with singleton value in the return stmt with the constant if the SSA_NAME_DEF_STMT is a musttail call? I guess that can be done if there aren't too many spots which perform such replacement (and I'm afraid there are, but haven't searched for them). For non-musttail we shouldn't do that though, we don't know whether it is tail-callable at all and whether having the constant in there is more beneficial for optimizations or possibly keeping it as maybe tail callable. And therefore I wrote the patch as is, it then fixes the 14/15 regression for it and as a benefit can even handle the cases where user wrote somefn (whatever); return 42; rather than return somefn (whatever); I'm willing to at least try the punting on singleton replacements for gnu::musttail though. Jakub