Re: [PATCH v2 1/3] x86/retpoline: rename retpoline Kconfig check to include GCC prefix

[Jan Beulich <jbeulich@xxxxxxxx>]

On 17.02.2022 11:37, Roger Pau Monné wrote:
> On Thu, Feb 17, 2022 at 10:04:01AM +0100, Jan Beulich wrote:
>> On 17.02.2022 09:59, Jan Beulich wrote:
>>> On 16.02.2022 17:21, Roger Pau Monne wrote:
>>>> Current retpoline checks apply to GCC only, so make it obvious by
>>>> prefixing the Kconfig option with GCC. Keep the previous option as a
>>>> way to signal generic retpoline support regardless of the underlying
>>>> compiler.
>>>>
>>>> No functional change intended.
>>>>
>>>> Signed-off-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>
>>>> Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>
>>>> ---
>>>> Changes since v1:
>>>>  - Put def_bool before depend on.
>>>
>>> Just for the record: A slightly shorter alternative would have been ...
>>>
>>>> --- a/xen/arch/x86/Kconfig
>>>> +++ b/xen/arch/x86/Kconfig
>>>> @@ -32,9 +32,13 @@ config ARCH_DEFCONFIG
>>>>    string
>>>>    default "arch/x86/configs/x86_64_defconfig"
>>>>  
>>>> -config INDIRECT_THUNK
>>>> +config GCC_INDIRECT_THUNK
>>>>    def_bool $(cc-option,-mindirect-branch-register)
>>>>  
>>>> +config INDIRECT_THUNK
>>>> +  def_bool y
>>>> +  depends on GCC_INDIRECT_THUNK
>>>
>>> config INDIRECT_THUNK
>>>     bool
>>>
>>> config GCC_INDIRECT_THUNK
>>>     def_bool $(cc-option,-mindirect-branch-register)
>>>     select INDIRECT_THUNK
>>
>> Oh, looking at patch 3 again (which I should have still had in mind)
>> this would of course not help. Yet ..
>>
>>> A more appropriate thing to use for "depends on" might have been
>>> CC_IS_GCC. With the next patch in mind this would then avoid
>>> potential confusion if e.g. Clang folks decided to (also) support
>>> the gcc style command line options.
>>
>> ... adding this dependency (and a respective one in patch 2) might
>> still be a good thing.
> 
> So you would like to make GCC_INDIRECT_THUNK depend on CC_IS_GCC?

It would seem more clean to me, but ...

> That would be fine, but I think it's extremely unlikely for CLANG to
> adopt the GCC options - I've already mentioned at implementation time
> and they refused.

... I'm not going to insist.

Jan