diff mbox

[1/n] Make sizetype no longer sign-extended

Message ID alpine.LNX.2.00.1108171719280.2130@zhemvz.fhfr.qr
State New
Headers show

Commit Message

Richard Biener Aug. 17, 2011, 3:27 p.m. UTC 
Of course when you work on one thing (do not force sizetype for
POINTER_PLUS_EXPR) you run into that other thing you had in the
works (but abandoned because it's sooo boring).  Thus, here we come,
the appearant (huh!?) only pre-requesites to making sizetypes
no longer sign-extended (well, unsigned ones).

The issue is as usual - we think of these sizetype things as
twos-complement stuff (and thus are not worrying about signs),
but then go on and do arbitrary (signed) precision arithmetic
on them (using HOST_WIDE_INTs or double_ints) and interpret
the result, especially sometimes the sign-bit.  To preserve
previous (albeit also questionable) behavior simply sign-extend
that twos-complement quantity if we want to end up with
something suitable for a signed HOST_WIDE_INT.  That's certainly
easier than trying to think of a way to make callers of
get_inner_reference recognize that this bit_offset is a
pointer-bits + log2(BITS_PER_UNITS) twos-complement thing.

Joseph mentioned during the GCC Gathering that we shouldn't really
do so much with bit offsets (which later are usually just divided
by BITS_PER_UNIT anyway ...) and thus properly split in
a BITS_PER_UNIT and a remainder.  That doesn't solve the issue
completely, but at least it would make issues less likely to show up.
Anyway, too much for now.

Bootstrapped on x86_64-unknown-linux-gnu, testing in progress.

I don't expect any issues, the patch is a noop right now (because
of that sign-extension of unsigned sizetype), but I'll leave it
until tomorrow for comments anyway.

Thanks,
Richard.

2011-08-17  Richard Guenther  <rguenther@suse.de>

	* expr.c (get_inner_reference): Sign-extend the constant
	twos-complement offset before doing arbitrary precision
	arithmetic on it.
	* tree-ssa-structalias.c (get_constraint_for_ptr_offset): Likewise.
	(get_constraint_for_1): Pass the offset of a MEM_REF unchanged
	to get_constraint_for_ptr_offset.
	* tree-cfg.c (verify_types_in_gimple_reference): Do not
	compare integer constants by pointer.

Comments

Eric Botcazou Aug. 17, 2011, 4:36 p.m. UTC | #1 
> I don't expect any issues, the patch is a noop right now (because
> of that sign-extension of unsigned sizetype), but I'll leave it
> until tomorrow for comments anyway.
>
> Thanks,
> Richard.
>
> 2011-08-17  Richard Guenther  <rguenther@suse.de>
>
> 	* expr.c (get_inner_reference): Sign-extend the constant
> 	twos-complement offset before doing arbitrary precision
> 	arithmetic on it.
> 	* tree-ssa-structalias.c (get_constraint_for_ptr_offset): Likewise.
> 	(get_constraint_for_1): Pass the offset of a MEM_REF unchanged
> 	to get_constraint_for_ptr_offset.
> 	* tree-cfg.c (verify_types_in_gimple_reference): Do not
> 	compare integer constants by pointer.

The verify_types_in_gimple_reference change would suggest that we now can have 
TYPE_SIZEs with different types.  Is that true?
Richard Biener Aug. 18, 2011, 8:28 a.m. UTC | #2 
On Wed, 17 Aug 2011, Eric Botcazou wrote:

> > I don't expect any issues, the patch is a noop right now (because
> > of that sign-extension of unsigned sizetype), but I'll leave it
> > until tomorrow for comments anyway.
> >
> > Thanks,
> > Richard.
> >
> > 2011-08-17  Richard Guenther  <rguenther@suse.de>
> >
> > 	* expr.c (get_inner_reference): Sign-extend the constant
> > 	twos-complement offset before doing arbitrary precision
> > 	arithmetic on it.
> > 	* tree-ssa-structalias.c (get_constraint_for_ptr_offset): Likewise.
> > 	(get_constraint_for_1): Pass the offset of a MEM_REF unchanged
> > 	to get_constraint_for_ptr_offset.
> > 	* tree-cfg.c (verify_types_in_gimple_reference): Do not
> > 	compare integer constants by pointer.
> 
> The verify_types_in_gimple_reference change would suggest that we now can have 
> TYPE_SIZEs with different types.  Is that true?

No, ISTR the issue was arising with LTO where we'd happily merge
bitsizetype with sizetype or so.  I'll leave out this hunk and will
check if that still is a problem after I changed us not to merge
sizetypes but leave them alone.

Thanks,
Richard.
diff mbox

Patch

