This is the mail archive of the
mailing list for the GCC project.
Re: [PATCH] Fix expand_builtin_atomic_fetch_op for pre-op (PR80902)
- From: Jeff Law <law at redhat dot com>
- To: Segher Boessenkool <segher at kernel dot crashing dot org>
- Cc: gcc-patches at gcc dot gnu dot org
- Date: Fri, 23 Jun 2017 11:48:00 -0600
- Subject: Re: [PATCH] Fix expand_builtin_atomic_fetch_op for pre-op (PR80902)
- Authentication-results: sourceware.org; auth=none
- Authentication-results: ext-mx10.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com
- Authentication-results: ext-mx10.extmail.prod.ext.phx2.redhat.com; spf=pass smtp.mailfrom=law at redhat dot com
- Dkim-filter: OpenDKIM Filter v2.11.0 mx1.redhat.com 9E06214286C
- Dmarc-filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 9E06214286C
- References: <email@example.com> <firstname.lastname@example.org> <20170623174419.GO16550@gate.crashing.org>
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),
>> 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?
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.