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: Combine known_cond() may create subreg of const_int


On Feb 15, 2002, Richard Henderson <rth@redhat.com> wrote:

>> Not yet.  I'll make the simplification you've suggested and start
>> trying alpha and sparc bootstraps, as well as some crosses (ppc,
>> mn10300, sh and sh64, most likely).

> Thanks.

Ok, so I needed an additional change for this to work on alpha.  The
problem was that we were zero_extending a CONST_INT to TImode, but
simplify_unary_operation() had what seemed to be a typo that caused it
to reject modes wider than 2*HOST_BITS_PER_INT, instead of
2*HOST_BITS_PER_WIDE_INT, which is the maximum width of a
CONST_DOUBLE.  So I went ahead and made this change, and then all of
the natives and crosses above build successfully, except sparc, that
required further changes.  The sparc port had numerous occurrences of
code that either assumed HOST_WIDE_INT to be 32-bits wide or assumed
that it was ok for CONST_INTs to not be valid sign-extensions for the
modes.  I put in some trunc_int_for_mode() here and there and got
sparc-unknown-linux-gnu and sparc-sun-solaris2.7 to bootstrap, and
built sparc-elf and sparc64-elf successfully (after fixing a minor bug
in libgloss).  Bootstrap hasn't completed on sparcv9-sun-solaris2.7
because I ran short on disk space on the nearest Solaris7 box, but I
could give it another push if you think it's worth it.  Anyway, here
are the patches.  Tested on all of the above, and also bootstrapped on
athlon-pc-linux-gnu.  Ok to install?

(Oh, I realize the patch for sparc also removes the inclusion of
sys/mman.h.  This is important because sol2.h, for some reason, is not
used only on Solaris, but also on sparc-elf and probably other sparc
targets that don't have this header.  I've verified that everything
builds just fine without the include, except for warnings when
compiling TRANSFER_FROM_TRAMPOLINE.)

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

	* combine.c (do_SUBST): Sanity check substitutions of
	CONST_INTs, and reject them in SUBREGs and ZERO_EXTENDs.
	(subst): Simplify SUBREG or ZERO_EXTEND instead of SUBSTing a
	CONST_INT into its operand.
	(known_cond): Likewise, for ZERO_EXTEND.
	* simplify-rtx.c (simplify_unary_operation): Fix condition to
	allow for simplification of wide modes.  Reject CONST_INTs in
	ZERO_EXTEND when their actual mode is not given.

Index: gcc/combine.c
===================================================================
RCS file: /cvs/gcc/egcs/gcc/combine.c,v
retrieving revision 1.265
diff -u -p -r1.265 combine.c
--- gcc/combine.c 2002/02/14 19:30:42 1.265
+++ gcc/combine.c 2002/02/15 03:00:03
@@ -424,6 +424,33 @@ do_SUBST (into, newval)
   if (oldval == newval)
     return;
 
+  /* We'd like to catch as many invalid transformations here as
+     possible.  Unfortunately, there are way too many mode changes
+     that are perfectly valid, so we'd waste too much effort for
+     little gain doing the checks here.  Focus on catching invalid
+     transformations involving integer constants.  */
+  if (GET_MODE_CLASS (GET_MODE (oldval)) == MODE_INT
+      && GET_CODE (newval) == CONST_INT)
+    {
+      /* Sanity check that we're replacing oldval with a CONST_INT
+	 that is a valid sign-extension for the original mode.  */
+      if (INTVAL (newval) != trunc_int_for_mode (INTVAL (newval),
+						 GET_MODE (oldval)))
+	abort ();
+
+      /* Replacing the operand of a SUBREG or a ZERO_EXTEND with a
+	 CONST_INT is not valid, because after the replacement, the
+	 original mode would be gone.  Unfortunately, we can't tell
+	 when do_SUBST is called to replace the operand thereof, so we
+	 perform this test on oldval instead, checking whether an
+	 invalid replacement took place before we got here.  */
+      if ((GET_CODE (oldval) == SUBREG
+	   && GET_CODE (SUBREG_REG (oldval)) == CONST_INT)
+	  || (GET_CODE (oldval) == ZERO_EXTEND
+	      && GET_CODE (XEXP (oldval, 0)) == CONST_INT))
+	abort ();
+     }
+
   if (undobuf.frees)
     buf = undobuf.frees, undobuf.frees = buf->next;
   else
