Re: sizetype, bitsizetype and DECL_FIELD_BITPOS

Re: sizetype, bitsizetype and DECL_FIELD_BITPOS

[Richard Kenner]

    >                                        I see places where it is
    > used in both contexts and, unless I'm missing something, we are missing
    > conversions.
    
    I suppose you mean this?

          offset = size_binop (PLUS_EXPR, offset,
                               size_binop (EXACT_DIV_EXPR, var,
                                           size_int (BITS_PER_UNIT)));

Yes.  VAR is clearly bitsizetype and OFFSET is supposed to be sizetype
and there's no conversion.

Re: sizetype, bitsizetype and DECL_FIELD_BITPOS

[Joern Rennecke]

> Yes.  VAR is clearly bitsizetype and OFFSET is supposed to be sizetype
> and there's no conversion.

size_binop is supposed to convert to sizetype.

Re: sizetype, bitsizetype and DECL_FIELD_BITPOS

[Joern Rennecke]

> Yes.  VAR is clearly bitsizetype and OFFSET is supposed to be sizetype
> and there's no conversion.

size_binop is supposed to convert to sizetype.

Re: sizetype, bitsizetype and DECL_FIELD_BITPOS

[Richard Kenner]

    >                                        I see places where it is
    > used in both contexts and, unless I'm missing something, we are missing
    > conversions.
    
    I suppose you mean this?

          offset = size_binop (PLUS_EXPR, offset,
                               size_binop (EXACT_DIV_EXPR, var,
                                           size_int (BITS_PER_UNIT)));

Yes.  VAR is clearly bitsizetype and OFFSET is supposed to be sizetype
and there's no conversion.

Re: sizetype, bitsizetype and DECL_FIELD_BITPOS

[Richard Kenner]

    > Yes.  VAR is clearly bitsizetype and OFFSET is supposed to be sizetype
    > and there's no conversion.

    size_binop is supposed to convert to sizetype.

In which cases?  Surely it can't convert both of its args unconditionally to
sizetype since it's called to do an EXACT_DIV_EXPR of a bitsizetype value by
BITS_PER_UNIT.  That division has to be in bitsizetype.  And in that case the
BITS_PER_UNIT also needs to be bitsizetype, while it's currently sizetype.
Moreover, in a number of cases, the WITH_RECORD_EXPR that's being made is
also the wrong type (sizetype instead of bitsizetype).

I'm certainly willing to fix this all up, but I'd like to try to get a
concensus as to what to fix it up *to*.  I'd really like to better understand
the reason for adding all this in the first place since it seems like it'll
be quite a lot of trouble to get this to work properly and I'm not convinced
it's worth it and it wouldn't be simpler just to back this all out and try to
find another way to fix the original problem (which I don't have a complete
explanation of yet).

Re: sizetype, bitsizetype and DECL_FIELD_BITPOS

[Joern Rennecke]

> Also, there is a comment referencing DECL_SIZE_UNIT, but I see no such
> macros.  Then there's the code in get_inner_reference: first of all,
> is OFFSET of bitsizetype or sizetype?

OFFSET is sizetype.

>                                        I see places where it is
> used in both contexts and, unless I'm missing something, we are missing
> conversions.

I suppose you mean this?

          offset = size_binop (PLUS_EXPR, offset,
                               size_binop (EXACT_DIV_EXPR, var,
                                           size_int (BITS_PER_UNIT)));

size_binop is documented to get its type from sizetype.
I appears to do something different, which will matter if offset is zero
and has a type that is not sizetype.

Otherwise, the EXACT_DIV_EXPR should be translated to a straight shift and
the rest of the computation should be done in sizetype.


>               Secondly, the array code talks about if the bitsize
> computation overflow, but I see no overflow test.

Yes, indeed, the comment is imprecise at best.  A non-zero
TREE_INT_CST_HIGH (xindex) doesn't mean that an overflow occured, it
just means that pbitpos is unsuitable to pass the result back to the
caller.  That can happen in a 32->64 bit cross-compilation, for instance.
Incidentally, pbitpos is an int *, but it would have to be a HOST_WIDE_INT
to accomodate TREE_INT_CST_LOW (xindex)

sizetype, bitsizetype and DECL_FIELD_BITPOS

[Richard Kenner]

I'm a bit confused about the new bitsizetype and TYPE_SIZE_UNIT.

As I understand it, the purpose of this new code is to allow types and
objects to be 2**32 *bytes* long instead of 2**32 *bits* long (on
32-bit machines) that we used to allow.

