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]

bit extract adjust_address versus scheduler


I got a bug report about perl, as in Spec95, not running correctly on
mips n32.  It turned out that it was the perl parser that was
miscompiled, because the scheduler would defer a store to the second
32-bit value in a 32-bit-aligned struct past a 64-bit unaligned load
from the same word.

The alias sets were correct, but the gotcha was that the size of the
unaligned load was marked as 1, so when the scheduler tested for
dependencies, it decided there weren't any.

At first, I thought the problem was that MIPS had QImode for the
memory operands, even though it actually expects word modes (kind of;
that's true of ! TARGET_64BIT, but TARGET_64BIT accepts both 32- and
64-bit values).  This would force extract_bit_field and
store_bit_field to convert the memory to QImode, limiting its size to
1, and then, when the mips-specific patterns adjusted the MEMs to
BLKmode, they'd inherit the smaller size.

It turned out that, even after fixing the MIPS-specific code, the
problem was still there, because the generic code insisted in
converting MEMs to byte_mode.  Even though I could still fix this in
MIPS-specific code, setting the MEM_SIZE to the right value (and I
actually wrote and tested a patch that does this), I figured it would
be better to fix the generic problem, even though it might momentarily
break other platforms that support unaligned loads from memory and
require QImode in their patterns; it should be trivial for each
port maintainer to figure it out and adjust the port accordingly, I
suppose.

Anyway, this is the patch I'm testing ATM.  Does this look like the
right fix?  Ok to install, if tests pass?

Index: gcc/ChangeLog
from  Alexandre Oliva  <aoliva@redhat.com>

	* config/mips/mips.md (extv, extzv, insv): Don't specify mode of
	memory operand.
	* expmed.c (store_bit_field): Adjust op0's address to BLKmode.
	(extract_bit_field): Likewise.

Index: gcc/expmed.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/expmed.c,v
retrieving revision 1.133
diff -u -p -r1.133 expmed.c
--- gcc/expmed.c 21 Apr 2003 21:32:00 -0000 1.133
+++ gcc/expmed.c 1 May 2003 09:38:23 -0000
@@ -607,9 +607,10 @@ store_bit_field (str_rtx, bitsize, bitnu
 	}
       volatile_ok = save_volatile_ok;
 
-      /* Add OFFSET into OP0's address.  */
+      /* Add OFFSET into OP0's address.  Use BLKmode instead of
+	 byte_mode such that MEM_SIZE is preserved.  */
       if (GET_CODE (xop0) == MEM)
-	xop0 = adjust_address (xop0, byte_mode, offset);
+	xop0 = adjust_address (xop0, BLKmode, offset);
 
       /* If xop0 is a register, we need it in MAXMODE
 	 to make it acceptable to the format of insv.  */
@@ -1319,8 +1320,9 @@ extract_bit_field (str_rtx, bitsize, bit
 		  /* XBITPOS counts within UNIT, which is what is expected.  */
 		}
 	      else
-		/* Get ref to first byte containing part of the field.  */
-		xop0 = adjust_address (xop0, byte_mode, xoffset);
+		/* Get ref to first byte containing part of the field.
+		   Preserve MEM_SIZE by using BLKmode.  */
+		xop0 = adjust_address (xop0, BLKmode, xoffset);
 
 	      volatile_ok = save_volatile_ok;
 	    }
@@ -1449,8 +1451,9 @@ extract_bit_field (str_rtx, bitsize, bit
 		  /* XBITPOS counts within UNIT, which is what is expected.  */
 		}
 	      else
-		/* Get ref to first byte containing part of the field.  */
-		xop0 = adjust_address (xop0, byte_mode, xoffset);
+		/* Get ref to first byte containing part of the field.
+		   Preserve MEM_SIZE by using BLKmode.  */
+		xop0 = adjust_address (xop0, BLKmode, xoffset);
 	    }
 
 	  /* If op0 is a register, we need it in MAXMODE (which is usually
Index: gcc/config/mips/mips.md
===================================================================
RCS file: /cvs/gcc/gcc/gcc/config/mips/mips.md,v
retrieving revision 1.164
diff -u -p -r1.164 mips.md
--- gcc/config/mips/mips.md 1 May 2003 02:33:13 -0000 1.164
+++ gcc/config/mips/mips.md 1 May 2003 09:38:26 -0000
@@ -4895,7 +4895,7 @@ move\\t%0,%z4\\n\\
 
 (define_expand "extv"
   [(set (match_operand 0 "register_operand" "")
-	(sign_extract (match_operand:QI 1 "memory_operand" "")
+	(sign_extract (match_operand 1 "memory_operand" "")
 		      (match_operand 2 "immediate_operand" "")
 		      (match_operand 3 "immediate_operand" "")))]
   "!TARGET_MIPS16"
@@ -4910,7 +4910,7 @@ move\\t%0,%z4\\n\\
 
 (define_expand "extzv"
   [(set (match_operand 0 "register_operand" "")
-	(zero_extract (match_operand:QI 1 "memory_operand" "")
+	(zero_extract (match_operand 1 "memory_operand" "")
 		      (match_operand 2 "immediate_operand" "")
 		      (match_operand 3 "immediate_operand" "")))]
   "!TARGET_MIPS16"
@@ -4924,7 +4924,7 @@ move\\t%0,%z4\\n\\
   })
 
 (define_expand "insv"
-  [(set (zero_extract (match_operand:QI 0 "memory_operand" "")
+  [(set (zero_extract (match_operand 0 "memory_operand" "")
 		      (match_operand 1 "immediate_operand" "")
 		      (match_operand 2 "immediate_operand" ""))
 	(match_operand 3 "reg_or_0_operand" ""))]
-- 
Alexandre Oliva   Enjoy Guarana', see http://www.ic.unicamp.br/~oliva/
Red Hat GCC Developer                 aoliva@{redhat.com, gcc.gnu.org}
CS PhD student at IC-Unicamp        oliva@{lsd.ic.unicamp.br, gnu.org}
Free Software Evangelist                Professional serial bug killer

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