On 6/3/19 4:34 AM, Richard Biener wrote:
On Mon, Jun 3, 2019 at 10:57 AM Jakub Jelinek <ja...@redhat.com> wrote:
On Mon, Jun 03, 2019 at 10:36:42AM +0200, Richard Biener wrote:
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
I must say I prefer the current MEM[ over the -gimple for human readable
dumps.
Sure, but then why ask for all information to be present when in the cases
you are curious you can look at -gimple dumps? A similar thing I've
hacked the pretty printer locally for debugging in the past is alignment info.
MEM<short int *>[(char *)&a] = 1;
Wouldn't that be
MEM<short int>[(char *)&a] instead?
Err, yes.
Couldn't we do it only if the TREE_TYPE (TREE_TYPE (TREE_OPERAND (mem, 1)))
is not compatible with TREE_TYPE (mem), so keep what we were doing in most
cases?
We could. Like we dump MEM_REF as * in some cases.
The question is still why fix things half-way if a complete solution
is already there?
Because it restores the important detail for those of us who
are accustomed to the "legacy" format. That's without a doubt
the majority of users. Note that godbolt.org only exposes
the classic dumps and doesn't make it possible to select
the -gimple form.
Those with a preference for the -gimple syntax are presumably
already using the -gimple dumps so they shouldn't be bothered
by a change to the legacy format.
But those with a preference for the traditional syntax will
not appreciate having the syntax changed. Scripts that parse
those dumps (like GCC's own test harness) have a reasonable
chance of continuing to be able to parse the syntax even with
the additional cast. They will certainly not be able to parse
it if it changes to MEM<T>(...)).
But if you refuse to accept the patch as is and insist on
the syntax with the pointy brackets please let me know. I
think it's more important get the size of the access restored
than the details of the syntax so I'm willing to spend the time
to adjust the fix, even at the risk of breaking scripts and
making some users unhappy.
Martin
Btw, VIEW_CONVERT dumping uses () instead of [], that I used
[] when I introduced MEM_REF was probably a mistake...
Is it just the parens kind you dislike?
Richard.
Jakub
