Re: [PATCH] Allow case-insensitive comparisons of register names by implementing CASE_INSENSITIVE_REGISTER_NAMES PR target/70320

[Richard Sandiford]

Jozef Lawrynowicz <joze...@mittosystems.com> writes:
> The attached patch adds a new target macro called 
> CASE_INSENSITIVE_REGISTER_NAMES, which allows the case of register names
> used in an asm statement clobber list, or given in a command line option, to 
> be
> disregarded when comparing with the register names defined for the target in
> REGISTER_NAMES. 
>
> The macro is set to 1 for msp430 only, and set to 0 by default, so
> comparisons continue to be case-sensitive for all targets except
> msp430.
>
> Previously, a register name provided by the user using one of the
> aforementioned methods must exactly match those defined in the targets
> REGISTER_NAMES macro.
>
> This means that, for example, for msp430-elf the following code emits an
> ambiguous error:
>
>> void
>> foo (void)
>> {
>>   __asm__ ("" : : : "r4", "R6");
>> }
>
>> asm-register-names.c:8:3: error: unknown register name 'r4' in 'asm'
>
> All the register names defined in the msp430 REGISTER_NAMES macro use an
> upper case 'R', so use of lower case 'r' gets rejected.
>
> Successfully bootstrapped and regtested on trunk for x86_64-pc-linux-gnu, and
> regtested for msp430-elf.
>
> Ok for trunk?
>
> From 82eadcdcbb8914b06818f7c8a10156336518e8d1 Mon Sep 17 00:00:00 2001
> From: Jozef Lawrynowicz <joze...@mittosystems.com>
> Date: Wed, 17 Jul 2019 11:48:23 +0100
> Subject: [PATCH] Implement CASE_INSENSITIVE_REGISTER_NAMES
>
> gcc/ChangeLog:
>
> 2019-07-18  Jozef Lawrynowicz  <joze...@mittosystems.com>
>
>       PR target/70320
>       * doc/tm.texi.in: Document new macro CASE_INSENSITIVE_REGISTER_NAMES.
>       * doc/tm.texi: Likewise.
>       * defaults.h: Define CASE_INSENSITIVE_REGISTER_NAMES to 0.
>       * config/msp430/msp430.h: Define CASE_INSENSITIVE_REGISTER_NAMES to 1.
>       * varasm.c (decode_reg_name_and_count): Use strcasecmp instead of
>       strcmp for comparisons of asmspec with a register name if 
>       CASE_INSENSITIVE_REGISTER_NAMES is defined to 1.

I really don't think we should be adding new target macros for things
like this.  The code is hardly on the critical path, so I don't think
compile time is a concern.  That said...

> diff --git a/gcc/varasm.c b/gcc/varasm.c
> index e886cdc71b8..ab04bc2c332 100644
> --- a/gcc/varasm.c
> +++ b/gcc/varasm.c
> @@ -947,7 +947,12 @@ decode_reg_name_and_count (const char *asmspec, int 
> *pnregs)
>  
>        for (i = 0; i < FIRST_PSEUDO_REGISTER; i++)
>       if (reg_names[i][0]
> -         && ! strcmp (asmspec, strip_reg_name (reg_names[i])))
> +#if CASE_INSENSITIVE_REGISTER_NAMES
> +         && ! strcasecmp (asmspec, strip_reg_name (reg_names[i]))
> +#else
> +         && ! strcmp (asmspec, strip_reg_name (reg_names[i]))
> +#endif /* CASE_INSENSITIVE_REGISTER_NAMES */
> +     )
>         return i;
>  
>  #ifdef OVERLAPPING_REGISTER_NAMES

...if we do keep it as a macro, we should use:

        if (reg_names[i][0]
            && (CASE_INSENSITIVE_REGISTER_NAMES
                ? !strcasecmp (asmspec, strip_reg_name (reg_names[i]))
                : !strcmp (asmspec, strip_reg_name (reg_names[i]))))

So TBH I still prefer the DEFHOOKPOD suggestion.  I won't object if
someone else wants to approve the macro version though.

Thanks,
Richard