From: Peter Bergner <bergner@linux.ibm.com>
To: Segher Boessenkool <segher@kernel.crashing.org>
Cc: David Edelsohn <dje.gcc@gmail.com>,
GCC Patches <gcc-patches@gcc.gnu.org>
Subject: Re: [PATCH] rs6000: Fix invalid address passed to __builtin_mma_disassemble_acc [PR104923]
Date: Mon, 14 Mar 2022 22:06:35 -0500 [thread overview]
Message-ID: <d8f39e2d-832f-210d-1a86-e8b0daabab28@linux.ibm.com> (raw)
In-Reply-To: <20220315012425.GH614@gate.crashing.org>
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
next parent reply other threads:[~2022-03-15 3:06 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <a3589644-3eb8-aca4-8b58-44383e8bf529@linux.ibm.com>
[not found] ` <20220315012425.GH614@gate.crashing.org>
2022-03-15 3:06 ` Peter Bergner [this message]
2022-03-15 13:53 ` Peter Bergner
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=d8f39e2d-832f-210d-1a86-e8b0daabab28@linux.ibm.com \
--to=bergner@linux.ibm.com \
--cc=dje.gcc@gmail.com \
--cc=gcc-patches@gcc.gnu.org \
--cc=segher@kernel.crashing.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).