On Thu, Feb 20, 2014 at 11:49:30AM -0600, Richard Henderson wrote:
> Tested on x86_64 and i686, and manually inspecting the generated code.
> Any ideas how to regression test this?
No idea about how to test this.
> @@ -5330,14 +5330,23 @@ expand_builtin_atomic_compare_exchange (enum
> machine_mode mode, tree exp,
> if (tree_fits_shwi_p (weak) && tree_to_shwi (weak) != 0)
> is_weak = true;
>
> + if (target == const0_rtx)
> + target = NULL;
> oldval = expect;
> - if (!expand_atomic_compare_and_swap ((target == const0_rtx ? NULL :
> &target),
> - &oldval, mem, oldval, desired,
> +
> + if (!expand_atomic_compare_and_swap (&target, &oldval, mem, oldval,
> desired,
I'm wondering if this shouldn't be instead:
oldval = NULL;
if (!expand_atomic_compare_and_swap (&target, &oldval, mem, expected, desired,
is_weak, success, failure))
because otherwise expand_atomic_compare_and_swap could in theory already
store into expect MEM, couldn't it? I mean, it does:
/* Load expected into a register for the compare and swap. */
if (MEM_P (expected))
expected = copy_to_reg (expected);
/* Make sure we always have some place to put the return oldval.
Further, make sure that place is distinct from the input expected,
just in case we need that path down below. */
if (ptarget_oval == NULL
|| (target_oval = *ptarget_oval) == NULL
|| reg_overlap_mentioned_p (expected, target_oval))
target_oval = gen_reg_rtx (mode);
so with NULL *ptarget_oval it will surely allocate a pseudo, but if it is
the expected MEM, as expected has been forced into register earlier,
I don't think it overlaps with that REG and thus it can be already stored
and have oldval == expect after the call.
Jakub
