Re: [PATCH] Fix PR optimization/11210

[Eric Botcazou <ebotcazou at libertysurf dot fr>]

> Sorry about the delay and confusion here.  I found this rather non
> obvious.

Yes, the actual semantics of PUNSIGNEDP in decode_field_reference is rather 
special. But I should have explained what I was doing in the patch and not 
have been that terse. Sorry too :-)

> I have managed to convince myself now that this patch is OK, and that it is 
> the smallest fastest patch that works.

OK. I also regtested it on sparc64-sun-solaris2.9 with no regressions.

> I still don't like the expr.c handled_component_p comment you added.  A
> ??? comment implies that there is something wrong.  I don't believe
> there is anything wrong with get_inner_reference.  If you want to add a
> comment to document that TYPE_PRECISION can change even though TYPE_MODE
> does not because of how bitfields are represented, then that makes
> sense, but it shouldn't be a ??? comment.

Here is my problem: I'd have expected (and initially thought) that 
get_inner_reference would return the same thing if passed a bitfield or a 
bitfield wrapped up in a NOP_EXPR that doesn't affect the mode. There is 
this chunk of code in the function:

      /* We can go inside most conversions: all NON_VALUE_EXPRs, all normal
	 conversions that don't change the mode, and all view conversions
	 except those that need to "step up" the alignment.  */
      else if (TREE_CODE (exp) != NON_LVALUE_EXPR
	       && ! (TREE_CODE (exp) == VIEW_CONVERT_EXPR
		     && ! ((TYPE_ALIGN (TREE_TYPE (exp))
			    > TYPE_ALIGN (TREE_TYPE (TREE_OPERAND (exp, 0))))
			   && STRICT_ALIGNMENT
			   && (TYPE_ALIGN (TREE_TYPE (TREE_OPERAND (exp, 0)))
			       < BIGGEST_ALIGNMENT)
			   && (TYPE_ALIGN_OK (TREE_TYPE (exp))
			       || TYPE_ALIGN_OK (TREE_TYPE
						 (TREE_OPERAND (exp, 0))))))
	       && ! ((TREE_CODE (exp) == NOP_EXPR
		      || TREE_CODE (exp) == CONVERT_EXPR)
		     && (TYPE_MODE (TREE_TYPE (exp))
			 == TYPE_MODE (TREE_TYPE (TREE_OPERAND (exp, 0))))))
	break;

So the function is able to look through NOP_EXPRs to find the correct bitpos, 
but not the correct bitsize! What's the rationale for this semantics?

> I also raised the issue of whether we need to set outer_type in the
> BIT_AND_EXPR case, but I can't find any evidence showing that it is
> necessary, so the safe thing would be to not change the code here.

Same reasoning here.

> So I am approving the fold-const.c part of this patch.  And I am
> approving a non-??? comment for handled_component_p that documents that
> TYPE_PRECISION can change if we have a bitfield, if you care to write
> one.

Thanks for this in-depth review.

-- 
Eric Botcazou