So sizetype always measures bytes and bitsizetype always measures bits.
We have a TYPE_SIZE_UNIT to represent the size of a type in bits and
that's of type sizetype.

On 32-bit machines, as I understand it, sizetype will be 32 bits and
bitsizetype will need 64 bit arithmetic.

But what about DECL_FIELD_BITPOS?  That's used all the time when a fiekld
of a struct has variable size.  It used to be 32-bit, but now it looks
like it involves 64-bit arithmetic.  Is that what was intended?

Also, there is a comment referencing DECL_SIZE_UNIT, but I see no such
macros.  Then there's the code in get_inner_reference: first of all,
is OFFSET of bitsizetype or sizetype?  I see places where it is
used in both contexts and, unless I'm missing something, we are missing
conversions.  Secondly, the array code talks about if the bitsize
computation overflow, but I see no overflow test.

We've always said that DECL_FIELD_BITPOS was either or constant or
a variable possibily plus a constnat, where the variable was known
to be a multiple of BITS_PER_UNIT.  But now do we have to say that
it's a PLUS_EXPR of a conversion plus a constant and then change
teh code using it appropriately? I certainly don't think we want to
force all such computations to 64 bits.

Am I missing something here or are those bugs?

Re: sizetype, bitsizetype and DECL_FIELD_BITPOS

[Joern Rennecke]

>     size_binop is supposed to convert to sizetype.
> 
> In which cases?  Surely it can't convert both of its args unconditionally to
> sizetype since it's called to do an EXACT_DIV_EXPR of a bitsizetype value by
> BITS_PER_UNIT.  That division has to be in bitsizetype.  And in that case the
> BITS_PER_UNIT also needs to be bitsizetype, while it's currently sizetype.
> Moreover, in a number of cases, the WITH_RECORD_EXPR that's being made is
> also the wrong type (sizetype instead of bitsizetype).

I read the comment that size_binop is supposed to convert the result
to sizetype.

If size_binop required all inputs and the output to be the same type, it
should just get the result type from one of the arguments, and be called
tree_binop or something similarily type-independent.

Re: sizetype, bitsizetype and DECL_FIELD_BITPOS

[Richard Kenner]

    I read the comment that size_binop is supposed to convert the result
    to sizetype.

Hmm... well, that *will* work in all current usages, except that we don't
always pass both inputs of the same type, but I'm concerned about such
a specification since it means that somebody accidentally calling it
to add two bitsizetype values will have an unexpected truncation.

    If size_binop required all inputs and the output to be the same type,
    it should just get the result type from one of the arguments, and be
    called tree_binop or something similarily type-independent.

Well, yes, I quite understand that.  And that's part of the reason why I'm
getting increasingly dubious about this whole idea of having both sizetype and
bitsizetype: it makes the use of size_binop *very* tricky.  Indeed, size_int
has the same problem. We have a bitsize_int now, but it seems that, for
some reason, its parameters are different (and nearly always called wrong,
since "0L" is totally bogus).  Perhaps we need a bitsize_binop and then
an explicit conversion, but I'd still like to be convinced that all this
complexity is actually buying us something that justifies it.

Re: sizetype, bitsizetype and DECL_FIELD_BITPOS

[Richard Kenner]

    I read the comment that size_binop is supposed to convert the result
    to sizetype.

Hmm... well, that *will* work in all current usages, except that we don't
always pass both inputs of the same type, but I'm concerned about such
a specification since it means that somebody accidentally calling it
to add two bitsizetype values will have an unexpected truncation.

    If size_binop required all inputs and the output to be the same type,
    it should just get the result type from one of the arguments, and be
    called tree_binop or something similarily type-independent.

Well, yes, I quite understand that.  And that's part of the reason why I'm
getting increasingly dubious about this whole idea of having both sizetype and
bitsizetype: it makes the use of size_binop *very* tricky.  Indeed, size_int
has the same problem. We have a bitsize_int now, but it seems that, for
some reason, its parameters are different (and nearly always called wrong,
since "0L" is totally bogus).  Perhaps we need a bitsize_binop and then
an explicit conversion, but I'd still like to be convinced that all this
complexity is actually buying us something that justifies it.

Re: sizetype, bitsizetype and DECL_FIELD_BITPOS

[Joern Rennecke]

>     size_binop is supposed to convert to sizetype.
> 
> In which cases?  Surely it can't convert both of its args unconditionally to
> sizetype since it's called to do an EXACT_DIV_EXPR of a bitsizetype value by
> BITS_PER_UNIT.  That division has to be in bitsizetype.  And in that case the
> BITS_PER_UNIT also needs to be bitsizetype, while it's currently sizetype.
> Moreover, in a number of cases, the WITH_RECORD_EXPR that's being made is
> also the wrong type (sizetype instead of bitsizetype).

