This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.

Index Nav:	[Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav:	[Date Prev] [Date Next]	[Thread Prev] [Thread Next]
Other format:	[Raw text]

Re: [Patch] MIPS: Improve HI and QI atomic memory ops.

From: David Daney <ddaney at avtrex dot com>
To: gcc-patches <gcc-patches at gcc dot gnu dot org>, rsandifo at nildram dot co dot uk
Date: Mon, 05 May 2008 14:03:39 -0700
Subject: Re: [Patch] MIPS: Improve HI and QI atomic memory ops.
References: <481DEEDC.9010804@avtrex.com> <874p9df8qv.fsf@firetop.home>

Richard Sandiford wrote:
> Thanks for doing this.  The patch is OK with one minor niggle...
>
> David Daney <ddaney@avtrex.com> writes:
>   
>> Index: config/mips/mips.c
>> ===================================================================
>> --- config/mips/mips.c	(revision 134846)
>> +++ config/mips/mips.c	(working copy)
>> @@ -5917,12 +5917,26 @@ mips_expand_compare_and_swap_12 (rtx res
>>  			  gen_rtx_NOT (SImode, mask)));
>>  
>>    /* Shift the old value into place.  */
>> -  oldvalsi = force_reg (SImode, gen_lowpart (SImode, oldval));
>> -  old_shifted = mips_force_binary (SImode, ASHIFT, oldvalsi, shiftsi);
>> +  if (CONST_INT_P (oldval) && INTVAL(oldval) == 0)
>> +    old_shifted = const0_rtx;
>>     
>
> Just check "oldval == const0_rtx" here.
>   
How about this version instead?

In addition to doing "oldval == const0_rtx", It seems that we only need
to mask the values if they are negative constants.  So I made the
masking operation conditional.

Currently bootstrapping/testing on mipsel-linux.

OK to commit this version?
2008-05-05  David Daney  <ddaney@avtrex.com>

    * lib/target-supports.exp (check_effective_target_sync_int_long): Add
    mips*-*-*.
    (check_effective_target_sync_char_short): Same.


2008-05-05  David Daney  <ddaney@avtrex.com>

    * config/mips/mips.md (mips_expand_compare_and_swap_12): Handle
    special case of constant zero operands.
    * config/mips/mips.c (mips_expand_compare_and_swap_12): Mask old and
    new values.  Special case constand zero values.
    * config/mips/mips.h (MIPS_COMPARE_AND_SWAP): Skip 'sync' if compare
    fails.
    (MIPS_COMPARE_AND_SWAP_12): Handle constant zero operands.
    (MIPS_COMPARE_AND_SWAP_12_0): New macro.

Index: testsuite/lib/target-supports.exp
===================================================================
--- testsuite/lib/target-supports.exp	(revision 134846)
+++ testsuite/lib/target-supports.exp	(working copy)
@@ -2052,7 +2052,8 @@ proc check_effective_target_sync_int_lon
 	     || [istarget s390*-*-*] 
 	     || [istarget powerpc*-*-*]
 	     || [istarget sparc64-*-*]
-	     || [istarget sparcv9-*-*] } {
+	     || [istarget sparcv9-*-*]
+	     || [istarget mips*-*-*] } {
            set et_sync_int_long_saved 1
         }
     }
@@ -2079,7 +2080,8 @@ proc check_effective_target_sync_char_sh
 	     || [istarget s390*-*-*] 
 	     || [istarget powerpc*-*-*]
 	     || [istarget sparc64-*-*]
-	     || [istarget sparcv9-*-*] } {
+	     || [istarget sparcv9-*-*]
+	     || [istarget mips*-*-*] } {
            set et_sync_char_short_saved 1
         }
     }
