Re: [PATCH] include MEM_REF type in tree dumps (PR 90676)

[Richard Biener <richard.guenther@gmail.com>]

On Sat, Jun 1, 2019 at 5:53 PM Martin Sebor <msebor@gmail.com> wrote:
>
> I spent a bunch of time the other day trying to understand why
> the second of the two assignments below to a char array was
> apparently not being done by trunk
>
>    a[0] = 1;
>    a[1] = 0;
>
> The optimized GIMPLE dump simply shows:
>
>    MEM[(char *)&a] = 1;
>
> when in the past it showed:
>
>    MEM[(char[2] *)&a2] = 1;
>
> After some debugging I figured out that this is the result of
> the store merging pass transforming the two assignments into
> one:
>
>    *(short int *)a = 1;
>
> and the MEM_REF dump mentioning only the type of the second
> operand and not the type of the access.
>
> To avoid this confusion the attached patch adds to the dump
> a cast to the MEM_REF type for accesses whose size is not equal
> to the size of the operand (when the sizes are the same no new
> cast is prepended).  The effect is that with store merging in
> effect, the dump for the above becomes
>
>    MEM[(short int *)(char *)&a] = 1;

I think this is confusing syntax.  Iff you absolutely refuse to
make the -gimple dump the default for MEM_REF and you insist
on fixing this issue then please follow how we dump VIEW_CONVERT_EXPR
which is the only other tree code we dump the access type, thus

 MEM<short int *>[(char *)&a] = 1;

btw, -gimple (iff only applied to MEM_REF) would have dumped

 __MEM <short int, 8> ((char *)&a2) = 1;

also including the effective alignment of the access.  Your variant
suggests that the alignment is that of short int which is wrong.

Yes, there's testsuite fallout I guess mostly about () vs. [] and spacing
(the GIMPLE FE doesn't insist on spacing before/after <>).

So please do not change MEM_REF dumping in this way.

Richard.

> This should make both the size and the type of the access clear
> and help avoid the confusion.  The output isn't the same as in
> earlier releases because because the access really is done via
> a short pointer and not as an array of char.
>
> There is more detail in MEM_REF that could be included here but
> it seems that the size of the access is essential to interpreting
> the dumps.
>
> Tested on x86_64-linux with only minimal testsuite fallout.
>
> Martin