[PATCH] BIT_FIELD_REF_UNSIGNED considered harmful

[Richard Guenther <rguenther at suse dot de>]

While adding constant-folding for integral argument BIT_FIELD_REF I
noticed we have no way of passing down BIT_FIELD_REF_UNSIGNED to
fold_ternary.  Luckily references to BIT_FIELD_REF_UNSIGNED are
rare - only expansion cares for it, through the use of 
get_inner_reference.

So I propose to tighthen the type constraints we set on valid
BIT_FIELD_REF to force its result type precision to match the extracted
bit-field size.  Which makes BIT_FIELD_REF_UNSIGNED redundant, as
for integral type the signedness of the result will determine how
it is extended to its full mode.  For non-integral result types we
always use unsigned extension and can force the bit-field size to
match the mode precision here.

So, minus fixing SRA and removing all traces of BIT_FIELD_REF_UNSIGNED,
the following makes sense for me.

Comments?

Thanks,
Richard.


2008-03-05  Richard Guenther  <rguenther@suse.de>

	* tree.def (BIT_FIELD_REF): Constrain result type and its precision.
	* tree-cfg.c (verify_expr): Verify BIT_FIELD_REF constraints on
	result type and precision.
	* expr.c (get_inner_reference): Set unsignedp based on the result
	type of BIT_FIELD_REF.

Index: tree.def
===================================================================
*** tree.def	(revision 132894)
--- tree.def	(working copy)
*************** DEFTREECODE (COMPONENT_REF, "component_r
*** 391,398 ****
     Operand 0 is the structure or union expression;
     operand 1 is a tree giving the constant number of bits being referenced;
     operand 2 is a tree giving the constant position of the first referenced bit.
!    The field can be either a signed or unsigned field;
!    BIT_FIELD_REF_UNSIGNED says which.  */
  DEFTREECODE (BIT_FIELD_REF, "bit_field_ref", tcc_reference, 3)
  
  /* The ordering of the following codes is optimized for the checking
--- 391,399 ----
     Operand 0 is the structure or union expression;
     operand 1 is a tree giving the constant number of bits being referenced;
     operand 2 is a tree giving the constant position of the first referenced bit.
!    The result type width has to match the number of bits referenced.
!    If the result type is integral, its signedness specifies how it is extended
!    to its mode width.  */
  DEFTREECODE (BIT_FIELD_REF, "bit_field_ref", tcc_reference, 3)
  
  /* The ordering of the following codes is optimized for the checking
Index: tree-cfg.c
===================================================================
*** tree-cfg.c	(revision 132894)
--- tree-cfg.c	(working copy)
*************** verify_expr (tree *tp, int *walk_subtree
*** 3273,3278 ****
--- 3273,3294 ----
  		  error ("invalid position or size operand to BIT_FIELD_REF");
  		  return t;
  		}
+ 	      if (INTEGRAL_TYPE_P (TREE_TYPE (t))
+ 		  && (TYPE_PRECISION (TREE_TYPE (t))
+ 		      != TREE_INT_CST_LOW (TREE_OPERAND (t, 1))))
+ 		{
+ 		  error ("integral result type precision does not match "
+ 			 "field size of BIT_FIELD_REF");
+ 		  return t;
+ 		}
+ 	      if (!INTEGRAL_TYPE_P (TREE_TYPE (t))
+ 		  && (GET_MODE_PRECISION (TYPE_MODE (TREE_TYPE (t)))
+ 		      != TREE_INT_CST_LOW (TREE_OPERAND (t, 1))))
+ 		{
+ 		  error ("mode precision of non-integral result does not "
+ 			 "match field size of BIT_FIELD_REF");
+ 		  return t;
+ 		}
  	    }
  
  	  t = TREE_OPERAND (t, 0);
Index: expr.c
===================================================================
*** expr.c	(revision 132893)
--- expr.c	(working copy)
*************** get_inner_reference (tree exp, HOST_WIDE
*** 5893,5899 ****
    else if (TREE_CODE (exp) == BIT_FIELD_REF)
      {
        size_tree = TREE_OPERAND (exp, 1);
!       *punsignedp = BIT_FIELD_REF_UNSIGNED (exp);
  
        /* For vector types, with the correct size of access, use the mode of
  	 inner type.  */
--- 5893,5900 ----
    else if (TREE_CODE (exp) == BIT_FIELD_REF)
      {
        size_tree = TREE_OPERAND (exp, 1);
!       *punsignedp = (! INTEGRAL_TYPE_P (TREE_TYPE (exp))
! 		     || TYPE_UNSIGNED (TREE_TYPE (exp)));
  
        /* For vector types, with the correct size of access, use the mode of
  	 inner type.  */