[PATCH] PR target/41331 -- fix powerpc bswap64 with -m32 -mpowerpc64

Michael Meissner meissner@linux.vnet.ibm.com
Fri Sep 11 22:35:00 GMT 2009


On Thu, Sep 10, 2009 at 07:25:32PM -0400, David Edelsohn wrote:
> On Thu, Sep 10, 2009 at 4:59 PM, Michael Meissner
> <meissner@linux.vnet.ibm.com> wrote:
> > On Thu, Sep 10, 2009 at 02:00:27PM -0400, David Edelsohn wrote:
> >> On Thu, Sep 10, 2009 at 12:39 PM, Michael Meissner
> >> <meissner@linux.vnet.ibm.com> wrote:
> >> > This patch should fix the problems that I introduced with my changes on July
> >> > 31st in __builtin_bswap64 if -m32 and -mpowerpc64 are used.
> >>
> >> Why should rs6000_emit_add() use the MODE of OP0 instead of
> >> TARGET_32BIT?  gen_rtx_PLUS uses Pmode, which is based on
> >> TARGET_32BIT.  Using TARGET_32BIT seems like a self-test that one does
> >> not pass an operand of the wrong MODE for an address.
> >
> > In looking at it further, I see optabs.c provides a gen_add3_insn function which
> > does the same thing that rs6000_emit_add did.
> 
> Yes, I just stumbled on the same function when I saw a patch from
> Jakub.  Neither Segher nor I had remembered that function although I
> thought something like it should exist.  I agree that you should
> revise the patch to use that function.

Here is the revised patch for 41331.  It does the usual bootstrap and make
check with no regressions.  While it wasn't necessary for the PR, I did clean
up the cases in rs6000.c that had the if TARGET_32BIT code to use gen_add3_insn
as well.

Is this ok to apply?

-- 
Michael Meissner, IBM
4 Technology Place Drive, MS 2203A, Westford, MA, 01886, USA
meissner@linux.vnet.ibm.com
-------------- next part --------------
[gcc]
2009-09-11  Michael Meissner  <meissner@linux.vnet.ibm.com>

	PR target/41331
	* config/rs6000/rs6000.c (rs6000_emit_move): Use gen_add3_insn
	instead of explicit addsi3/adddi3 calls.
	(rs6000_split_multireg_move): Ditto.
	(rs6000_emit_allocate_stack): Ditto.
	(rs6000_emit_prologue): Ditto.
	(rs6000_output_mi_thunk): Ditto.

	* config/rs6000/rs6000.md (bswapdi*): Don't assume the pointer
	size is 64 bits if we can use 64-bit registers.

[gcc/testsuite]
2009-09-10  Michael Meissner  <meissner@linux.vnet.ibm.com>

	PR target/41331
	* gcc.target/powerpc/bswap64-4.c: New file to test bswap64 on a
	-m32 -mpowerpc64 system.

Index: gcc/config/rs6000/rs6000.c
===================================================================
--- gcc/config/rs6000/rs6000.c	(revision 151575)
+++ gcc/config/rs6000/rs6000.c	(working copy)
@@ -6488,10 +6488,7 @@ rs6000_emit_move (rtx dest, rtx source, 
 	      rtx other = XEXP (XEXP (operands[1], 0), 1);
 
 	      sym = force_reg (mode, sym);
-	      if (mode == SImode)
-		emit_insn (gen_addsi3 (operands[0], sym, other));
-	      else
-		emit_insn (gen_adddi3 (operands[0], sym, other));
+	      emit_insn (gen_add3_insn (operands[0], sym, other));
 	      return;
 	    }
 
@@ -16473,9 +16470,7 @@ rs6000_split_multireg_move (rtx dst, rtx
 	      delta_rtx = (GET_CODE (XEXP (src, 0)) == PRE_INC
 			   ? GEN_INT (GET_MODE_SIZE (GET_MODE (src)))
 			   : GEN_INT (-GET_MODE_SIZE (GET_MODE (src))));
-	      emit_insn (TARGET_32BIT
-			 ? gen_addsi3 (breg, breg, delta_rtx)
-			 : gen_adddi3 (breg, breg, delta_rtx));
+	      emit_insn (gen_add3_insn (breg, breg, delta_rtx));
 	      src = replace_equiv_address (src, breg);
 	    }
 	  else if (! rs6000_offsettable_memref_p (src))
@@ -16525,9 +16520,7 @@ rs6000_split_multireg_move (rtx dst, rtx
 		  used_update = true;
 		}
 	      else
-		emit_insn (TARGET_32BIT
-			   ? gen_addsi3 (breg, breg, delta_rtx)
-			   : gen_adddi3 (breg, breg, delta_rtx));
+		emit_insn (gen_add3_insn (breg, breg, delta_rtx));
 	      dst = replace_equiv_address (dst, breg);
 	    }
 	  else
