* Re: [PATCH] rs6000: Fix invalid address passed to __builtin_mma_disassemble_acc [PR104923]
[not found] ` <20220315012425.GH614@gate.crashing.org>
@ 2022-03-15 3:06 ` Peter Bergner
2022-03-15 13:53 ` Peter Bergner
0 siblings, 1 reply; 2+ messages in thread
From: Peter Bergner @ 2022-03-15 3:06 UTC (permalink / raw)
To: Segher Boessenkool; +Cc: David Edelsohn, GCC Patches
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
^ permalink raw reply [flat|nested] 2+ messages in thread