Index: trunk/gcc/expr.c
===================================================================
--- trunk.orig/gcc/expr.c	2011-08-17 16:28:06.000000000 +0200
+++ trunk/gcc/expr.c	2011-08-17 16:24:58.000000000 +0200
@@ -6502,12 +6502,14 @@  get_inner_reference (tree exp, HOST_WIDE
   /* If OFFSET is constant, see if we can return the whole thing as a
      constant bit position.  Make sure to handle overflow during
      this conversion.  */
-  if (host_integerp (offset, 0))
+  if (TREE_CODE (offset) == INTEGER_CST)
     {
-      double_int tem = double_int_lshift (tree_to_double_int (offset),
-					  BITS_PER_UNIT == 8
-					  ? 3 : exact_log2 (BITS_PER_UNIT),
-					  HOST_BITS_PER_DOUBLE_INT, true);
+      double_int tem = tree_to_double_int (offset);
+      tem = double_int_sext (tem, TYPE_PRECISION (sizetype));
+      tem = double_int_lshift (tree_to_double_int (offset),
+			       BITS_PER_UNIT == 8
+			       ? 3 : exact_log2 (BITS_PER_UNIT),
+			       HOST_BITS_PER_DOUBLE_INT, true);
       tem = double_int_add (tem, bit_offset);
       if (double_int_fits_in_shwi_p (tem))
 	{
Index: trunk/gcc/tree-cfg.c
===================================================================
--- trunk.orig/gcc/tree-cfg.c	2011-08-17 14:03:15.000000000 +0200
+++ trunk/gcc/tree-cfg.c	2011-08-17 16:27:30.000000000 +0200
@@ -3030,7 +3030,8 @@  verify_types_in_gimple_reference (tree e
 	      return true;
 	    }
 	  else if (TREE_CODE (op) == SSA_NAME
-		   && TYPE_SIZE (TREE_TYPE (expr)) != TYPE_SIZE (TREE_TYPE (op)))
+		   && !tree_int_cst_equal (TYPE_SIZE (TREE_TYPE (expr)),
+					   TYPE_SIZE (TREE_TYPE (op))))
 	    {
 	      error ("conversion of register to a different size");
 	      debug_generic_stmt (expr);
Index: trunk/gcc/tree-ssa-structalias.c
===================================================================
--- trunk.orig/gcc/tree-ssa-structalias.c	2011-08-17 14:13:21.000000000 +0200
+++ trunk/gcc/tree-ssa-structalias.c	2011-08-17 16:08:25.000000000 +0200
@@ -2876,7 +2876,7 @@  get_constraint_for_ptr_offset (tree ptr,
 {
   struct constraint_expr c;
   unsigned int j, n;
-  HOST_WIDE_INT rhsunitoffset, rhsoffset;
+  HOST_WIDE_INT rhsoffset;
 
   /* If we do not do field-sensitive PTA adding offsets to pointers
      does not change the points-to solution.  */
@@ -2891,15 +2891,24 @@  get_constraint_for_ptr_offset (tree ptr,
      solution which includes all sub-fields of all pointed-to
      variables of ptr.  */
   if (offset == NULL_TREE
-      || !host_integerp (offset, 0))
+      || TREE_CODE (offset) != INTEGER_CST)
     rhsoffset = UNKNOWN_OFFSET;
   else
     {
-      /* Make sure the bit-offset also fits.  */
-      rhsunitoffset = TREE_INT_CST_LOW (offset);
-      rhsoffset = rhsunitoffset * BITS_PER_UNIT;
-      if (rhsunitoffset != rhsoffset / BITS_PER_UNIT)
+      /* Sign-extend the offset.  */
+      double_int soffset
+	= double_int_sext (tree_to_double_int (offset),
+			   TYPE_PRECISION (TREE_TYPE (offset)));
+      if (!double_int_fits_in_shwi_p (soffset))
 	rhsoffset = UNKNOWN_OFFSET;
+      else
+	{
+	  /* Make sure the bit-offset also fits.  */
+	  HOST_WIDE_INT rhsunitoffset = soffset.low;
+	  rhsoffset = rhsunitoffset * BITS_PER_UNIT;
+	  if (rhsunitoffset != rhsoffset / BITS_PER_UNIT)
+	    rhsoffset = UNKNOWN_OFFSET;
+	}
     }
 
   get_constraint_for_rhs (ptr, results);
@@ -3260,8 +3269,8 @@  get_constraint_for_1 (tree t, VEC (ce_s,
 	    {
 	      struct constraint_expr cs;
 	      varinfo_t vi, curr;
-	      tree off = convert_to_ptrofftype (TREE_OPERAND (t, 1));
-	      get_constraint_for_ptr_offset (TREE_OPERAND (t, 0), off, results);
+	      get_constraint_for_ptr_offset (TREE_OPERAND (t, 0),
+					     TREE_OPERAND (t, 1), results);
 	      do_deref (results);
 
 	      /* If we are not taking the address then make sure to process