wide-int, rtl

Eric Botcazou ebotcazou@adacore.com
Wed Nov 27 17:43:00 GMT 2013


> Richi has asked the we break the wide-int patch so that the individual port
> and front end maintainers can review their parts without have to go through
> the entire patch.    This patch covers the first half of the rtl code.

--- a/gcc/cse.c
+++ b/gcc/cse.c
@@ -2336,15 +2336,23 @@ hash_rtx_cb (const_rtx x, enum machine_mode mode,
                + (unsigned int) INTVAL (x));
       return hash;
 
+    case CONST_WIDE_INT:
+      {
+	int i;
+	for (i = 0; i < CONST_WIDE_INT_NUNITS (x); i++)
+	  hash += CONST_WIDE_INT_ELT (x, i);
+      }
+      return hash;

You can write "for (int i = 0; ..." now and remove the parentheses.


--- a/gcc/cselib.c
+++ b/gcc/cselib.c
@@ -1121,15 +1120,23 @@ cselib_hash_rtx (rtx x, int create, enum machine_mode 
memmode)
       hash += ((unsigned) CONST_INT << 7) + INTVAL (x);
       return hash ? hash : (unsigned int) CONST_INT;
 
+    case CONST_WIDE_INT:
+      {
+	int i;
+	for (i = 0; i < CONST_WIDE_INT_NUNITS (x); i++)
+	  hash += CONST_WIDE_INT_ELT (x, i);
+      }
+      return hash;
+

Likewise.


--- a/gcc/expmed.c
+++ b/gcc/expmed.c
@@ -3298,23 +3268,13 @@ choose_multiplier (unsigned HOST_WIDE_INT d, int n, 
int precision,
   pow = n + lgup;
   pow2 = n + lgup - precision;
 
-  /* We could handle this with some effort, but this case is much
-     better handled directly with a scc insn, so rely on caller using
-     that.  */
-  gcc_assert (pow != HOST_BITS_PER_DOUBLE_INT);

Why removing it?



--- a/gcc/expr.c
+++ b/gcc/expr.c
@@ -722,64 +722,33 @@ convert_modes (enum machine_mode mode, enum machine_mode 
oldmode, rtx x, int uns
   if (mode == oldmode)
     return x;
 
-  /* There is one case that we must handle specially: If we are converting
-     a CONST_INT into a mode whose size is twice HOST_BITS_PER_WIDE_INT and
-     we are to interpret the constant as unsigned, gen_lowpart will do
-     the wrong if the constant appears negative.  What we want to do is
-     make the high-order word of the constant zero, not all ones.  */
-
-  if (unsignedp && GET_MODE_CLASS (mode) == MODE_INT
-      && GET_MODE_BITSIZE (mode) == HOST_BITS_PER_DOUBLE_INT
-      && CONST_INT_P (x) && INTVAL (x) < 0)
+  if (CONST_SCALAR_INT_P (x)
+      && GET_MODE_CLASS (mode) == MODE_INT)

On a single line.


     {
-      double_int val = double_int::from_uhwi (INTVAL (x));
-
-      /* We need to zero extend VAL.  */
-      if (oldmode != VOIDmode)
-	val = val.zext (GET_MODE_BITSIZE (oldmode));
-
-      return immed_double_int_const (val, mode);
+      /* If the caller did not tell us the old mode, then there is
+	 not much to do with respect to canonization.  We have to assume
+	 that all the bits are significant.  */
+      if (GET_MODE_CLASS (oldmode) != MODE_INT)
+	oldmode = MAX_MODE_INT;
+      wide_int w = wide_int::from (std::make_pair (x, oldmode),
+				   GET_MODE_PRECISION (mode),
+				   unsignedp ? UNSIGNED : SIGNED);
+      return immed_wide_int_const (w, mode);

canonicalization?


@@ -5301,10 +5271,10 @@ store_expr (tree exp, rtx target, int call_param_p, 
bool nontemporal)
 			       &alt_rtl);
     }
 
-  /* If TEMP is a VOIDmode constant and the mode of the type of EXP is not
-     the same as that of TARGET, adjust the constant.  This is needed, for
-     example, in case it is a CONST_DOUBLE and we want only a word-sized
-     value.  */
+  /* If TEMP is a VOIDmode constant and the mode of the type of EXP is
+     not the same as that of TARGET, adjust the constant.  This is
+     needed, for example, in case it is a CONST_DOUBLE or
+     CONST_WIDE_INT and we want only a word-sized value.  */
   if (CONSTANT_P (temp) && GET_MODE (temp) == VOIDmode
       && TREE_CODE (exp) != ERROR_MARK
       && GET_MODE (target) != TYPE_MODE (TREE_TYPE (exp)))

Why reformatting the whole comment?


@@ -9481,11 +9459,19 @@ expand_expr_real_1 (tree exp, rtx target, enum 
machine_mode tmode,
       return decl_rtl;
 
     case INTEGER_CST:
-      temp = immed_double_const (TREE_INT_CST_LOW (exp),
-				 TREE_INT_CST_HIGH (exp), mode);
-
-      return temp;
-
+      {
+	tree type = TREE_TYPE (exp);

Redundant, see the beginning of the function.

+	/* One could argue that GET_MODE_PRECISION (TYPE_MODE (type))
+	   should always be the same as TYPE_PRECISION (type).
+	   However, it is not.  Since we are converting from tree to
+	   rtl, we have to expose this ugly truth here.  */
+	temp = immed_wide_int_const (wide_int::from
+				       (exp,
+					GET_MODE_PRECISION (TYPE_MODE (type)),
+					TYPE_SIGN (type)),
+				     TYPE_MODE (type));
+	return temp;
+      }

I don't really see how one could argue that, given that there are much fewer
modes than possible type precisions, so please rewrite the comment, e.g.:

"Given that TYPE_PRECISION (type) is not always equal to
GET_MODE_PRECISION (TYPE_MODE (type)), we need to extend from the former
to the latter according to the signedness of the type".

What about a fast track where the precisions are indeed equal?



@@ -11145,8 +11131,8 @@ const_vector_from_tree (tree exp)
 	RTVEC_ELT (v, i) = CONST_FIXED_FROM_FIXED_VALUE (TREE_FIXED_CST (elt),
 							 inner);
       else
-	RTVEC_ELT (v, i) = immed_double_int_const (tree_to_double_int (elt),
-						   inner);
+	RTVEC_ELT (v, i)
+	  = immed_wide_int_const (elt, TYPE_MODE (TREE_TYPE (elt)));
     }
 
   return gen_rtx_CONST_VECTOR (mode, v);

Why replacing inner?


--- a/gcc/machmode.def
+++ b/gcc/machmode.def
@@ -229,6 +229,9 @@ UACCUM_MODE (USA, 4, 16, 16); /* 16.16 */
 UACCUM_MODE (UDA, 8, 32, 32); /* 32.32 */
 UACCUM_MODE (UTA, 16, 64, 64); /* 64.64 */
 
+/* Should be overridden by EXTRA_MODES_FILE if wrong.  */
+#define MAX_BITS_PER_UNIT 8
+

What is it for?  It's not documented at all.


--- a/gcc/print-rtl.c
+++ b/gcc/print-rtl.c
@@ -617,6 +617,12 @@ print_rtx (const_rtx in_rtx)
 	  fprintf (outfile, " [%s]", s);
 	}
       break;
