On 26.04.2024 10:26, Oleksii wrote: > On Mon, 2024-04-22 at 17:41 +0200, Oleksii wrote: >> On Mon, 2024-04-22 at 11:43 +0200, Jan Beulich wrote: >>> 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 <[email protected]> >>> >>> Acked-by: Jan Beulich <[email protected]> >>> 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.
Well, as per this v3 went in already. Hence ... >> I am okay with all your suggestions. So the final version will look >> like ( if I understood you correctly ): > Jan, > > Could you please review the changes below? I just want to ensure that > you are okay with them. If you are, I'll add your Acked-by that you > gave to this patch in previous answers. ... if you now want to further update the logic, it'll be a new patch anyway. The adjustments below look okay to me, but I'm not going to insist at this point that this be further fiddled with. Jan >> --- a/xen/arch/riscv/arch.mk >> +++ b/xen/arch/riscv/arch.mk >> @@ -13,12 +13,19 @@ riscv-generic-flags := $(riscv-abi-y) - >> march=$(riscv-march-y) >> >> # 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)) >> +# Usage: $(call check-extension,extension_name). >> +# it should be defined variable with name: extension-name := >> "insn" >> >> -zbb-insn := "andn t0, t0, t0" >> -zbb := $(call check-extension,zbb,$(zbb-insn)) >> -zihintpause := $(call check-extension,zihintpause,"pause") >> +define check-extension = >> +$(eval $(1) := \ >> + $(call as-insn,$(CC) $(riscv-generic-flags)_$(1),$(value >> $(1)- >> insn),_$(1))) >> +endef >> + >> +zbb-insn := "andn t0$(comma)t0$(comma)t0" >> +$(call check-extension,zbb) >> + >> +zihintpause-insn := "pause" >> +$(call check-extension,zihintpause) >> >> extensions := $(zbb) $(zihintpause) >> >> If the diff above looks good, I'll sent a new patch version. >> >> Thanks. >> >> ~ Oleksii >