@@ -17728,14 +17721,7 @@ rs6000_emit_allocate_stack (HOST_WIDE_IN
 	  && REGNO (stack_limit_rtx) > 1
 	  && REGNO (stack_limit_rtx) <= 31)
 	{
-	  emit_insn (TARGET_32BIT
-		     ? gen_addsi3 (tmp_reg,
-				   stack_limit_rtx,
-				   GEN_INT (size))
-		     : gen_adddi3 (tmp_reg,
-				   stack_limit_rtx,
-				   GEN_INT (size)));
-
+	  emit_insn (gen_add3_insn (tmp_reg, stack_limit_rtx, GEN_INT (size)));
 	  emit_insn (gen_cond_trap (LTU, stack_reg, tmp_reg,
 				    const0_rtx));
 	}
@@ -18648,9 +18634,7 @@ rs6000_emit_prologue (void)
           rtx ptr_reg = (sp_reg_rtx == frame_reg_rtx
                          ? sp_reg_rtx : r11);
 
-          emit_insn (TARGET_32BIT
-                     ? gen_addsi3 (r11, ptr_reg, offset)
-                     : gen_adddi3 (r11, ptr_reg, offset));
+	  emit_insn (gen_add3_insn (r11, ptr_reg, offset));
         }
 
       par = rs6000_make_savres_rtx (info, frame_reg_rtx,
@@ -20090,12 +20074,7 @@ rs6000_output_mi_thunk (FILE *file, tree
 
   /* Apply the constant offset, if required.  */
   if (delta)
-    {
-      rtx delta_rtx = GEN_INT (delta);
-      emit_insn (TARGET_32BIT
-		 ? gen_addsi3 (this_rtx, this_rtx, delta_rtx)
-		 : gen_adddi3 (this_rtx, this_rtx, delta_rtx));
-    }
+    emit_insn (gen_add3_insn (this_rtx, this_rtx, GEN_INT (delta)));
 
   /* Apply the offset from the vtable, if required.  */
   if (vcall_offset)
@@ -20106,9 +20085,7 @@ rs6000_output_mi_thunk (FILE *file, tree
       emit_move_insn (tmp, gen_rtx_MEM (Pmode, this_rtx));
       if (((unsigned HOST_WIDE_INT) vcall_offset) + 0x8000 >= 0x10000)
 	{
-	  emit_insn (TARGET_32BIT
-		     ? gen_addsi3 (tmp, tmp, vcall_offset_rtx)
-		     : gen_adddi3 (tmp, tmp, vcall_offset_rtx));
+	  emit_insn (gen_add3_insn (tmp, tmp, vcall_offset_rtx));
 	  emit_move_insn (tmp, gen_rtx_MEM (Pmode, tmp));
 	}
       else
@@ -20117,9 +20094,7 @@ rs6000_output_mi_thunk (FILE *file, tree
 
 	  emit_move_insn (tmp, gen_rtx_MEM (Pmode, loc));
 	}
-      emit_insn (TARGET_32BIT
-		 ? gen_addsi3 (this_rtx, this_rtx, tmp)
-		 : gen_adddi3 (this_rtx, this_rtx, tmp));
+      emit_insn (gen_add3_insn (this_rtx, this_rtx, tmp));
     }
 
   /* Generate a tail call to the target function.  */
Index: gcc/config/rs6000/rs6000.md
===================================================================
--- gcc/config/rs6000/rs6000.md	(revision 151575)
+++ gcc/config/rs6000/rs6000.md	(working copy)
@@ -2392,9 +2392,11 @@ (define_expand "bswapdi2"
   if (!REG_P (operands[0]) && !REG_P (operands[1]))
     operands[1] = force_reg (DImode, operands[1]);
 
-  if (TARGET_32BIT)
+  if (!TARGET_POWERPC64)
     {
-      /* 32-bit needs fewer scratch registers.  */
+      /* 32-bit mode needs fewer scratch registers, but 32-bit addressing mode
+	 that uses 64-bit registers needs the same scratch registers as 64-bit
+	 mode.  */
       emit_insn (gen_bswapdi2_32bit (operands[0], operands[1]));
       DONE;
     }
@@ -2453,13 +2455,13 @@ (define_split
   addr1 = XEXP (src, 0);
   if (GET_CODE (addr1) == PLUS)
     {
-      emit_insn (gen_adddi3 (op2, XEXP (addr1, 0), GEN_INT (4)));
-      addr2 = gen_rtx_PLUS (DImode, op2, XEXP (addr1, 1));
+      emit_insn (gen_add3_insn (op2, XEXP (addr1, 0), GEN_INT (4)));
+      addr2 = gen_rtx_PLUS (Pmode, op2, XEXP (addr1, 1));
     }
   else
     {
       emit_move_insn (op2, GEN_INT (4));
-      addr2 = gen_rtx_PLUS (DImode, op2, addr1);
+      addr2 = gen_rtx_PLUS (Pmode, op2, addr1);
     }
 
   if (BYTES_BIG_ENDIAN)
@@ -2503,13 +2505,13 @@ (define_split
   addr1 = XEXP (dest, 0);
   if (GET_CODE (addr1) == PLUS)
     {
-      emit_insn (gen_adddi3 (op2, XEXP (addr1, 0), GEN_INT (4)));
-      addr2 = gen_rtx_PLUS (DImode, op2, XEXP (addr1, 1));
+      emit_insn (gen_add3_insn (op2, XEXP (addr1, 0), GEN_INT (4)));
+      addr2 = gen_rtx_PLUS (Pmode, op2, XEXP (addr1, 1));
     }
   else
     {
       emit_move_insn (op2, GEN_INT (4));
-      addr2 = gen_rtx_PLUS (DImode, op2, addr1);
+      addr2 = gen_rtx_PLUS (Pmode, op2, addr1);
     }
 
   emit_insn (gen_lshrdi3 (op3, src, GEN_INT (32)));
@@ -2559,7 +2561,7 @@ (define_insn "bswapdi2_32bit"
   [(set (match_operand:DI 0 "reg_or_mem_operand" "=&r,Z,??&r")
 	(bswap:DI (match_operand:DI 1 "reg_or_mem_operand" "Z,r,r")))
    (clobber (match_scratch:SI 2 "=&b,&b,X"))]
-  "TARGET_32BIT && (REG_P (operands[0]) || REG_P (operands[1]))"
+  "!TARGET_POWERPC64 && (REG_P (operands[0]) || REG_P (operands[1]))"
   "#"
   [(set_attr "length" "16,12,36")])
 
@@ -2567,7 +2569,7 @@ (define_split
   [(set (match_operand:DI 0 "gpc_reg_operand" "")
 	(bswap:DI (match_operand:DI 1 "indexed_or_indirect_operand" "")))
    (clobber (match_operand:SI 2 "gpc_reg_operand" ""))]
-  "TARGET_32BIT && reload_completed"
+  "!TARGET_POWERPC64 && reload_completed"
   [(const_int 0)]
   "
 {
@@ -2584,7 +2586,7 @@ (define_split
   addr1 = XEXP (src, 0);
   if (GET_CODE (addr1) == PLUS)
     {
-      emit_insn (gen_addsi3 (op2, XEXP (addr1, 0), GEN_INT (4)));
+      emit_insn (gen_add3_insn (op2, XEXP (addr1, 0), GEN_INT (4)));
       addr2 = gen_rtx_PLUS (SImode, op2, XEXP (addr1, 1));
     }
   else
@@ -2612,7 +2614,7 @@ (define_split
   [(set (match_operand:DI 0 "indexed_or_indirect_operand" "")
 	(bswap:DI (match_operand:DI 1 "gpc_reg_operand" "")))
    (clobber (match_operand:SI 2 "gpc_reg_operand" ""))]
-  "TARGET_32BIT && reload_completed"
+  "!TARGET_POWERPC64 && reload_completed"
   [(const_int 0)]
   "
 {
@@ -2629,7 +2631,7 @@ (define_split
   addr1 = XEXP (dest, 0);
   if (GET_CODE (addr1) == PLUS)
     {
-      emit_insn (gen_addsi3 (op2, XEXP (addr1, 0), GEN_INT (4)));
+      emit_insn (gen_add3_insn (op2, XEXP (addr1, 0), GEN_INT (4)));
       addr2 = gen_rtx_PLUS (SImode, op2, XEXP (addr1, 1));
     }
   else
@@ -2657,7 +2659,7 @@ (define_split
   [(set (match_operand:DI 0 "gpc_reg_operand" "")
 	(bswap:DI (match_operand:DI 1 "gpc_reg_operand" "")))
    (clobber (match_operand:SI 2 "" ""))]
-  "TARGET_32BIT && reload_completed"
+  "!TARGET_POWERPC64 && reload_completed"
   [(const_int 0)]
   "
 {
Index: gcc/testsuite/gcc.target/powerpc/bswap64-4.c
===================================================================
--- gcc/testsuite/gcc.target/powerpc/bswap64-4.c	(revision 0)
+++ gcc/testsuite/gcc.target/powerpc/bswap64-4.c	(revision 0)
@@ -0,0 +1,9 @@
+/* { dg-do compile { target { powerpc*-*-* } } } */
+/* { dg-options "-O2 -mpowerpc64" } */
+/* { dg-require-effective-target ilp32 } */
+/* { dg-final { scan-assembler-times "lwbrx" 2 } } */
+/* { dg-final { scan-assembler-times "stwbrx" 2 } } */
+
+long long swap_load (long long *a) { return __builtin_bswap64 (*a); }
+long long swap_reg (long long a) { return __builtin_bswap64 (a); }
+void swap_store (long long *a, long long b) { *a = __builtin_bswap64 (b); }


More information about the Gcc-patches mailing list