+
+    case CONST_WIDE_INT:
+      if (! flag_simple)
+	fprintf (outfile, " ");
+      cwi_output_hex (outfile, in_rtx);
+      break;
 #endif
 
Remove the if (! flag_simple) test.


--- a/gcc/recog.c
+++ b/gcc/recog.c

 /* Returns 1 if OP is an operand that is a constant integer or constant
-   floating-point number.  */
+   floating-point number of MODE.  */
+
+int
+const_double_operand (rtx op, enum machine_mode mode)
+{
+  return (GET_CODE (op) == CONST_DOUBLE)
+	  && (GET_MODE (op) == mode || mode == VOIDmode);
+}

Can CONST_DOUBLEs still represent constant integers?


diff --git a/gcc/rtl.def b/gcc/rtl.def
index 15a997b..a76b28b 100644
--- a/gcc/rtl.def
+++ b/gcc/rtl.def
@@ -345,6 +345,9 @@ DEF_RTL_EXPR(TRAP_IF, "trap_if", "ee", RTX_EXTRA)
 /* numeric integer constant */
 DEF_RTL_EXPR(CONST_INT, "const_int", "w", RTX_CONST_OBJ)
 
+/* numeric integer constant */
+DEF_RTL_EXPR(CONST_WIDE_INT, "const_wide_int", "", RTX_CONST_OBJ)
+

I think that we should consider using a new letter here, for example W.
"" is reserved to very specific cases.


--- a/gcc/rtl.h
+++ b/gcc/rtl.h
@@ -20,6 +20,7 @@ along with GCC; see the file COPYING3.  If not see
 #ifndef GCC_RTL_H
 #define GCC_RTL_H
 
+#include <utility>

Ouch.  Can't this really be avoided?


@@ -336,6 +348,14 @@ struct GTY((chain_next ("RTX_NEXT (&%h)"),
      1 in a VALUE or DEBUG_EXPR is NO_LOC_P in var-tracking.c.  */
   unsigned return_val : 1;
 
+  union {
+    /* RTXs are free to use up to 32 bit from here.  */
+
+    /* In a CONST_WIDE_INT (aka hwivec_def), this is the number of
+       HOST_WIDE_INTs in the hwivec_def.  */
+    unsigned GTY ((tag ("CONST_WIDE_INT"))) num_elem:32;
+  } GTY ((desc ("GET_CODE (&%0)"))) u2;
+
   /* The first element of the operands of this rtx.
      The number of operands and their types are controlled
      by the `code' field, according to rtl.def.  */

Why can't this be put into hwivec_def?


--- a/gcc/simplify-rtx.c
+++ b/gcc/simplify-rtx.c

+      if (op_mode == VOIDmode)
+	{
+	  /* CONST_INT have VOIDmode as the mode.  We assume that all
+	     the bits of the constant are significant, though, this is
+	     a dangerous assumption as many times CONST_INTs are
+	     created and used with garbage in the bits outside of the
+	     precision of the implied mode of the const_int.  */
+	  op_mode = MAX_MODE_INT;
+	}

Well, if that's really dangerous, we should find another way or at least
stop the compiler before generating wrong code.


+#if TARGET_SUPPORTS_WIDE_INT == 0
+      /* This assert keeps the simplification from producing a result
+	 that cannot be represented in a CONST_DOUBLE but a lot of
+	 upstream callers expect that this function never fails to
+	 simplify something and so you if you added this to the test
+	 above the code would die later anyway.  If this assert
+	 happens, you just need to make the port support wide int.  */
+      gcc_assert (width <= HOST_BITS_PER_DOUBLE_INT);
+#endif

Can't we be more forgiving here and in the other similar places?


+  /* We support any size mode.  */
+  max_bitsize = MAX (GET_MODE_BITSIZE (outermode),
+		     GET_MODE_BITSIZE (innermode));

max_bitsize
  = MAX (GET_MODE_BITSIZE (outermode), GET_MODE_BITSIZE (innermode));


-- 
Eric Botcazou



More information about the Gcc-patches mailing list