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.kuroc...@gmail.com>

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.)

Furthermore this extra variable suggests there's yet more room for
abstraction (as already suggested before).

> +zbb := $(call as-insn,$(CC) $(riscv-generic-flags)_zbb,${zbb_insn},_zbb)

Why figure braces in one case when everywhere else we use parentheses for
variable references? There's no functional difference sure, but inconsistent
use specifically may raise the question for some future reader whether there
actually is one.

Jan

Reply via email to