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]

[PATCH] Don't truncate paradoxical subregs in store_bit_field_1


My patch here:

  http://gcc.gnu.org/ml/gcc-patches/2008-12/msg00766.html

was basically wrong.  We create a truncation of a paradoxical subreg,
i.e. note the second insn from the link:

  (set (reg:SI 193)
       (truncate:SI (subreg:DI (reg:SI 193) 0)))

With more aggressive optization of truncate (I am working on a patch to handle
TRUNCATE in force_to_mode) the above is turned into a simple no-op copy, which
I think is a valid optimization.

I don't think we can safely set a paradoxical subreg between modes that
require truncation so my patch below modifies the code to use a temporary.
For an SI-mode insertion this what's generated after the patch:

  (insn 7 6 8 3 ins-2.c:24 (set (reg:DI 197)
          (subreg:DI (reg/v:SI 195 [ s ]) 0)) -1 (nil))
  
  (insn 8 7 9 3 ins-2.c:24 (set (zero_extract:DI (reg:DI 197)
              (const_int 3 [0x3])
              (const_int 29 [0x1d]))
          (reg/v:DI 196 [ a+-4 ])) -1 (nil))
  
  (insn 9 8 10 3 ins-2.c:24 (set (reg/v:SI 195 [ s ])
          (truncate:SI (reg:DI 197))) -1 (nil))
  
If 197 and 195 can be assigned to the same hardreg we get:

  (insn 8 6 9 2 ins-2.c:24 (set (zero_extract:DI (reg:DI 2 $2 [orig:197 s+-4 ] [197])
              (const_int 3 [0x3])
              (const_int 29 [0x1d]))
          (reg/v:DI 4 $4 [orig:196 a+-4 ] [196])) 239 {insvdi} (nil))
  
  (insn 9 8 10 2 ins-2.c:24 (set (reg/v:SI 2 $2 [orig:195 s ] [195])
          (truncate:SI (reg:DI 2 $2 [orig:197 s+-4 ] [197]))) 167 {truncdisi2} (nil))

which is the same code as before.

There is of course also the alternative approach of making the bit-inseration
expander machine-mode specific.  Benchmarking reveals that the patch does not
affect performance in a negative way so I didn't do that.  My hope is that
with further middle-end tweaks I will be able to use an SI-mode insertion at
the end -- we'll see.

Bootstrapped and regstested on mips64octeon-linux and x86_64-linux.  Also
regtested on mipsis64r2-elf.

The testcase is the same as in the original patch.  Also if the truncation is
optimized away incorrectly there are many existing tests that fail with both
mipsisa64r2-elf and mips64octeon-linux.

OK for trunk?

Adam


	Revert:
	2009-01-11  Adam Nemet  <anemet@caviumnetworks.com>
	* expmed.c (store_bit_field_1): Properly truncate the paradoxical
	subreg of op0 to the original op0.

	* expmed.c (store_bit_field_1): Use a temporary as the destination
	instead of a paradoxical subreg when we need to truncate the result.

Index: gcc/expmed.c
===================================================================
--- gcc.orig/expmed.c	2009-06-23 10:53:50.000000000 -0700
+++ gcc/expmed.c	2009-06-23 11:05:47.000000000 -0700
@@ -685,6 +685,7 @@ store_bit_field_1 (rtx str_rtx, unsigned
       rtx xop0 = op0;
       rtx last = get_last_insn ();
       rtx pat;
+      bool copy_back = false;
 
       /* Add OFFSET into OP0's address.  */
       if (MEM_P (xop0))
@@ -699,6 +700,23 @@ store_bit_field_1 (rtx str_rtx, unsigned
       if (REG_P (xop0) && GET_MODE (xop0) != op_mode)
 	xop0 = gen_rtx_SUBREG (op_mode, xop0, 0);
 
+      /* If the destination is a paradoxical subreg such that we need a
+	 truncate to the inner mode, perform the insertion on a temporary and
+	 truncate the result to the original destination.  Note that we can't
+	 just truncate the paradoxical subreg as (truncate:N (subreg:W (reg:N
+	 X) 0)) is (reg:N X).  */
+      if (GET_CODE (xop0) == SUBREG
+	  && REG_P (SUBREG_REG (xop0))
+	  && (!TRULY_NOOP_TRUNCATION
+	      (GET_MODE_BITSIZE (GET_MODE (SUBREG_REG (xop0))),
+	       GET_MODE_BITSIZE (op_mode))))
+	{
+	  rtx tem = gen_reg_rtx (op_mode);
+	  emit_move_insn (tem, xop0);
+	  xop0 = tem;
+	  copy_back = true;
+	}
+
       /* On big-endian machines, we count bits from the most significant.
 	 If the bit field insn does not, we must invert.  */
 
@@ -758,15 +776,8 @@ store_bit_field_1 (rtx str_rtx, unsigned
 	{
 	  emit_insn (pat);
 
-	  /* If the mode of the insertion is wider than the mode of the
-	     target register we created a paradoxical subreg for the
-	     target.  Truncate the paradoxical subreg of the target to
-	     itself properly.  */
-	  if (!TRULY_NOOP_TRUNCATION (GET_MODE_BITSIZE (GET_MODE (op0)),
-				      GET_MODE_BITSIZE (op_mode))
-	      && (REG_P (xop0)
-		  || GET_CODE (xop0) == SUBREG))
-	      convert_move (op0, xop0, true);
+	  if (copy_back)
+	    convert_move (op0, xop0, true);
 	  return true;
 	}
       delete_insns_since (last);


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