Re: [PATCH] Fix PR optimization/11210

[Eric Botcazou <ebotcazou at libertysurf dot fr>]

> > decode_field_reference() throws away surrounding NOPs in the expression
> > it is passed. Now it sets 'punsignedp' to the signedness of the field,
> > so I think it must strip only NOPs that don't affect the sign.
>
> OK.

While the idea is correct, the implementation is far from being blameless :-(

I got caught by expr.c:get_inner_reference(), which is advertised as handling 
NOP_EXPR and the likes, but doesn't return the value you would have expected 
for NOP_EXPRs wrapping up bitfields. Not sure if this is a bug or a feature, 
but this is certainly not what we want in decode_field_reference(). It's 
amazing that neither IA-32 nor PPC caught the problem.

So I ended up patching decode_field_reference() itself and came up with the 
following (admittedly a bit ad hoc) patch.

Bootstrapped/regtested on i586-redhat-linux-gnu (except Ada), bootstrapped on 
sparc64-sun-solaris2.9 (except Ada).

-- 
Eric Botcazou


2003-06-28  Eric Botcazou  <ebotcazou@libertysurf.fr>

        PR optimization/11210
	* expr.c (handled_component_p) [NOP_EXPR]: Add ??? note
	about the behaviour with regard to bitfields.
        * fold-const (decode_field_reference): Record outermost type in
	case the expression is a NOP. Strip all NOPs. Set the signedness
	to that of the outermost type (if any) when the bitsize is equal to the
	size of the type.

Index: fold-const.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/fold-const.c,v
retrieving revision 1.267
diff -u -p -r1.267 fold-const.c
--- fold-const.c	26 Jun 2003 10:37:33 -0000	1.267
+++ fold-const.c	27 Jun 2003 22:30:02 -0000
@@ -2731,6 +2731,7 @@ decode_field_reference (exp, pbitsize, p
      tree *pmask;
      tree *pand_mask;
 {
+  tree outer_type = 0;
   tree and_mask = 0;
   tree mask, inner, offset;
   tree unsigned_type;
@@ -2742,8 +2743,14 @@ decode_field_reference (exp, pbitsize, p
   if (! INTEGRAL_TYPE_P (TREE_TYPE (exp)))
     return 0;
 
-  /* Signedness matters here.  */
-  STRIP_SIGN_NOPS (exp);
+  /* We are interested in the bare arrangement of bits, so strip everything
+     that doesn't affect the machine mode.  However, record the type of the
+     outermost expression if it may matter below.  */
+  if (TREE_CODE (exp) == NOP_EXPR
+      || TREE_CODE (exp) == CONVERT_EXPR
+      || TREE_CODE (exp) == NON_LVALUE_EXPR)
+    outer_type = TREE_TYPE (exp);
+  STRIP_NOPS (exp);
 
   if (TREE_CODE (exp) == BIT_AND_EXPR)
     {
@@ -2760,6 +2767,12 @@ decode_field_reference (exp, pbitsize, p
       || *pbitsize < 0 || offset != 0
       || TREE_CODE (inner) == PLACEHOLDER_EXPR)
     return 0;
+
+  /* If the number of bits in the reference is the same as the bitsize of
+     the outer type, then the outer type gives the signedness. Otherwise
+     (in case of a small bitfield) the signedness is unchanged.  */
+  if (outer_type && *pbitsize == tree_low_cst (TYPE_SIZE (outer_type), 1))
+    *punsignedp = TREE_UNSIGNED (outer_type);
 
   /* Compute the mask to access the bitfield.  */
   unsigned_type = (*lang_hooks.types.type_for_size) (*pbitsize, 1);
Index: expr.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/expr.c,v
retrieving revision 1.555
diff -u -p -r1.555 expr.c
--- expr.c	23 Jun 2003 15:27:35 -0000	1.555
+++ expr.c	28 Jun 2003 07:56:30 -0000
@@ -5998,6 +5998,8 @@ handled_component_p (t)
 
     case NOP_EXPR:
     case CONVERT_EXPR:
+      /* ??? Sure they are handled, but they don't return what
+	 you would expect in all cases (e.g. for bitfields).  */
       return (TYPE_MODE (TREE_TYPE (t))
 	      == TYPE_MODE (TREE_TYPE (TREE_OPERAND (t, 0))));