Re: [patch] Fix wrong code with VCE to bit-field type at -O

[Eric Botcazou <ebotcazou at adacore dot com>]

> We are using the type of a bitfield field for the replacement which
> we IMHO should avoid because the FIELD_DECLs size is 24
> but the fields type TYPE_SIZE is 32 (it's precision is 24).  That's
> all not an issue until you start to VIEW_CONVERT to such type
> (VIEW_CONVERT being a reference op just cares for size not
> precision).  Other ops are treated correctly by expansion.
> 
> Now - using a non-mode precision integer type as scalar replacement
> isn't going to produce great code and, as we can see, has "issues"
> when using VIEW_CONVERT_EXPRs.
> 
> SRA should either avoid this transform or fixup by VIEW_CONVERTing
> memory reads only to mode-precision integer types and then inserting
> a fixup cast.  The direct VIEW_CONVERsion it creates, from
> 
>   my_rec2.r1 = VIEW_CONVERT_EXPR<struct opt31__rec1>(*_56[1 ...]{lb: _3 sz:
> 1}); _58 = my_rec2.r1.f;
> 
> 
> to basically
> 
>   _58 = VIEW_CONVERT_EXPR<opt31__time_t___XDLU_0__11059199>(*_56[1
> ...]{lb: _3 sz: 1});
> 
> is simply wrong.

There is nothing obvious I think, i.e. that's debatable.  I agree that a VCE 
from a 32-bit object to a 32-bit integer with 24-bit precision should not 
clear the upper 8 bits (so the REDUCE_BIT_FIELD part of my patch is wrong).
But here we have a VCE from a 24-bit object to a 32-bit integer with 24-bit 
precision which reads *more bits* than the size of the source type; that I 
think is plain wrong and is fixed by the bit-field extraction in the patch.

> If you "fix" expansion then consider a nested VIEW_CONVERT_EXPR
> that views back to the aggregate type - is that now supposed to
> clear the upper 8 bits because of the VIEW_CONVERT_EXPR in the
> middle?  Not so.  So fixing VIEW_CONVERT_EXPR sounds conceptually
> wrong to me.

I agree that we need not clear, but we need to prevent the expansion from 
reading more bits than what is contained in the source type.  And this is 
sufficient to fix the regression.

> Not scalarizing a field to a DECL_BIT_FIELD FIELD_DECLs type looks like
> the best fix to me.

That seems like a big hammer though.

-- 
Eric Botcazou