Re: [rs6000] Streamline compare-and-swap success return value computation

2011-11-29 Thread David Edelsohn
On Mon, Nov 28, 2011 at 7:33 PM, Richard Henderson r...@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


Re: [rs6000] Streamline compare-and-swap success return value computation

2011-11-29 Thread Richard Henderson
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~


Re: [rs6000] Streamline compare-and-swap success return value computation

2011-11-29 Thread David Edelsohn
On Tue, Nov 29, 2011 at 10:50 AM, Richard Henderson r...@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


[rs6000] Streamline compare-and-swap success return value computation

2011-11-28 Thread Richard Henderson
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.


r~
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.  */


Re: [rs6000] Streamline compare-and-swap success return value computation

2011-11-28 Thread Alan Modra
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.

-- 
Alan Modra
Australia Development Lab, IBM