MEM_REF representation problem, and folding fix

Currently, MEM_REF contains two pointer arguments, one which is supposed
to be a base object and another which is supposed to be a constant
offset. This representation is somewhat problematic, as not all machines
treat pointer values as essentially integers. On machines where size_t
is smaller than a pointer, for example m32c where it's due to
limitations in the compiler, or the port I've been working on recently
where pointers contain a segment selector that does not participate in
additions, this is not an accurate representation, and it does cause
real issues.

It would be better to use a representation more like POINTER_PLUS with a
pointer and a real sizetype integer. Can someone explain the comment in
tree.def which states that the type of the constant offset is used for
TBAA purposes? It states "MEM_REF <p, c> is equivalent to
((typeof(c))p)->x [...]", so why not represent it as MEM_REF <(desired
type)p, (size_t)c>?

The following patch works around one instance of the problem. When we
fold an offset addition, the addition must be performed in sizetype,
otherwise we may get unwanted overflow. This bug triggers on m32c for
example, where an offset of 65528 (representing -8) and and offset of 8
are added, yielding an offset of 65536 instead of zero. Solved by
performing the intermediate computation in sizetype.

Bootstrapped and tested on x86_64-linux (all languages except Ada) with
no changes in the tests, and tested on m32c-elf where it fixes 22
failures. Ok?


Bernd

On Mon, Apr 29, 2013 at 11:20 PM, Bernd Schmidt <bernds@codesourcery.com> wrote:
> Currently, MEM_REF contains two pointer arguments, one which is supposed
> to be a base object and another which is supposed to be a constant
> offset. This representation is somewhat problematic, as not all machines
> treat pointer values as essentially integers. On machines where size_t
> is smaller than a pointer, for example m32c where it's due to
> limitations in the compiler, or the port I've been working on recently
> where pointers contain a segment selector that does not participate in
> additions, this is not an accurate representation, and it does cause
> real issues.
>
> It would be better to use a representation more like POINTER_PLUS with a
> pointer and a real sizetype integer. Can someone explain the comment in
> tree.def which states that the type of the constant offset is used for
> TBAA purposes? It states "MEM_REF <p, c> is equivalent to
> ((typeof(c))p)->x [...]", so why not represent it as MEM_REF <(desired
> type)p, (size_t)c>?

Because if p is not of desired type then we have to emit a separate
stmt with a type conversion (which are all useless now, btw).  Initially
I played with having an extra type operand, but all hell breaks lose
if you have tree->exp.operand[] be a TREE_TYPE.  So I settled for
the convenient place of using the constants type.

> The following patch works around one instance of the problem. When we
> fold an offset addition, the addition must be performed in sizetype,
> otherwise we may get unwanted overflow. This bug triggers on m32c for
> example, where an offset of 65528 (representing -8) and and offset of 8
> are added, yielding an offset of 65536 instead of zero. Solved by
> performing the intermediate computation in sizetype.

Ah, yes.  Note that this is why we have mem_ref_offset () in tree.c.

So a better fix would be to use

     return fold_build2 (MEM_REF, type,
            build_fold_addr_expr (base),
            double_int_to_tree (type1, tree_to_double_int (arg1).sext
(TYPE_PRECISION (TREE_TYPE (arg1))) + coffset));

that is, perform the arithmetic using double_int.  Note that

+         arg1 = fold_convert (size_type_node, arg1);

would no longer sign-extend arg1, you'd need to use ssize_type_node.
And using [s]size_type_node for the offset would wreck targets that
have 16bit sizetype and 24bit pointer type (it would truncate parts of the
offset).

Richard.

> Bootstrapped and tested on x86_64-linux (all languages except Ada) with
> no changes in the tests, and tested on m32c-elf where it fixes 22
> failures. Ok?

>
> Bernd