Index: config/mips/mips.md
===================================================================
--- config/mips/mips.md	(revision 134846)
+++ config/mips/mips.md	(working copy)
@@ -4462,19 +4462,22 @@ (define_expand "sync_compare_and_swap<mo
 
 ;; Helper insn for mips_expand_compare_and_swap_12.
 (define_insn "compare_and_swap_12"
-  [(set (match_operand:SI 0 "register_operand" "=&d")
-	(match_operand:SI 1 "memory_operand" "+R"))
+  [(set (match_operand:SI 0 "register_operand" "=&d,&d")
+	(match_operand:SI 1 "memory_operand" "+R,R"))
    (set (match_dup 1)
-	(unspec_volatile:SI [(match_operand:SI 2 "register_operand" "d")
-			     (match_operand:SI 3 "register_operand" "d")
-			     (match_operand:SI 4 "register_operand" "d")
-			     (match_operand:SI 5 "register_operand" "d")]
+	(unspec_volatile:SI [(match_operand:SI 2 "register_operand" "d,d")
+			     (match_operand:SI 3 "register_operand" "d,d")
+			     (match_operand:SI 4 "reg_or_0_operand" "dJ,dJ")
+			     (match_operand:SI 5 "reg_or_0_operand" "d,J")]
 			    UNSPEC_COMPARE_AND_SWAP_12))]
   "GENERATE_LL_SC"
 {
-  return MIPS_COMPARE_AND_SWAP_12;
+  if (which_alternative == 0)
+    return MIPS_COMPARE_AND_SWAP_12;
+  else
+    return MIPS_COMPARE_AND_SWAP_12_0;
 }
-  [(set_attr "length" "40")])
+  [(set_attr "length" "40,36")])
 
 (define_insn "sync_add<mode>"
   [(set (match_operand:GPR 0 "memory_operand" "+R,R")
Index: config/mips/mips.c
===================================================================
--- config/mips/mips.c	(revision 134846)
+++ config/mips/mips.c	(working copy)
@@ -5917,12 +5917,30 @@ mips_expand_compare_and_swap_12 (rtx res
 			  gen_rtx_NOT (SImode, mask)));
 
   /* Shift the old value into place.  */
-  oldvalsi = force_reg (SImode, gen_lowpart (SImode, oldval));
-  old_shifted = mips_force_binary (SImode, ASHIFT, oldvalsi, shiftsi);
+  if (oldval == const0_rtx)
+    old_shifted = const0_rtx;
+  else
+    {
+      oldvalsi = force_reg (SImode, gen_lowpart (SImode, oldval));
+      old_shifted = mips_force_binary (SImode, ASHIFT, oldvalsi, shiftsi);
+      /* Clear any sign extension bits in constants.  Non-constant
+         RTXs are already masked.  */
+      if (CONST_INT_P (oldval) && INTVAL (oldval) < 0)
+	mips_emit_binary (AND, old_shifted, old_shifted, mask);
+    }
 
   /* Do the same for the new value.  */
-  newvalsi = force_reg (SImode, gen_lowpart (SImode, newval));
-  new_shifted = mips_force_binary (SImode, ASHIFT, newvalsi, shiftsi);
+  if (newval == const0_rtx)
+    new_shifted = const0_rtx;
+  else
+    {
+      newvalsi = force_reg (SImode, gen_lowpart (SImode, newval));
+      new_shifted = mips_force_binary (SImode, ASHIFT, newvalsi, shiftsi);
+      /* Clear any sign extension bits in constants.  Non-constant
+         RTXs are already masked.  */
+      if (CONST_INT_P (newval) && INTVAL (newval) < 0)
+	mips_emit_binary (AND, new_shifted, new_shifted, mask);
+    }
 
   /* Do the SImode atomic access.  */
   res = gen_reg_rtx (SImode);
Index: config/mips/mips.h
===================================================================
--- config/mips/mips.h	(revision 134846)
+++ config/mips/mips.h	(working copy)
@@ -2902,7 +2902,8 @@ while (0)
   "\tsc" SUFFIX "\t%@,%1\n"			\
   "\tbeq\t%@,%.,1b\n"				\
   "\tnop\n"					\
-  "2:\tsync%-%]%>%)"
+  "\tsync%-%]%>%)\n"				\
+  "2:\n"
 
 /* Return an asm string that atomically:
 
@@ -2919,7 +2920,7 @@ while (0)
   "%(%<%[%|sync\n"				\
   "1:\tll\t%0,%1\n"				\
   "\tand\t%@,%0,%2\n"				\
-  "\tbne\t%@,%4,2f\n"				\
+  "\tbne\t%@,%z4,2f\n"				\
   "\tand\t%@,%0,%3\n"				\
   "\tor\t%@,%@,%5\n"				\
   "\tsc\t%@,%1\n"				\
@@ -2928,6 +2929,20 @@ while (0)
   "\tsync%-%]%>%)\n"				\
   "2:\n"
 
+/* Like MIPS_COMPARE_AND_SWAP_12, except %5 is a constant zero,
+   so the OR can be omitted.  */
+#define MIPS_COMPARE_AND_SWAP_12_0		\
+  "%(%<%[%|sync\n"				\
+  "1:\tll\t%0,%1\n"				\
+  "\tand\t%@,%0,%2\n"				\
+  "\tbne\t%@,%z4,2f\n"				\
+  "\tand\t%@,%0,%3\n"				\
+  "\tsc\t%@,%1\n"				\
+  "\tbeq\t%@,%.,1b\n"				\
+  "\tnop\n"					\
+  "\tsync%-%]%>%)\n"				\
+  "2:\n"
+
 /* Return an asm string that atomically:
 
      - Sets memory reference %0 to %0 INSN %1.

Follow-Ups:
- Re: [Patch] MIPS: Improve HI and QI atomic memory ops.
  - From: Richard Sandiford

References:
- [Patch] MIPS: Improve HI and QI atomic memory ops.
  - From: David Daney
- Re: [Patch] MIPS: Improve HI and QI atomic memory ops.
  - From: Richard Sandiford

Index Nav:	[Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav:	[Date Prev] [Date Next]	[Thread Prev] [Thread Next]