I read the comment that size_binop is supposed to convert the result
to sizetype.

If size_binop required all inputs and the output to be the same type, it
should just get the result type from one of the arguments, and be called
tree_binop or something similarily type-independent.

Re: sizetype, bitsizetype and DECL_FIELD_BITPOS

[Richard Kenner]

    > Yes.  VAR is clearly bitsizetype and OFFSET is supposed to be sizetype
    > and there's no conversion.

    size_binop is supposed to convert to sizetype.

In which cases?  Surely it can't convert both of its args unconditionally to
sizetype since it's called to do an EXACT_DIV_EXPR of a bitsizetype value by
BITS_PER_UNIT.  That division has to be in bitsizetype.  And in that case the
BITS_PER_UNIT also needs to be bitsizetype, while it's currently sizetype.
Moreover, in a number of cases, the WITH_RECORD_EXPR that's being made is
also the wrong type (sizetype instead of bitsizetype).

I'm certainly willing to fix this all up, but I'd like to try to get a
concensus as to what to fix it up *to*.  I'd really like to better understand
the reason for adding all this in the first place since it seems like it'll
be quite a lot of trouble to get this to work properly and I'm not convinced
it's worth it and it wouldn't be simpler just to back this all out and try to
find another way to fix the original problem (which I don't have a complete
explanation of yet).

Re: sizetype, bitsizetype and DECL_FIELD_BITPOS

[Joern Rennecke]

> Also, there is a comment referencing DECL_SIZE_UNIT, but I see no such
> macros.  Then there's the code in get_inner_reference: first of all,
> is OFFSET of bitsizetype or sizetype?

OFFSET is sizetype.

>                                        I see places where it is
> used in both contexts and, unless I'm missing something, we are missing
> conversions.

I suppose you mean this?

          offset = size_binop (PLUS_EXPR, offset,
                               size_binop (EXACT_DIV_EXPR, var,
                                           size_int (BITS_PER_UNIT)));

size_binop is documented to get its type from sizetype.
I appears to do something different, which will matter if offset is zero
and has a type that is not sizetype.

Otherwise, the EXACT_DIV_EXPR should be translated to a straight shift and
the rest of the computation should be done in sizetype.


>               Secondly, the array code talks about if the bitsize
> computation overflow, but I see no overflow test.

Yes, indeed, the comment is imprecise at best.  A non-zero
TREE_INT_CST_HIGH (xindex) doesn't mean that an overflow occured, it
just means that pbitpos is unsuitable to pass the result back to the
caller.  That can happen in a 32->64 bit cross-compilation, for instance.
Incidentally, pbitpos is an int *, but it would have to be a HOST_WIDE_INT
to accomodate TREE_INT_CST_LOW (xindex)

sizetype, bitsizetype and DECL_FIELD_BITPOS

[Richard Kenner]

I'm a bit confused about the new bitsizetype and TYPE_SIZE_UNIT.

As I understand it, the purpose of this new code is to allow types and
objects to be 2**32 *bytes* long instead of 2**32 *bits* long (on
32-bit machines) that we used to allow.

So sizetype always measures bytes and bitsizetype always measures bits.
We have a TYPE_SIZE_UNIT to represent the size of a type in bits and
that's of type sizetype.

On 32-bit machines, as I understand it, sizetype will be 32 bits and
bitsizetype will need 64 bit arithmetic.

But what about DECL_FIELD_BITPOS?  That's used all the time when a fiekld
of a struct has variable size.  It used to be 32-bit, but now it looks
like it involves 64-bit arithmetic.  Is that what was intended?

Also, there is a comment referencing DECL_SIZE_UNIT, but I see no such
macros.  Then there's the code in get_inner_reference: first of all,
is OFFSET of bitsizetype or sizetype?  I see places where it is
used in both contexts and, unless I'm missing something, we are missing
conversions.  Secondly, the array code talks about if the bitsize
computation overflow, but I see no overflow test.

We've always said that DECL_FIELD_BITPOS was either or constant or
a variable possibily plus a constnat, where the variable was known
to be a multiple of BITS_PER_UNIT.  But now do we have to say that
it's a PLUS_EXPR of a conversion plus a constant and then change
teh code using it appropriately? I certainly don't think we want to
force all such computations to 64 bits.

Am I missing something here or are those bugs?