Re: MEM_REF representation problem, and folding fix

[Richard Biener <richard dot guenther at gmail dot com>]

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