| Message ID | 95441782d1920f219d63dee1c82c7df68424d374.1713523124.git.oleksii.kurochko@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v3] xen/riscv: check whether the assembler has Zbb extension support | expand |
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@gmail.com> Acked-by: Jan Beulich <jbeulich@suse.com> 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
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 <oleksii.kurochko@gmail.com> > > Acked-by: Jan Beulich <jbeulich@suse.com> > 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. I am okay with all your suggestions. So the final version will look like ( if I understood you correctly ): diff --git a/xen/arch/riscv/arch.mk b/xen/arch/riscv/arch.mk index dd242c91d1..f172187144 100644 --- 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
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 <oleksii.kurochko@gmail.com> > > > > Acked-by: Jan Beulich <jbeulich@suse.com> > > 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. > 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. Thanks in advance. ~ Oleksii > diff --git a/xen/arch/riscv/arch.mk b/xen/arch/riscv/arch.mk > index dd242c91d1..f172187144 100644 > --- 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
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 <oleksii.kurochko@gmail.com> >>> >>> Acked-by: Jan Beulich <jbeulich@suse.com> >>> 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 >
diff --git a/xen/arch/riscv/arch.mk b/xen/arch/riscv/arch.mk index 53f3575e7d..dd242c91d1 100644 --- 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") extensions := $(zbb) $(zihintpause)
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@gmail.com> --- Changes in V3: - code style fixes: use "-" insteaf of "_" for names. - update the commit message. --- Changes in V2: - introduce check_extenstion to abstract a check of if extenstion is supported or not. - update the commit message. --- xen/arch/riscv/arch.mk | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-)