On 06/23/2017 11:44 AM, Segher Boessenkool wrote: > On Thu, Jun 22, 2017 at 10:59:05PM -0600, Jeff Law wrote: >> On 05/28/2017 06:31 AM, Segher Boessenkool wrote: >>> __atomic_add_fetch adds a value to some memory, and returns the result. >>> If there is no direct support for this, expand_builtin_atomic_fetch_op >>> is asked to implement this as __atomic_fetch_add (which returns the >>> original value of the mem), followed by the addition. Now, the >>> __atomic_add_fetch could have been a tail call, but we shouldn't >>> perform the __atomic_fetch_add as a tail call: following code would >>> not be executed, and in fact thrown away because there is a barrier >>> after tail calls. >>> >>> This fixes it. > >>> PR middle-end/80902 >>> * builtins.c (expand_builtin_atomic_fetch_op): If emitting code after >>> a call, force the call to not be a tail call. >> Hmmm. I wonder if we have similar problems elsewhere. For example >> expand_builtin_int_roundingfn_2, stack_protect_epilogue, >> expand_builtin_trap (though this one probably isn't broken in practice), >> expand_ifn_atomic_compare_exchange_into_call. >> >> OK, but please check the other instances where we call expand_call, then >> continue generating code afterwards. Fixing those can be a follow-up patch. > > I guess we want an expand_call_notail helper? Probably.
Or, hrm, why are function > calls expanded as tail calls at all, should that not be decided later? That's how I thought it worked. We create two streams of insns, then decide later which of the two streams to use. But I think part of the criteria for creating streams was that call was in the tail position to start with. And that's not the case with the code you pointed out and the others I found. Jeff
