| Message ID | 4ED42876.8030704@redhat.com |
|---|---|
| State | New |
| Headers | show |
On Mon, Nov 28, 2011 at 04:33:58PM -0800, Richard Henderson wrote: > On 11/28/2011 04:26 PM, Richard Henderson wrote: > > On 11/28/2011 03:05 PM, Richard Henderson wrote: > >> On 11/28/2011 02:16 PM, Alan Modra wrote: > >>> Hmm, I suppose you could argue that powerpc and others ought to not > >>> generate those three extra instructions when using the return value. > >>> I'll see about fixing powerpc. > >> > >> However, we can do better by considering the value to be stored in CR0... > > > > Try this and see if it generates the sort of code you want. Untested. > > ... actually, this one. There's no reason to differentiate between strong > and weak compare-and-swap when computing boolval. Looks very similar to what I wrote, except > + else if (reg_overlap_mentioned_p (retval, oldval)) > + oldval = copy_to_reg (oldval); I missed this bit > - x = gen_rtx_NE (VOIDmode, x, oldval); > - x = rs6000_generate_compare (x, mode); > + cond = gen_reg_rtx (CCmode); > + x = gen_rtx_COMPARE (CCmode, x, oldval); > + emit_insn (gen_rtx_SET (VOIDmode, cond, x)); > + > + x = gen_rtx_NE (VOIDmode, cond, const0_rtx); and instead pulled cond out of the return from rs6000_generate_compare. Results look good.
On Mon, Nov 28, 2011 at 7:33 PM, Richard Henderson <rth@redhat.com> wrote: > On 11/28/2011 04:26 PM, Richard Henderson wrote: >> On 11/28/2011 03:05 PM, Richard Henderson wrote: >>> On 11/28/2011 02:16 PM, Alan Modra wrote: >>>> Hmm, I suppose you could argue that powerpc and others ought to not >>>> generate those three extra instructions when using the return value. >>>> I'll see about fixing powerpc. >>> >>> However, we can do better by considering the value to be stored in CR0... >> >> Try this and see if it generates the sort of code you want. Untested. > > ... actually, this one. There's no reason to differentiate between strong > and weak compare-and-swap when computing boolval. Has anyone bootstrapped and regression-tested the patch? Thanks, David
On 11/29/2011 07:13 AM, David Edelsohn wrote: >> ... actually, this one. There's no reason to differentiate between strong >> and weak compare-and-swap when computing boolval. > > Has anyone bootstrapped and regression-tested the patch? Yes, I did so last night on gcc110. r~
On Tue, Nov 29, 2011 at 10:50 AM, Richard Henderson <rth@redhat.com> wrote: > On 11/29/2011 07:13 AM, David Edelsohn wrote: >>> ... actually, this one. There's no reason to differentiate between strong >>> and weak compare-and-swap when computing boolval. >> >> Has anyone bootstrapped and regression-tested the patch? > > Yes, I did so last night on gcc110. Great. Then the patch is okay with me, if you have not committed it already. Thanks! David
diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c index f01353b..5a33f91 100644 --- a/gcc/config/rs6000/rs6000.c +++ b/gcc/config/rs6000/rs6000.c @@ -17352,11 +17352,11 @@ rs6000_expand_atomic_compare_and_swap (rtx operands[]) retval = gen_reg_rtx (SImode); mode = SImode; } + else if (reg_overlap_mentioned_p (retval, oldval)) + oldval = copy_to_reg (oldval); rs6000_pre_atomic_barrier (mod_s); - emit_move_insn (boolval, const0_rtx); - label1 = NULL_RTX; if (!is_weak) { @@ -17374,28 +17374,23 @@ rs6000_expand_atomic_compare_and_swap (rtx operands[]) NULL_RTX, 1, OPTAB_LIB_WIDEN); } - x = gen_rtx_NE (VOIDmode, x, oldval); - x = rs6000_generate_compare (x, mode); + cond = gen_reg_rtx (CCmode); + x = gen_rtx_COMPARE (CCmode, x, oldval); + emit_insn (gen_rtx_SET (VOIDmode, cond, x)); + + x = gen_rtx_NE (VOIDmode, cond, const0_rtx); emit_unlikely_jump (x, label2); x = newval; if (mask) x = rs6000_mask_atomic_subword (retval, newval, mask); - cond = gen_reg_rtx (CCmode); emit_store_conditional (mode, cond, mem, x); - if (is_weak) - { - /* ??? It's either this or an unlikely jump over (set bool 1). */ - x = gen_rtx_EQ (SImode, cond, const0_rtx); - emit_insn (gen_rtx_SET (VOIDmode, boolval, x)); - } - else + if (!is_weak) { x = gen_rtx_NE (VOIDmode, cond, const0_rtx); emit_unlikely_jump (x, label1); - emit_move_insn (boolval, const1_rtx); } if (mod_f != MEMMODEL_RELAXED) @@ -17408,6 +17403,10 @@ rs6000_expand_atomic_compare_and_swap (rtx operands[]) if (shift) rs6000_finish_atomic_subword (operands[1], retval, shift); + + /* In all cases, CR0 contains EQ on success, and NE on failure. */ + x = gen_rtx_EQ (SImode, cond, const0_rtx); + emit_insn (gen_rtx_SET (VOIDmode, boolval, x)); } /* Expand an atomic exchange operation. */