On Mon, 08 Jul 2019 21:14:36 +0100 Richard Sandiford <richard.sandif...@arm.com> wrote:
> Segher Boessenkool <seg...@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
