From: Jozef Lawrynowicz <jozef.l@mittosystems.com>
To: Richard Sandiford <richard.sandiford@arm.com>
Cc: Segher Boessenkool <segher@kernel.crashing.org>, gcc-patches@gcc.gnu.org
Subject: Re: [PATCH] Perform case-insensitive comparison when decoding register names (PR target/70320)
Date: Mon, 08 Jul 2019 21:42:00 -0000 [thread overview]
Message-ID: <20190708222129.59562c2e@jozef-kubuntu> (raw)
In-Reply-To: <mptv9wcqo5v.fsf@arm.com>
On Mon, 08 Jul 2019 21:14:36 +0100
Richard Sandiford <richard.sandiford@arm.com> wrote:
> Segher Boessenkool <segher@kernel.crashing.org> writes:
> > On Thu, Jul 04, 2019 at 01:32:59PM +0100, Jozef Lawrynowicz wrote:
> >> The attached patch allows the case of register names used in an asm statement
> >> clobber list to be disregarded when checking the validity of the register names.
> >>
> >> Currently, the register name used in asm statement clobber list must exactly
> >> match those defined in the targets REGISTER_NAMES macro.
> >
> >> All the register names defined in the msp430 REGISTER_NAMES macro use an
> >> upper case 'R', so use of lower case 'r' gets rejected.
> >>
> >> I guess this could also be fixed by defining all the registers in
> >> ADDITIONAL_REGISTER_NAMES using a lower case r, but I prefer this generic
> >> solution and I cannot think of any negative side effects to what is proposed.
> >
> > It isn't obviously safe either. Are there any targets that have names
> > for different registers that differ only in case? You could say that
> > such a design deserves what is coming for it, but :-)
Indeed, I did have a read through all the definitions of REGISTER_NAMES in the
gcc/config and could not spot any cases where different register nanes differed
only in their case. I didn't check it programmatically though, so it's
not impossible I missed something..
> >
> >> --- a/gcc/varasm.c
> >> +++ b/gcc/varasm.c
> >> @@ -947,7 +947,7 @@ decode_reg_name_and_count (const char *asmspec, int *pnregs)
> >
> > This is used for more than just clobber lists. Is this change safe, and
> > a good thing, in the other contexts where it changes things?
It appears to be used for only two purposes (mostly via the
"decode_reg_name" wrapper):
- Decoding the register name in an asm spec and reporting any misuse
- Decoding parameters passed to command line options
Generic options using it are -fcall-used/saved-REG and -ffixed-REG
-fstack-limit-register.
Backends use it for target specific options such as -mfixed-range= for SPU.
Apart from that there appears to be a single other use of it in make_decl_rtl
to report when "register name given for non-register variable", although I
could not immediately reproduce this myself to understand this specific case it
is triggered for.
> >
> >> - if (!strcmp (asmspec, "memory"))
> >> + if (!strcasecmp (asmspec, "memory"))
> >> return -4;
> >>
> >> - if (!strcmp (asmspec, "cc"))
> >> + if (!strcasecmp (asmspec, "cc"))
> >> return -3;
> >
> > You don't need to change these.
>
> Agreed. There's also the problem that if we make this available for
> all targets now, people might start using it without realising that
> it implicitly requires GCC 10+.
>
> But having the feature opt-in (by a DEFHOOKPOD?) sounds good, and
> definitely more maintainable than having to add aliases in the
> target code.
>
> Thanks,
> Richard
Ok, yes a DEFHOOKPOD or similar sounds like a good idea, I'll look into this
alternative.
Thanks,
Jozef
next prev parent reply other threads:[~2019-07-08 21:21 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-07-04 12:40 Jozef Lawrynowicz
2019-07-05 7:08 ` Segher Boessenkool
2019-07-08 20:36 ` Richard Sandiford
2019-07-08 21:42 ` Jozef Lawrynowicz [this message]
2019-07-08 21:44 ` Segher Boessenkool
2019-07-09 21:21 ` Jozef Lawrynowicz
2019-07-09 21:44 ` Segher Boessenkool
2019-07-10 16:37 ` Jozef Lawrynowicz
2019-07-10 16:53 ` Segher Boessenkool
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=20190708222129.59562c2e@jozef-kubuntu \
--to=jozef.l@mittosystems.com \
--cc=gcc-patches@gcc.gnu.org \
--cc=richard.sandiford@arm.com \
--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).