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

[Martin Sebor]

The attached update adds the MEM_REF type in pointy braces like
in the -gimple format, but without the access size:

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

Martin

On 6/4/19 4:57 AM, Richard Biener wrote:
> On Mon, Jun 3, 2019 at 5:13 PM Martin Sebor <msebor@gmail.com> wrote:
>>
>> On 6/3/19 4:34 AM, Richard Biener wrote:
>>> On Mon, Jun 3, 2019 at 10:57 AM Jakub Jelinek <jakub@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.
> 
> I think introducing inconsistencies (and I find two "casts"
> confusing as well) with existing VIEW_CONVERT_EXPR
> dumping isn't good.  So yes, I'd rather prefer
> 
> MEM <access-type> [(alias-pointer-type) ptr]
> 
> note access-type isn't a pointer type to the access type.
> 
> I can definitely live with this incremental but consistent change.
> Also consider eliding access-type dumping as Jakub suggested
> (when equal to *alias-pointer-type).
> 
> As said in the PR dump format changes have the chance
> to make testcases testing for sth _not_ to appear no longer
> testing what they want to test for (one reason those testcases
> are broken).  A quick grep for MEM on a scan-*-dump-not
> might reveal candidates that could need a second look.
> 
> Note that it was pure laziness on my side that I didn't change
> the "legacy" format for the way the GIMPLE FE likes it :/
> And I feel sorry about that.
> 
> Richard.
> 
>> 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
>>

-------------- next part --------------
A non-text attachment was scrubbed...
Name: gcc-90676.diff
Type: text/x-patch
Size: 17356 bytes
Desc: not available
URL: <http://gcc.gnu.org/pipermail/gcc-patches/attachments/20190610/1a07a8bf/attachment.bin>