@@ -3505,7 +3534,24 @@ subst (x, from, to, in_dest, unique_copy
 	      if (GET_CODE (new) == CLOBBER && XEXP (new, 0) == const0_rtx)
 		return new;
 
-	      SUBST (XEXP (x, i), new);
+	      if (GET_CODE (new) == CONST_INT && GET_CODE (x) == SUBREG)
+		{
+		  x = simplify_subreg (GET_MODE (x), new,
+				       GET_MODE (SUBREG_REG (x)),
+				       SUBREG_BYTE (x));
+		  if (! x)
+		    abort ();
+		}
+	      else if (GET_CODE (new) == CONST_INT
+		       && GET_CODE (x) == ZERO_EXTEND)
+		{
+		  x = simplify_unary_operation (ZERO_EXTEND, GET_MODE (x),
+						new, GET_MODE (XEXP (x, 0)));
+		  if (! x)
+		    abort ();
+		}
+	      else
+		SUBST (XEXP (x, i), new);
 	    }
 	}
     }
@@ -7445,6 +7491,31 @@ known_cond (x, cond, reg, val)
 	    return new;
 	  else
 	    SUBST (SUBREG_REG (x), r);
+	}
+
+      return x;
+    }
+  /* We don't have to handle SIGN_EXTEND here, because even in the
+     case of replacing something with a modeless CONST_INT, a
+     CONST_INT is already (supposed to be) a valid sign extension for
+     its narrower mode, which implies it's already properly
+     sign-extended for the wider mode.  Now, for ZERO_EXTEND, the
+     story is different.  */
+  else if (code == ZERO_EXTEND)
+    {
+      enum machine_mode inner_mode = GET_MODE (XEXP (x, 0));
+      rtx new, r = known_cond (XEXP (x, 0), cond, reg, val);
+
+      if (XEXP (x, 0) != r)
+	{
+	  /* We must simplify the zero_extend here, before we lose
+             track of the original inner_mode.  */
+	  new = simplify_unary_operation (ZERO_EXTEND, GET_MODE (x),
+					  r, inner_mode);
+	  if (new)
+	    return new;
+	  else
+	    SUBST (XEXP (x, 0), r);
 	}
 
       return x;
Index: gcc/simplify-rtx.c
===================================================================
RCS file: /cvs/gcc/egcs/gcc/simplify-rtx.c,v
retrieving revision 1.92
diff -u -p -r1.92 simplify-rtx.c
--- gcc/simplify-rtx.c 2002/01/12 07:38:47 1.92
+++ gcc/simplify-rtx.c 2002/02/17 10:05:35
@@ -528,8 +528,10 @@ simplify_unary_operation (code, mode, op
 	  break;
 
 	case ZERO_EXTEND:
+	  /* When zero-extending a CONST_INT, we need to know its
+             original mode.  */
 	  if (op_mode == VOIDmode)
-	    op_mode = mode;
+	    abort ();
 	  if (GET_MODE_BITSIZE (op_mode) == HOST_BITS_PER_WIDE_INT)
 	    {
 	      /* If we were really extending the mode,
@@ -587,7 +589,8 @@ simplify_unary_operation (code, mode, op
 
   /* We can do some operations on integer CONST_DOUBLEs.  Also allow
      for a DImode operation on a CONST_INT.  */
-  else if (GET_MODE (trueop) == VOIDmode && width <= HOST_BITS_PER_INT * 2
+  else if (GET_MODE (trueop) == VOIDmode
+	   && width <= HOST_BITS_PER_WIDE_INT * 2
 	   && (GET_CODE (trueop) == CONST_DOUBLE
 	       || GET_CODE (trueop) == CONST_INT))
     {
@@ -631,8 +634,10 @@ simplify_unary_operation (code, mode, op
 	  break;
 
 	case ZERO_EXTEND:
-	  if (op_mode == VOIDmode
-	      || GET_MODE_BITSIZE (op_mode) > HOST_BITS_PER_WIDE_INT)
+	  if (op_mode == VOIDmode)
+	    abort ();
+
+	  if (GET_MODE_BITSIZE (op_mode) > HOST_BITS_PER_WIDE_INT)
 	    return 0;
 
 	  hv = 0;
Index: gcc/ChangeLog
from  Alexandre Oliva  <aoliva@redhat.com>

	* config/sparc/sol2.h: Don't include sys/mman.h.
	* config/sparc/sparc.c (arith_operand): Use SMALL_INT32.
	(arith_4096_operand): Don't throw high bits away.
	(const64_operand): Take sign extension of CONST_INTs into account.
	(const64_high_operand, sparc_emit_set_const32): Likewise.
	(GEN_HIGHINT64): Likewise.
	(sparc_emit_set_const64_quick1): Likewise.
	(const64_is_2insns): Likewise.
	(print_operand): Use trunc_int_for_mode for sign extension.
	* config/sparc/sparc.h (SMALL_INT32): Likewise.
	(SPARC_SETHI_P): Simplify.
	* config/sparc/sparc.md (movqi): Sign-extend CONST_DOUBLE
	chars.  Assume CONST_INT is already properly sign-extended.
	(movdi split): Sign-extend each SImode part.
	(andsi3 split): Don't mask high bits off, so that result
	remains properly sign-extend.
	(iorsi3 split): Likewise.
	(xorsi3 split): Likewise.

Index: gcc/config/sparc/sol2.h
===================================================================
RCS file: /cvs/gcc/egcs/gcc/config/sparc/sol2.h,v
retrieving revision 1.35
diff -u -p -r1.35 sol2.h
--- gcc/config/sparc/sol2.h 2002/02/04 18:16:06 1.35
+++ gcc/config/sparc/sol2.h 2002/02/20 17:47:10
@@ -243,7 +243,7 @@ Boston, MA 02111-1307, USA.  */
 
 /* This declares mprotect (used in TRANSFER_FROM_TRAMPOLINE) for
    libgcc2.c.  */
-#ifdef L_trampoline
+#if 0 /* def L_trampoline */
 #include <sys/mman.h>
 #endif
 
Index: gcc/config/sparc/sparc.c
===================================================================
RCS file: /cvs/gcc/egcs/gcc/config/sparc/sparc.c,v
retrieving revision 1.182
diff -u -p -r1.182 sparc.c
--- gcc/config/sparc/sparc.c 2002/02/07 18:48:13 1.182
+++ gcc/config/sparc/sparc.c 2002/02/20 17:47:15
@@ -950,13 +950,11 @@ arith_operand (op, mode)
      rtx op;
      enum machine_mode mode;
 {
-  int val;
   if (register_operand (op, mode))
     return 1;
   if (GET_CODE (op) != CONST_INT)
     return 0;
-  val = INTVAL (op) & 0xffffffff;
-  return SPARC_SIMM13_P (val);
+  return SMALL_INT32 (op);
 }
 
 /* Return true if OP is a constant 4096  */
@@ -969,7 +967,6 @@ arith_4096_operand (op, mode)
   int val;
   if (GET_CODE (op) != CONST_INT)
     return 0;
-  val = INTVAL (op) & 0xffffffff;
   return val == 4096;
 }
 
@@ -998,7 +995,7 @@ const64_operand (op, mode)
 	      && SPARC_SIMM13_P (CONST_DOUBLE_LOW (op))
 	      && (CONST_DOUBLE_HIGH (op) ==
 		  ((CONST_DOUBLE_LOW (op) & 0x80000000) != 0 ?
-		   (HOST_WIDE_INT)0xffffffff : 0)))
+		   (HOST_WIDE_INT)-1 : 0)))
 #endif
 	  );
 }
