On Fri, Jun 27, 2014 at 8:51 AM, Thomas Preud'homme
<thomas.preudho...@arm.com> wrote:
> Greetings everybody,
>
> I'm seeking your advice on how to best solve a bug. The issue has to do with
> folding a bitfield contained in a union. Consider the following example:
>
> union U {
> unsigned int a:24;
> unsigned int b:20;
> } u = { .a = 0x345678 };
>
> int foo (void)
> {
> return u.b;
> }
>
> Currently, folding (fold_nonarray_ctor_reference and fold_ctor_reference)
> does the following:
>
> 1) Compute the difference between bit offset of a and b
> 2) Check that b's area is within a's area in memory
> 3) Do nothing if 1) yields a non-zero difference
> 4) Cast value stored in a according to the size of b by removing bits of
> highest weight (via VIEW_CONVERT_EXPR).
Fact is that 4) already is not working.
/* We are at the end of walk, see if we can view convert the
result. */
if (!AGGREGATE_TYPE_P (TREE_TYPE (ctor)) && !offset
/* VIEW_CONVERT_EXPR is defined only for matching sizes. */
&& operand_equal_p (TYPE_SIZE (type),
TYPE_SIZE (TREE_TYPE (ctor)), 0))
while it's true we don't check precision in the gimple-verifier
VIEW_CONVERT_EXPR isn't supposed to be applied to types
with same size but differing precision. VIEW_CONVERT_EXPR
is supposed to _not_ strip any excess bits! (it's supposed to do
"nothing").
fold_view_convert_expr might do the correct thing (but only possibly
because we now use wide_int here...).
> This does not work for big endian targets since bitfield respect byte
> endianness (and I support word endianness) when layed out in memory.
> So a would end up like this (highest addresses on the right):
>
> --------------------------
> | Ox34 | 0x56 | 0x78 |
> --------------------------
>
> b would be the first two byte and then 4 highest bits of the third byte,
> as if memory was a sequence of bits instead bytes with highest value
> bits at the lowest "bit address"
>
> So the expected result would be 0x34567 but a cast as does
> VIEW_CONVERT_EXPR would return 0x45678. Forbidding folding in such
> case for big endian would work but I've been looking about a way to
> make bitfield folding also work for big endian. The approach I've been
> taking so far is to enhance native_encode_integer and
> native_interpret_integer to take a start parameter and express start
> and len as addressing bits rather than byte. Then in
> fold_nonarray_ctor_reference whenever the constructor is for a
> bitfield, that bitfield is encoded with its bit offset and bit length to
> target memory layout, then interpreted to the host as its type qualifier
> (unsigned int in the above example). Finally, a call to fold_ternary with
> a subcode of BIT_FIELD_REF is made which will encode that integer to
> the target memory layout as the type qualifier and decode it according
> to the bit offset and bit length of bitfield b. Note that juste replacing
> VIEW_CONVERT_EXPR by BIT_FIELD_REF has the advantage of
> allowing folding in simpler solutions like a union of an integer and
> a char which is currently not folded even for little endian target.
Hmm. I wonder if the code behaves correctly for
union {
struct { a : 10; b : 6 } a;
struct { a : 8; b : 8 } b;
} u = { .a = { .... } };
and reading u.b.a or u.b.b?
That is, is fold_nonarray_ctor_reference doing the right thing with
consecutive bitfield fields?
Is the current code behaving correctly for size % BITS_PER_UNIT == 0?
If so that might be a good fix for branches (who cares about
constant folding bitfield constants ... not many, given the bug you
show).
> However this double conversion between host and target memory
> layout is unelegant and unefficient. I could do just one conversion
> completely in fold_nonarray_ctor_reference but somehow I feel it is
> not the right place to do it. As anybody a better advice?
I wonder if you have a patch to show us how unelegant it is?
Btw, another idea would be to support native_encode for the
whole CONSTRUCTOR and then use native_interpret with
offset/size. Of course for large CONSTRUCTORs that may
be prohibitive (but native_encode could get a start offset as well,
to skip uninteresting bytes).
Being able to native encode a CONSTRUCTOR could be
factored and re-used from asm output.
And it could be used to more efficiently store large aggregate
constant initializers (native encode it and compress it instead
of keeping CONSTRUCTOR).
So any incremental improvement to native_{encode/interpret}_expr
is good.
Richard.
> Best regards,
>
> Thomas Preud'homme
>
>
