Re: [PATCH v2 1/2] Allow subtarget customization of CC1_SPEC

[Richard Sandiford <richard.sandiford@arm.com>]

Sebastian Huber <sebastian.huber@embedded-brains.de> writes:
> On 06.12.22 22:06, Thomas Schwinge wrote:
>> Hi!
>> 
>> I suppose I just fail to see some detail here, but:
>> 
>> On 2022-11-21T08:25:25+0100, Sebastian Huber<sebastian.huber@embedded-brains.de>  wrote:
>>> gcc/ChangeLog:
>>>
>>>        * gcc.cc (SUBTARGET_CC1_SPEC): Define if not defined.
>>>        (cc1_spec): Append SUBTARGET_CC1_SPEC.
>>> ---
>>> v2: Append SUBTARGET_CC1_SPEC directly to cc1_spec and not through CC1_SPEC.
>>>      This avoids having to modify all the CC1_SPEC definitions in the targets.
>>>
>>>   gcc/gcc.cc | 9 ++++++++-
>>>   1 file changed, 8 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/gcc/gcc.cc b/gcc/gcc.cc
>>> index 830ab88701f..4e1574a4df1 100644
>>> --- a/gcc/gcc.cc
>>> +++ b/gcc/gcc.cc
>>> @@ -706,6 +706,13 @@ proper position among the other output files.  */
>>>   #define CPP_SPEC ""
>>>   #endif
>>>
>>> +/* Subtargets can define SUBTARGET_CC1_SPEC to provide extra args to cc1 and
>>> +   cc1plus or extra switch-translations.  The SUBTARGET_CC1_SPEC is appended
>>> +   to CC1_SPEC.  */
>>> +#ifndef SUBTARGET_CC1_SPEC
>>> +#define SUBTARGET_CC1_SPEC ""
>>> +#endif
>>> +
>>>   /* config.h can define CC1_SPEC to provide extra args to cc1 and cc1plus
>>>      or extra switch-translations.  */
>>>   #ifndef CC1_SPEC
>>> @@ -1174,7 +1181,7 @@ proper position among the other output files.  */
>>>   static const char *asm_debug = ASM_DEBUG_SPEC;
>>>   static const char *asm_debug_option = ASM_DEBUG_OPTION_SPEC;
>>>   static const char *cpp_spec = CPP_SPEC;
>>> -static const char *cc1_spec = CC1_SPEC;
>>> +static const char *cc1_spec = CC1_SPEC SUBTARGET_CC1_SPEC;
>>>   static const char *cc1plus_spec = CC1PLUS_SPEC;
>>>   static const char *link_gcc_c_sequence_spec = LINK_GCC_C_SEQUENCE_SPEC;
>>>   static const char *link_ssp_spec = LINK_SSP_SPEC;
>> ... doesn't this (at least potentially?) badly interact with any existing
>> 'SUBTARGET_CC1_SPEC' definitions -- which pe rabove get appended to
>> 'cc1_spec'?
>> 
>>      gcc/config/loongarch/gnu-user.h-   and provides this hook instead.  */
>>      gcc/config/loongarch/gnu-user.h:#undef SUBTARGET_CC1_SPEC
>>      gcc/config/loongarch/gnu-user.h:#define SUBTARGET_CC1_SPEC GNU_USER_TARGET_CC1_SPEC
>>      gcc/config/loongarch/gnu-user.h-
>>      --
>>      gcc/config/loongarch/loongarch.h-#define EXTRA_SPECS \
>>      gcc/config/loongarch/loongarch.h:  {"subtarget_cc1_spec", SUBTARGET_CC1_SPEC}, \
>>      gcc/config/loongarch/loongarch.h-  {"subtarget_cpp_spec", SUBTARGET_CPP_SPEC}, \
>>      --
>>      gcc/config/mips/gnu-user.h-   and provides this hook instead.  */
>>      gcc/config/mips/gnu-user.h:#undef SUBTARGET_CC1_SPEC
>>      gcc/config/mips/gnu-user.h:#define SUBTARGET_CC1_SPEC GNU_USER_TARGET_CC1_SPEC
>>      gcc/config/mips/gnu-user.h-
>>      --
>>      gcc/config/mips/linux-common.h-
>>      gcc/config/mips/linux-common.h:#undef  SUBTARGET_CC1_SPEC
>>      gcc/config/mips/linux-common.h:#define SUBTARGET_CC1_SPEC                                           \
>>      gcc/config/mips/linux-common.h-  LINUX_OR_ANDROID_CC (GNU_USER_TARGET_CC1_SPEC,                     \
>>      --
>>      gcc/config/mips/mips.h-
>>      gcc/config/mips/mips.h:/* SUBTARGET_CC1_SPEC is passed to the compiler proper.  It may be
>>      gcc/config/mips/mips.h-   overridden by subtargets.  */
>>      gcc/config/mips/mips.h:#ifndef SUBTARGET_CC1_SPEC
>>      gcc/config/mips/mips.h:#define SUBTARGET_CC1_SPEC ""
>>      gcc/config/mips/mips.h-#endif
>>      --
>>      gcc/config/mips/mips.h-#define EXTRA_SPECS                                                  \
>>      gcc/config/mips/mips.h:  { "subtarget_cc1_spec", SUBTARGET_CC1_SPEC },                              \
>>      gcc/config/mips/mips.h-  { "subtarget_cpp_spec", SUBTARGET_CPP_SPEC },                              \
>>      --
>>      gcc/config/mips/r3900.h-/* By default (if not mips-something-else) produce code for the r3900 */
>>      gcc/config/mips/r3900.h:#undef SUBTARGET_CC1_SPEC
>>      gcc/config/mips/r3900.h:#define SUBTARGET_CC1_SPEC "\
>>      gcc/config/mips/r3900.h-%{mhard-float:%e-mhard-float not supported} \
>
> Oh, I came up with the name SUBTARGET_CC1_SPEC after a discussion on the 
> mailing list and I have to admit that I didn't check that it was 
> actually already in use. What about renaming the loongarch/mips define 
> to LOONGARCH_CC1_SPEC and MIPS_CC1_SPEC?

One drawback to that is that people might have their own spec files
that reference the existing names.  Typically those would be produced
by using -dumpspecs and editing the output.  I think we only really
support that if people regenerate the specs files for each release,
but for targets like MIPS that don't see much activity, it's probably
easy to get away without doing that.

How about going back to Jose's suggestion from the original thread
of using OS_CC1_SPEC?  The patch is OK with that change if no-one
objects in 24 hours.

Thanks,
Richard