Re: [PATCH] rs6000: Fix invalid address passed to __builtin_mma_disassemble_acc [PR104923]

Re: [PATCH] rs6000: Fix invalid address passed to __builtin_mma_disassemble_acc [PR104923]

[Peter Bergner]

I forgot to CC the gcc-patches mailing list on the original patch submission.
Adding it in now.  Sorry.



On 3/14/22 8:24 PM, Segher Boessenkool wrote:
>> gcc/
>>     PR target/104923
>>     * config/rs6000/predicates.md (mma_disassemble_output_operand):
>>     Restrict acceptable MEM addresses.
>>
>> gcc/testsuite/
>>     PR target/104923
>>     * gcc.target/powerpc/pr104923.c: New test.
> 
> Changelogs are indented with tabs, not with four spaces.  Maybe the
> commit script will fix this up though, dunno.

They are actually indented correctly.  I think when I was fixing one
of my Thunderbird setting for my IBM account, it reverted my settings
to always send in plain-text and probably munged the whitespace on
me.  I think I fixed that now.  Thanks for letting me know it was
broken.




> Including that tab the maximum line length is 80.  You don't have to end
> that first line with a colon (it looks like something is missing like
> this)

Ok, I can move "Restrict" to the line above and still stay within 80 chars.



>> +  if (MEM_P (op))
>> +    return indexed_or_indirect_address (XEXP (op, 0), mode)
>> +       || quad_address_p (XEXP (op, 0), mode, false);
> 
> The indent of this last line is wrong.  The || should align with
> indexed_or_indirect_address, and every leading eight spaces is a tab.

Again, it's indented correctly for me, but got munged when sending.
Hopefully fixed now.


> You might want to name that common expression, "rtx addr = XEXP (op, 0);"
> or something.  Dunno what is best

Will do.



> Please put that new MEM_P code first, followed by a blank line, and only
> then do the SUBREG thing.  As written it will allow subregs of mem.  And
> the blank line is important of course ;-)

Will do.


> Okay for trunk with those changes.  Also okay for 10 and 11 after an
> appropriate soak period.  Thanks!

Thanks.  I'll verify things are still working with the changes agreed
to above before committing.  Thanks.


> Btw.  A good cleanup would be to have mma_assemble_input_operand written
> like this, too?

Ok, I'll have a look at doing that as part of a separate change.

Peter

Re: [PATCH] rs6000: Fix invalid address passed to __builtin_mma_disassemble_acc [PR104923]

[Peter Bergner]

On 3/14/22 10:06 PM, Peter Bergner wrote:
> On 3/14/22 8:24 PM, Segher Boessenkool wrote:
>> You might want to name that common expression, "rtx addr = XEXP (op, 0);"
>> or something.  Dunno what is best
> 
> Will do.
> 
> 
>> Please put that new MEM_P code first, followed by a blank line, and only
>> then do the SUBREG thing.  As written it will allow subregs of mem.  And
>> the blank line is important of course ;-)
> 
> Will do.
> 
> 
>> Okay for trunk with those changes.  Also okay for 10 and 11 after an
>> appropriate soak period.  Thanks!

Testing of the updates came back clean so I pushed the fix.
I'll wait a few days before pushing the backports.  Thanks!

Peter