On 02/20/2014 12:09 PM, Jakub Jelinek wrote: > 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.
I don't know any target that actually accepts a MEM for oldval, and since the current definition of __atomic_compare_and_swap_n takes an address for expected, we'll always have a MEM. So at present we'll always allocate a new pseudo just as if we zero out oldval. But, fair enough. It does seem generally safer your way. r~
