Re: [PATCH] xen/riscv: check whether the assembler has Zbb extension support

[Jan Beulich <jbeulich@xxxxxxxx>]

On 18.04.2024 15:09, Oleksii wrote:
> On Thu, 2024-04-18 at 12:00 +0200, Jan Beulich wrote:
>> On 09.04.2024 10:00, Oleksii Kurochko wrote:
>>> Update the argument of the as-insn for the Zbb case to verify that
>>> Zbb is supported not only by a compiler, but also by an assembler.
>>>
>>> Signed-off-by: Oleksii Kurochko <oleksii.kurochko@xxxxxxxxx>
>>
>> While technically all if fine here, I'm afraid I have a couple of
>> nits:
>>
>>> --- a/xen/arch/riscv/arch.mk
>>> +++ b/xen/arch/riscv/arch.mk
>>> @@ -11,7 +11,8 @@ riscv-march-$(CONFIG_RISCV_ISA_C)       :=
>>> $(riscv-march-y)c
>>>  
>>>  riscv-generic-flags := $(riscv-abi-y) -march=$(riscv-march-y)
>>>  
>>> -zbb := $(call as-insn,$(CC) $(riscv-generic-flags)_zbb,"",_zbb)
>>> +zbb_insn := "andn t0, t0, t0"
>>
>> As can be seen on the following line (as-insn, riscv-generic-flags)
>> we
>> prefer dashes over underscores in new variables' names. (Another
>> question is
>> whether the variable is needed in the first place, but that's pretty
>> surely
>> personal taste territory.)
> 
> It seems to me that we need it; otherwise, if we don't use the variable
> and put everything directly:
>   zbb := $(call as-insn,$(CC) $(riscv-generic-flags)_zbb,"andn t0, t0,
> t0",_zbb)
> Then as-insn will receive incorrect arguments because of the ',' used
> in the instruction. It will parse it as 3 separete arguments ("and, t0
> and t0"), which will lead to a compilation error:
>    /bin/sh: -c: line 1: unexpected EOF while looking for matching `''
>    /bin/sh: -c: line 2: syntax error: unexpected end of file
> 
> Probably I am missing something and it can be done in a different way.

Well, as said - this is perhaps largely personal taste. Yet technically
it isn't needed, assuming you're aware of the "comma" and "space" macros
that we have at the top of ./Config.mk. You'll find $(comma) used for
similar purposes in x86.

Jan