RFA: Fix SRA bug on big-endian targets

Richard Sandiford rsandifo@nildram.co.uk
Sun Nov 25 19:54:00 GMT 2007


execute/20040709-2.c started failing on many big-endian MIPS targets
after Alex's SRA changes from a month or so ago.  I didn't flag it then
because I thought I'd found a case that failed even before his patches,
but I'm no longer certain if that's true.

The problem in the testcase was triggered by:

  /* Now we know the bit range we're interested in.  Find the smallest
     machine mode we can use to access it.  */

  for (mode = smallest_mode_for_size (size, MODE_INT);
       ;
       mode = GET_MODE_WIDER_MODE (mode))
    {
      gcc_assert (mode != VOIDmode);

      alchk = GET_MODE_PRECISION (mode) - 1;
      alchk = ~alchk;

      if ((bit & alchk) == ((bit + size - 1) & alchk))
	break;
    }

  gcc_assert (~alchk < align);

  /* Create the field group as a single variable.  */

  type = lang_hooks.types.type_for_mode (mode, 1);
  gcc_assert (type);
  var = build3 (BIT_FIELD_REF, type, NULL_TREE,
		bitsize_int (size),
		bitsize_int (bit));
  BIT_FIELD_REF_UNSIGNED (var) = 1;

which seems to deliberately go out of its way to avoid
lang_hooks.types.type_for_size, and probably for good reason.
But this "smallest containing mode" thing means that the mode
(and thus the type) may be wider than the BIT_FIELD_REF itself.

So: must a BIT_FIELD_REF's length operand match the TYPE_SIZE of
its type?  I couldn't find a definite answer in the comments.  I assume
that the answer is "no" based on the code above, and based on the assumption
that we wouldn't have a length operand at all if the type was supposed
to be a reliable indicator.

If the answer is indeed "no", there seems to be a missing endian
correction in the calls to sra_explode_bitfield_assignment from
scalarize_use.  BIT_FIELD_REFs always seem to use MSB==0 numbering
for BYTES_BIG_ENDIAN (rather than testing BITS_BIG_ENDIAN, etc.),
so if the variable passed to sra_explode_bitfield_assignment
is wider than the access, the incoming vpos should account
for the padding.

Bootstrapped & regression-tested on x86_64-linux-gnu.  Also
regression-tested on mipsisa64-elfoabi, where it fixes the
20040709-2.c failures.  OK to install?

Richard


gcc/
	* tree-sra.c (scalarize_use): Adjust the vpos argument to
	sra_explode_bitfield_assignment in cases where the type is
	wider than the bitfield.

Index: gcc/tree-sra.c
===================================================================
--- gcc/tree-sra.c	(revision 130395)
+++ gcc/tree-sra.c	(working copy)
@@ -3147,7 +3147,7 @@ scalarize_use (struct sra_elt *elt, tree
       if (!elt->use_block_copy)
 	{
 	  tree type = TREE_TYPE (bfexpr);
-	  tree var = make_rename_temp (type, "SR"), tmp, st;
+	  tree var = make_rename_temp (type, "SR"), tmp, st, vpos;
 
 	  GIMPLE_STMT_OPERAND (stmt, 0) = var;
 	  update = true;
@@ -3162,8 +3162,11 @@ scalarize_use (struct sra_elt *elt, tree
 	      var = tmp;
 	    }
 
+	  vpos = (BYTES_BIG_ENDIAN
+		  ? size_binop (MINUS_EXPR, TYPE_SIZE (type), blen)
+		  : bitsize_int (0));
 	  sra_explode_bitfield_assignment
-	    (var, bitsize_int (0), false, &listafter, blen, bpos, elt);
+	    (var, vpos, false, &listafter, blen, bpos, elt);
 	}
       else
 	sra_sync_for_bitfield_assignment
@@ -3199,7 +3202,7 @@ scalarize_use (struct sra_elt *elt, tree
       if (!elt->use_block_copy)
 	{
 	  tree type = TREE_TYPE (bfexpr);
-	  tree var;
+	  tree var, vpos;
 
 	  if (!TYPE_UNSIGNED (type))
 	    type = unsigned_type_for (type);
@@ -3210,8 +3213,11 @@ scalarize_use (struct sra_elt *elt, tree
 				    (var, build_int_cst_wide (type, 0, 0)),
 				    &list);
 
+	  vpos = (BYTES_BIG_ENDIAN
+		  ? size_binop (MINUS_EXPR, TYPE_SIZE (type), blen)
+		  : bitsize_int (0));
 	  sra_explode_bitfield_assignment
-	    (var, bitsize_int (0), true, &list, blen, bpos, elt);
+	    (var, vpos, true, &list, blen, bpos, elt);
 
 	  GIMPLE_STMT_OPERAND (stmt, 1) = var;
 	  update = true;



More information about the Gcc-patches mailing list