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

[Jan Beulich <jbeulich@xxxxxxxx>]

On 19.04.2024 16:23, 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.
> 
> Also, check-extenstion(ext_name, "insn") helper macro is introduced
> to check whether extension is supported by a compiler and an assembler.
> 
> Signed-off-by: Oleksii Kurochko <oleksii.kurochko@xxxxxxxxx>

Acked-by: Jan Beulich <jbeulich@xxxxxxxx>
despite ...

> --- a/xen/arch/riscv/arch.mk
> +++ b/xen/arch/riscv/arch.mk
> @@ -11,9 +11,14 @@ 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)
> -zihintpause := $(call as-insn, \
> -                      $(CC) 
> $(riscv-generic-flags)_zihintpause,"pause",_zihintpause)
> +# check-extension: Check whether extenstion is supported by a compiler and
> +#                  an assembler.
> +# Usage: $(call check-extension,extension_name,"instr")
> +check-extension = $(call as-insn,$(CC) 
> $(riscv-generic-flags)_$(1),$(2),_$(1))
> +
> +zbb-insn := "andn t0, t0, t0"
> +zbb := $(call check-extension,zbb,$(zbb-insn))
> +zihintpause := $(call check-extension,zihintpause,"pause")

... still not really being happy with this: Either, as said before, zbb-insn
would better be avoided (by using $(comma) as necessary), or you should have
gone yet a step further to fully address my "redundancy" concern. Note how
the two ultimate lines still have 3 (zbb) and 2 (zihintpause) references
respectively, when the goal ought to be to have exactly one. E.g. along the
lines of

$(call check-extension,zbb)
$(call check-extension,zihintpause)

suitably using $(eval ...) (to effect the variable definitions) and defining
both zbb-insn and zihintpause-insn.

But I'll nevertheless put this in as is, unless you tell me you're up to
going the extra step.

Jan