@@ -1010,7 +1007,7 @@ const64_high_operand (op, mode)
      enum machine_mode mode ATTRIBUTE_UNUSED;
 {
   return ((GET_CODE (op) == CONST_INT
-	   && (INTVAL (op) & 0xfffffc00) != 0
+	   && (INTVAL (op) & ~(HOST_WIDE_INT)0x3ff) != 0
 	   && SPARC_SETHI_P (INTVAL (op))
 #if HOST_BITS_PER_WIDE_INT != 64
 	   /* Must be positive on non-64bit host else the
@@ -1021,7 +1018,7 @@ const64_high_operand (op, mode)
 	   )
 	  || (GET_CODE (op) == CONST_DOUBLE
 	      && CONST_DOUBLE_HIGH (op) == 0
-	      && (CONST_DOUBLE_LOW (op) & 0xfffffc00) != 0
+	      && (CONST_DOUBLE_LOW (op) & ~(HOST_WIDE_INT)0x3ff) != 0
 	      && SPARC_SETHI_P (CONST_DOUBLE_LOW (op))));
 }
 
@@ -1335,11 +1332,13 @@ sparc_emit_set_const32 (op0, op1)
 	  && (INTVAL (op1) & 0x80000000) != 0)
 	emit_insn (gen_rtx_SET
 		   (VOIDmode, temp,
-		    gen_rtx_CONST_DOUBLE (VOIDmode, INTVAL (op1) & 0xfffffc00,
+		    gen_rtx_CONST_DOUBLE (VOIDmode,
+					  INTVAL (op1) & ~(HOST_WIDE_INT)0x3ff,
 					  0)));
       else
 	emit_insn (gen_rtx_SET (VOIDmode, temp,
-				GEN_INT (INTVAL (op1) & 0xfffffc00)));
+				GEN_INT (INTVAL (op1)
+					 & ~(HOST_WIDE_INT)0x3ff)));
 
       emit_insn (gen_rtx_SET (VOIDmode,
 			      op0,
@@ -1509,15 +1508,15 @@ static rtx gen_safe_OR64 PARAMS ((rtx, H
 static rtx gen_safe_XOR64 PARAMS ((rtx, HOST_WIDE_INT));
 
 #if HOST_BITS_PER_WIDE_INT == 64
-#define GEN_HIGHINT64(__x)		GEN_INT ((__x) & 0xfffffc00)
+#define GEN_HIGHINT64(__x)		GEN_INT ((__x) & ~(HOST_WIDE_INT)0x3ff)
 #define GEN_INT64(__x)			GEN_INT (__x)
 #else
 #define GEN_HIGHINT64(__x) \
-	gen_rtx_CONST_DOUBLE (VOIDmode, (__x) & 0xfffffc00, 0)
+	gen_rtx_CONST_DOUBLE (VOIDmode, (__x) & ~(HOST_WIDE_INT)0x3ff, 0)
 #define GEN_INT64(__x) \
 	gen_rtx_CONST_DOUBLE (VOIDmode, (__x) & 0xffffffff, \
 			      ((__x) & 0x80000000 \
-			       ? 0xffffffff : 0))
+			       ? -1 : 0))
 #endif
 
 /* The optimizer is not to assume anything about exactly
@@ -1602,7 +1601,8 @@ sparc_emit_set_const64_quick1 (op0, temp
 	{
 	  emit_insn (gen_rtx_SET (VOIDmode, op0,
 				  gen_safe_XOR64 (temp,
-						  (-0x400 | (low_bits & 0x3ff)))));
+						  (-(HOST_WIDE_INT)0x400
+						   | (low_bits & 0x3ff)))));
 	}
     }
 }
@@ -1835,7 +1835,7 @@ const64_is_2insns (high_bits, low_bits)
   int highest_bit_set, lowest_bit_set, all_bits_between_are_set;
 
   if (high_bits == 0
-      || high_bits == 0xffffffff)
+      || high_bits == -1)
     return 1;
 
   analyze_64bit_constant (high_bits, low_bits,
@@ -6000,9 +6000,7 @@ print_operand (file, x, code)
     case 'b':
       {
 	/* Print a sign-extended character.  */
-	int i = INTVAL (x) & 0xff;
-	if (i & 0x80)
-	  i |= 0xffffff00;
+	int i = trunc_int_for_mode (INTVAL (x), QImode);
 	fprintf (file, "%d", i);
 	return;
       }
Index: gcc/config/sparc/sparc.h
===================================================================
RCS file: /cvs/gcc/egcs/gcc/config/sparc/sparc.h,v
retrieving revision 1.159
diff -u -p -r1.159 sparc.h
--- gcc/config/sparc/sparc.h 2002/02/06 06:58:31 1.159
+++ gcc/config/sparc/sparc.h 2002/02/20 17:47:18
@@ -1402,10 +1402,9 @@ extern const char leaf_reg_remap[];
    SMALL_INT is used throughout the port so we continue to use it.  */
 #define SMALL_INT(X) (SPARC_SIMM13_P (INTVAL (X)))
 /* 13 bit immediate, considering only the low 32 bits */
-#define SMALL_INT32(X) (SPARC_SIMM13_P ((int)INTVAL (X) & 0xffffffff))
-#define SPARC_SETHI_P(X) \
-(((unsigned HOST_WIDE_INT) (X) & \
-  (TARGET_ARCH64 ? ~(unsigned HOST_WIDE_INT) 0xfffffc00 : 0x3ff)) == 0)
+#define SMALL_INT32(X) (SPARC_SIMM13_P (trunc_int_for_mode \
+					(INTVAL (X), SImode)))
+#define SPARC_SETHI_P(X) (((unsigned HOST_WIDE_INT) (X) & 0x3ff) == 0)
 
 #define CONST_OK_FOR_LETTER_P(VALUE, C)  \
   ((C) == 'I' ? SPARC_SIMM13_P (VALUE)			\
Index: gcc/config/sparc/sparc.md
===================================================================
RCS file: /cvs/gcc/egcs/gcc/config/sparc/sparc.md,v
retrieving revision 1.146
diff -u -p -r1.146 sparc.md
--- gcc/config/sparc/sparc.md 2002/02/20 01:21:06 1.146
+++ gcc/config/sparc/sparc.md 2002/02/20 17:47:21
@@ -2053,15 +2053,9 @@
      a double if needed.  */
   if (GET_CODE (operands[1]) == CONST_DOUBLE)
     {
-      operands[1] = GEN_INT (CONST_DOUBLE_LOW (operands[1]) & 0xff);
+      operands[1] = GEN_INT (trunc_int_for_mode
+			     (CONST_DOUBLE_LOW (operands[1]), QImode));
     }
-  else if (GET_CODE (operands[1]) == CONST_INT)
-    {
-      /* And further, we know for all QI cases that only the
-	 low byte is significant, which we can always process
-	 in a single insn.  So mask it now.  */
-      operands[1] = GEN_INT (INTVAL (operands[1]) & 0xff);
-    }
 
   /* Handle sets of MEM first.  */
   if (GET_CODE (operands[0]) == MEM)
@@ -2769,8 +2763,8 @@
 #else
   unsigned int low, high;
 
-  low = INTVAL (operands[1]) & 0xffffffff;
-  high = (INTVAL (operands[1]) >> 32) & 0xffffffff;
+  low = trunc_int_for_mode (INTVAL (operands[1]), SImode);
+  high = trunc_int_for_mode (INTVAL (operands[1]) >> 32, SImode);
   emit_insn (gen_movsi (gen_highpart (SImode, operands[0]), GEN_INT (high)));
 
   /* Slick... but this trick loses if this subreg constant part
@@ -6574,7 +6568,7 @@
    (set (match_dup 0) (and:SI (not:SI (match_dup 3)) (match_dup 1)))]
   "
 {
-  operands[4] = GEN_INT (~INTVAL (operands[2]) & 0xffffffff);
+  operands[4] = GEN_INT (~INTVAL (operands[2]));
 }")
 
 ;; Split DImode logical operations requiring two instructions.
@@ -6717,7 +6711,7 @@
    (set (match_dup 0) (ior:SI (not:SI (match_dup 3)) (match_dup 1)))]
   "
 {
-  operands[4] = GEN_INT (~INTVAL (operands[2]) & 0xffffffff);
+  operands[4] = GEN_INT (~INTVAL (operands[2]));
 }")
 
 (define_insn "*or_not_di_sp32"
@@ -6833,7 +6827,7 @@
    (set (match_dup 0) (not:SI (xor:SI (match_dup 3) (match_dup 1))))]
   "
 {
-  operands[4] = GEN_INT (~INTVAL (operands[2]) & 0xffffffff);
+  operands[4] = GEN_INT (~INTVAL (operands[2]));
 }")
 
 (define_split
@@ -6848,7 +6842,7 @@
    (set (match_dup 0) (xor:SI (match_dup 3) (match_dup 1)))]
   "
 {
-  operands[4] = GEN_INT (~INTVAL (operands[2]) & 0xffffffff);
+  operands[4] = GEN_INT (~INTVAL (operands[2]));
 }")
 
 ;; xnor patterns.  Note that (a ^ ~b) == (~a ^ b) == ~(a ^ b).


-- 
Alexandre Oliva   Enjoy Guarana', see http://www.ic.unicamp.br/~oliva/
Red Hat GCC Developer                  aoliva@{cygnus.com, redhat.com}
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]