* [RFC PATCH 0/1] RISC-V: Implement "extension variants" for diagnostics @ 2022-10-01 5:27 Tsukasa OI 2022-10-01 5:27 ` [RFC PATCH 1/1] RISC-V: Implement extension variants Tsukasa OI 2022-10-01 7:21 ` [RFC PATCH 0/1] RISC-V: Implement "extension variants" for diagnostics Nelson Chu 0 siblings, 2 replies; 6+ messages in thread From: Tsukasa OI @ 2022-10-01 5:27 UTC (permalink / raw) To: Tsukasa OI, Nelson Chu, Kito Cheng, Palmer Dabbelt; +Cc: binutils Hi all RISC-V folks, GitHub tracker: <https://github.com/a4lg/binutils-gdb/wiki/riscv_insn_ext_variants> While investigating possible implementation of both 'Z[fdq]inx' and 'P' implementations, I found something common (not just register pairs). Here, this patch implements what I call "extension variants". It was formerly a part of 'Zfinx' fixes (formerly the flag was named "INSN_F_OR_X"). If there is a instruction with multiple variants with different requirements and the assembler fails to parse all variants, there is a case that needs to refer ALL variants to generate proper diagnostics. If an instruction with "INSN_HAS_EXT_VARS" fails on all variants, the assembler now has a chance to modify the instruction class for proper diagnostics. A typical use of this feature is to select wider instruction class when necessary. [Usage: CLZ instruction ('Zbpbo')] To implement proposed 'Zbpbo' extension, updating instruction class for CLZ instruction is not enough. If we change CLZ instruction class from "INSN_CLASS_ZBB" to "INSN_CLASS_ZBB_OR_ZBPBO", we will **mistakenly** support that instruction on RV64_Zbpbo because CLZ on 'Zbpbo' is RV32-only. Possible use with "INSN_HAS_EXT_VARS" is to define two variants of CLZ instruction with that flag: 1. 'Zbb' (RV32/RV64) 2. 'Zbpbo' (RV32) If both fails, we can widen the instruction class from "INSN_CLASS_ZBPBO" to "INSN_CLASS_ZBB_OR_ZBPBO" if XLEN is 32. After doing that, we will get following diagnostics: - On RV32: 'Zbb' or 'Zbpbo' is required - On RV64: 'Zbb' is required [Usage: 'D'/'Zdinx' or 'Q'/'Zqinx'] Due to use of register pairs, we need to split 'D'/'Q' and 'Zdinx'/'Zqinx' variants. For instance, if parsing "fmin.d" fails on both 'D' and 'Zdinx' variants, we have to require 'D' or 'Zdinx', not just 'Zdinx', the last "fmin.d" variant in riscv_opcodes. 1. 'D' 2. 'Zdinx' If both fails, we can widen the instruction class from "INSN_CLASS_ZDINX" to "INSN_CLASS_D_OR_ZDINX". After doing that, we will get diagnostics saying 'D' or 'Zdinx' is required. Thanks, Tsukasa Tsukasa OI (1): RISC-V: Implement extension variants gas/config/tc-riscv.c | 27 +++++++++++++++++++++++++-- include/opcode/riscv.h | 5 +++++ 2 files changed, 30 insertions(+), 2 deletions(-) base-commit: b4477c7f666bdeb7f8e998633c7b0cb62310b9ef -- 2.34.1 ^ permalink raw reply [flat|nested] 6+ messages in thread
* [RFC PATCH 1/1] RISC-V: Implement extension variants 2022-10-01 5:27 [RFC PATCH 0/1] RISC-V: Implement "extension variants" for diagnostics Tsukasa OI @ 2022-10-01 5:27 ` Tsukasa OI 2022-10-01 7:21 ` [RFC PATCH 0/1] RISC-V: Implement "extension variants" for diagnostics Nelson Chu 1 sibling, 0 replies; 6+ messages in thread From: Tsukasa OI @ 2022-10-01 5:27 UTC (permalink / raw) To: Tsukasa OI, Nelson Chu, Kito Cheng, Palmer Dabbelt; +Cc: binutils If there is a instruction with multiple variants with different requirements and the assembler fails to parse all variants, there is a case that needs to refer ALL variants to generate proper diagnostics. If an instruction with INSN_HAS_EXT_VARS fails on all variants, the assembler now has a chance to modify the instruction class for proper diagnostics. A typical use of this feature is to select _wider_ instruction class when necessary. gas/ChangeLog: * config/tc-riscv.c (riscv_ip): Add empty INSN_HAS_EXT_VARS handling. include/ChangeLog: * opcode/riscv.h (INSN_HAS_EXT_VARS): New. --- gas/config/tc-riscv.c | 27 +++++++++++++++++++++++++-- include/opcode/riscv.h | 5 +++++ 2 files changed, 30 insertions(+), 2 deletions(-) diff --git a/gas/config/tc-riscv.c b/gas/config/tc-riscv.c index 5e6fca3de9f..f83b0ff3dad 100644 --- a/gas/config/tc-riscv.c +++ b/gas/config/tc-riscv.c @@ -2376,6 +2376,7 @@ riscv_ip (char *str, struct riscv_cl_insn *ip, expressionS *imm_expr, unsigned int regno; const struct percent_op_match *p; struct riscv_ip_error error; + enum riscv_insn_class insn_class; error.msg = "unrecognized opcode"; error.statement = str; error.missing_ext = NULL; @@ -2402,8 +2403,30 @@ riscv_ip (char *str, struct riscv_cl_insn *ip, expressionS *imm_expr, if (!riscv_multi_subset_supports (&riscv_rps_as, insn->insn_class)) { - error.missing_ext = riscv_multi_subset_supports_ext (&riscv_rps_as, - insn->insn_class); + insn_class = insn->insn_class; + if (insn->pinfo != INSN_MACRO && insn->pinfo & INSN_HAS_EXT_VARS) + { + /* If an instruction is not supported for example, there is a + case where some extensions (that is not related to the last + "same name" variants) should be printed for diagnostics. + + For instance, if parsing "fmin.d" fails on both 'D' and + 'Zdinx' variants, we have to require 'D' or 'Zdinx', not just + 'Zdinx', the last "fmin.d" variant in riscv_opcodes. + + For instructions with INSN_HAS_EXT_VARS, we should rewrite the + instruction class (from a specific one to a generic one) + to provide proper error output. + */ + switch (insn_class) + { + default: + break; + } + } + if (!riscv_multi_subset_supports (&riscv_rps_as, insn_class)) + error.missing_ext = riscv_multi_subset_supports_ext (&riscv_rps_as, + insn_class); continue; } diff --git a/include/opcode/riscv.h b/include/opcode/riscv.h index dd2569f6d55..3cfe132babe 100644 --- a/include/opcode/riscv.h +++ b/include/opcode/riscv.h @@ -496,6 +496,11 @@ struct riscv_opcode #define INSN_8_BYTE 0x00000040 #define INSN_16_BYTE 0x00000050 +/* Instruction has different entry that shares the name but differs + in extension requirements (extension variants). Those instructions must be + taken care if we should print an "extension required" error. */ +#define INSN_HAS_EXT_VARS 0x00000080 + /* Instruction is actually a macro. It should be ignored by the disassembler, and requires special treatment by the assembler. */ #define INSN_MACRO 0xffffffff -- 2.34.1 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFC PATCH 0/1] RISC-V: Implement "extension variants" for diagnostics 2022-10-01 5:27 [RFC PATCH 0/1] RISC-V: Implement "extension variants" for diagnostics Tsukasa OI 2022-10-01 5:27 ` [RFC PATCH 1/1] RISC-V: Implement extension variants Tsukasa OI @ 2022-10-01 7:21 ` Nelson Chu 2022-10-01 8:25 ` Tsukasa OI 1 sibling, 1 reply; 6+ messages in thread From: Nelson Chu @ 2022-10-01 7:21 UTC (permalink / raw) To: Tsukasa OI; +Cc: Kito Cheng, Palmer Dabbelt, binutils On Sat, Oct 1, 2022 at 1:27 PM Tsukasa OI <research_trasio@irq.a4lg.com> wrote: > > Hi all RISC-V folks, > > GitHub tracker: > <https://github.com/a4lg/binutils-gdb/wiki/riscv_insn_ext_variants> > > While investigating possible implementation of both 'Z[fdq]inx' and 'P' > implementations, I found something common (not just register pairs). > > Here, this patch implements what I call "extension variants". > It was formerly a part of 'Zfinx' fixes (formerly the flag was named > "INSN_F_OR_X"). > > If there is a instruction with multiple variants with different requirements > and the assembler fails to parse all variants, there is a case that needs to > refer ALL variants to generate proper diagnostics. > > If an instruction with "INSN_HAS_EXT_VARS" fails on all variants, the > assembler now has a chance to modify the instruction class for proper > diagnostics. A typical use of this feature is to select wider instruction > class when necessary. > > > [Usage: CLZ instruction ('Zbpbo')] > > To implement proposed 'Zbpbo' extension, updating instruction class for > CLZ instruction is not enough. If we change CLZ instruction class from > "INSN_CLASS_ZBB" to "INSN_CLASS_ZBB_OR_ZBPBO", we will **mistakenly** > support that instruction on RV64_Zbpbo because CLZ on 'Zbpbo' is RV32-only. > > Possible use with "INSN_HAS_EXT_VARS" is to define two variants of CLZ > instruction with that flag: > > 1. 'Zbb' (RV32/RV64) > 2. 'Zbpbo' (RV32) Will we write an extra entry for clz can resolve the problem? {"clz", 32, INSN_CLASS_ZBPBO, ... {"clz", 0, INSN_CLASS_ZBB, ... Nelson > If both fails, we can widen the instruction class from "INSN_CLASS_ZBPBO" > to "INSN_CLASS_ZBB_OR_ZBPBO" if XLEN is 32. > After doing that, we will get following diagnostics: > > - On RV32: 'Zbb' or 'Zbpbo' is required > - On RV64: 'Zbb' is required > > > [Usage: 'D'/'Zdinx' or 'Q'/'Zqinx'] > > Due to use of register pairs, we need to split 'D'/'Q' and 'Zdinx'/'Zqinx' > variants. For instance, if parsing "fmin.d" fails on both 'D' and 'Zdinx' > variants, we have to require 'D' or 'Zdinx', not just 'Zdinx', the last > "fmin.d" variant in riscv_opcodes. > > 1. 'D' > 2. 'Zdinx' > > If both fails, we can widen the instruction class from "INSN_CLASS_ZDINX" to > "INSN_CLASS_D_OR_ZDINX". > After doing that, we will get diagnostics saying 'D' or 'Zdinx' is required. > > > > Thanks, > Tsukasa > > > > > Tsukasa OI (1): > RISC-V: Implement extension variants > > gas/config/tc-riscv.c | 27 +++++++++++++++++++++++++-- > include/opcode/riscv.h | 5 +++++ > 2 files changed, 30 insertions(+), 2 deletions(-) > > > base-commit: b4477c7f666bdeb7f8e998633c7b0cb62310b9ef > -- > 2.34.1 > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFC PATCH 0/1] RISC-V: Implement "extension variants" for diagnostics 2022-10-01 7:21 ` [RFC PATCH 0/1] RISC-V: Implement "extension variants" for diagnostics Nelson Chu @ 2022-10-01 8:25 ` Tsukasa OI 2022-10-06 11:12 ` Nelson Chu 0 siblings, 1 reply; 6+ messages in thread From: Tsukasa OI @ 2022-10-01 8:25 UTC (permalink / raw) To: Nelson Chu; +Cc: Binutils On 2022/10/01 16:21, Nelson Chu wrote: > On Sat, Oct 1, 2022 at 1:27 PM Tsukasa OI <research_trasio@irq.a4lg.com> wrote: >> >> Hi all RISC-V folks, >> >> GitHub tracker: >> <https://github.com/a4lg/binutils-gdb/wiki/riscv_insn_ext_variants> >> >> While investigating possible implementation of both 'Z[fdq]inx' and 'P' >> implementations, I found something common (not just register pairs). >> >> Here, this patch implements what I call "extension variants". >> It was formerly a part of 'Zfinx' fixes (formerly the flag was named >> "INSN_F_OR_X"). >> >> If there is a instruction with multiple variants with different requirements >> and the assembler fails to parse all variants, there is a case that needs to >> refer ALL variants to generate proper diagnostics. >> >> If an instruction with "INSN_HAS_EXT_VARS" fails on all variants, the >> assembler now has a chance to modify the instruction class for proper >> diagnostics. A typical use of this feature is to select wider instruction >> class when necessary. >> >> >> [Usage: CLZ instruction ('Zbpbo')] >> >> To implement proposed 'Zbpbo' extension, updating instruction class for >> CLZ instruction is not enough. If we change CLZ instruction class from >> "INSN_CLASS_ZBB" to "INSN_CLASS_ZBB_OR_ZBPBO", we will **mistakenly** >> support that instruction on RV64_Zbpbo because CLZ on 'Zbpbo' is RV32-only. >> >> Possible use with "INSN_HAS_EXT_VARS" is to define two variants of CLZ >> instruction with that flag: >> >> 1. 'Zbb' (RV32/RV64) >> 2. 'Zbpbo' (RV32) > > Will we write an extra entry for clz can resolve the problem? > > {"clz", 32, INSN_CLASS_ZBPBO, ... > {"clz", 0, INSN_CLASS_ZBB, ... > > Nelson I initially thought so but it turned out that it cannot. I learned it when I splitted D/Zdinx and Q/Zqinx. If an instruction entry is not matched because an extension is missing, information to store missing extension is stored before trying the next variant. The point here is, this information is _overwritten_ when not matched and only the last one is used to generate the error message when all variants are not matched. So (on environment with -march=rv32i with no 'Zbb' or 'Zbpbo'), just placing two entries like that results in the message saying 'Zbb' is required ('Zbpbo' is going to be ignored). That's why something has to change (and I think that this is the one of the simplest solution). Thanks, Tsukasa > >> If both fails, we can widen the instruction class from "INSN_CLASS_ZBPBO" >> to "INSN_CLASS_ZBB_OR_ZBPBO" if XLEN is 32. >> After doing that, we will get following diagnostics: >> >> - On RV32: 'Zbb' or 'Zbpbo' is required >> - On RV64: 'Zbb' is required >> >> >> [Usage: 'D'/'Zdinx' or 'Q'/'Zqinx'] >> >> Due to use of register pairs, we need to split 'D'/'Q' and 'Zdinx'/'Zqinx' >> variants. For instance, if parsing "fmin.d" fails on both 'D' and 'Zdinx' >> variants, we have to require 'D' or 'Zdinx', not just 'Zdinx', the last >> "fmin.d" variant in riscv_opcodes. >> >> 1. 'D' >> 2. 'Zdinx' >> >> If both fails, we can widen the instruction class from "INSN_CLASS_ZDINX" to >> "INSN_CLASS_D_OR_ZDINX". >> After doing that, we will get diagnostics saying 'D' or 'Zdinx' is required. >> >> >> >> Thanks, >> Tsukasa >> >> >> >> >> Tsukasa OI (1): >> RISC-V: Implement extension variants >> >> gas/config/tc-riscv.c | 27 +++++++++++++++++++++++++-- >> include/opcode/riscv.h | 5 +++++ >> 2 files changed, 30 insertions(+), 2 deletions(-) >> >> >> base-commit: b4477c7f666bdeb7f8e998633c7b0cb62310b9ef >> -- >> 2.34.1 >> > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFC PATCH 0/1] RISC-V: Implement "extension variants" for diagnostics 2022-10-01 8:25 ` Tsukasa OI @ 2022-10-06 11:12 ` Nelson Chu 2022-10-06 12:08 ` Tsukasa OI 0 siblings, 1 reply; 6+ messages in thread From: Nelson Chu @ 2022-10-06 11:12 UTC (permalink / raw) To: Tsukasa OI; +Cc: Binutils, Kito Cheng, Palmer Dabbelt Combining the INSN_CLASS will make things complicated and may need lots of special combine rules in the future. You probably can combine INSN_CLASS_A and INSN_CLASS_B easier, but it is hard to combine something like INSN_CLASS_A_OR_B, INSN_CLASS_C_AND_D, ... into one CLASS, so if we choose your proposal, then we will need a lot of special handlings in the future, and that's what I don't really hope to see. Generally, consider we have, { insn, INSN_CLASS_A, ... }, { insn, INSN_CLASS_B, ... }, { insn, INSN_CLASS_C, ... }, (may have more ... ) The current assembler just reports an error for the last entry. The checking rule is, INSN_CLASS_A? -> invalid operands of A -> INSN_CLASS_B? -> invalid operands of B -> INSN_CLASS_C? -> invalid operands of C Too strict error report is a bit impractical, since it is difficult to report accurately by the current mechanism. So two cases, 1. Once any of INSN_CLASS_* is passed, just reporting "illegal operands" should be enough. Or you probably want to report more, something like "illegal operands for extension A...", but in fact you may have a chance to see that all ABC extensions (or both A and C) are enabled, so which one you want to report? So that's why I said, just reporting "illegal operands" is acceptable and correct. 2. we don't have any illegal operands since all of the INSN_CLASS* are not passed. Assume the strings of INSN_CLASS_A/B/C from riscv_multi_subset_supports_ext are string_A/B/C, then you probably can just report "extension string_A or string_B or string_C is/are required". Since we have the beginning entry of riscv_opcode from str_hash_find, it should be easy to get the whole INSN_CLASS* and their error string easier. Anyway, I hope we can try the more general and compromise solutions for every case, rather than make things too complicated and specially. Thanks Nelson On Sat, Oct 1, 2022 at 4:26 PM Tsukasa OI <research_trasio@irq.a4lg.com> wrote: > > On 2022/10/01 16:21, Nelson Chu wrote: > > On Sat, Oct 1, 2022 at 1:27 PM Tsukasa OI <research_trasio@irq.a4lg.com> wrote: > >> > >> Hi all RISC-V folks, > >> > >> GitHub tracker: > >> <https://github.com/a4lg/binutils-gdb/wiki/riscv_insn_ext_variants> > >> > >> While investigating possible implementation of both 'Z[fdq]inx' and 'P' > >> implementations, I found something common (not just register pairs). > >> > >> Here, this patch implements what I call "extension variants". > >> It was formerly a part of 'Zfinx' fixes (formerly the flag was named > >> "INSN_F_OR_X"). > >> > >> If there is a instruction with multiple variants with different requirements > >> and the assembler fails to parse all variants, there is a case that needs to > >> refer ALL variants to generate proper diagnostics. > >> > >> If an instruction with "INSN_HAS_EXT_VARS" fails on all variants, the > >> assembler now has a chance to modify the instruction class for proper > >> diagnostics. A typical use of this feature is to select wider instruction > >> class when necessary. > >> > >> > >> [Usage: CLZ instruction ('Zbpbo')] > >> > >> To implement proposed 'Zbpbo' extension, updating instruction class for > >> CLZ instruction is not enough. If we change CLZ instruction class from > >> "INSN_CLASS_ZBB" to "INSN_CLASS_ZBB_OR_ZBPBO", we will **mistakenly** > >> support that instruction on RV64_Zbpbo because CLZ on 'Zbpbo' is RV32-only. > >> > >> Possible use with "INSN_HAS_EXT_VARS" is to define two variants of CLZ > >> instruction with that flag: > >> > >> 1. 'Zbb' (RV32/RV64) > >> 2. 'Zbpbo' (RV32) > > > > Will we write an extra entry for clz can resolve the problem? > > > > {"clz", 32, INSN_CLASS_ZBPBO, ... > > {"clz", 0, INSN_CLASS_ZBB, ... > > > > Nelson > > I initially thought so but it turned out that it cannot. I learned it > when I splitted D/Zdinx and Q/Zqinx. > > If an instruction entry is not matched because an extension is missing, > information to store missing extension is stored before trying the next > variant. The point here is, this information is _overwritten_ when not > matched and only the last one is used to generate the error message when > all variants are not matched. > > So (on environment with -march=rv32i with no 'Zbb' or 'Zbpbo'), just > placing two entries like that results in the message saying 'Zbb' is > required ('Zbpbo' is going to be ignored). > > That's why something has to change (and I think that this is the one of > the simplest solution). > > Thanks, > Tsukasa > > > > >> If both fails, we can widen the instruction class from "INSN_CLASS_ZBPBO" > >> to "INSN_CLASS_ZBB_OR_ZBPBO" if XLEN is 32. > >> After doing that, we will get following diagnostics: > >> > >> - On RV32: 'Zbb' or 'Zbpbo' is required > >> - On RV64: 'Zbb' is required > >> > >> > >> [Usage: 'D'/'Zdinx' or 'Q'/'Zqinx'] > >> > >> Due to use of register pairs, we need to split 'D'/'Q' and 'Zdinx'/'Zqinx' > >> variants. For instance, if parsing "fmin.d" fails on both 'D' and 'Zdinx' > >> variants, we have to require 'D' or 'Zdinx', not just 'Zdinx', the last > >> "fmin.d" variant in riscv_opcodes. > >> > >> 1. 'D' > >> 2. 'Zdinx' > >> > >> If both fails, we can widen the instruction class from "INSN_CLASS_ZDINX" to > >> "INSN_CLASS_D_OR_ZDINX". > >> After doing that, we will get diagnostics saying 'D' or 'Zdinx' is required. > >> > >> > >> > >> Thanks, > >> Tsukasa > >> > >> > >> > >> > >> Tsukasa OI (1): > >> RISC-V: Implement extension variants > >> > >> gas/config/tc-riscv.c | 27 +++++++++++++++++++++++++-- > >> include/opcode/riscv.h | 5 +++++ > >> 2 files changed, 30 insertions(+), 2 deletions(-) > >> > >> > >> base-commit: b4477c7f666bdeb7f8e998633c7b0cb62310b9ef > >> -- > >> 2.34.1 > >> > > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFC PATCH 0/1] RISC-V: Implement "extension variants" for diagnostics 2022-10-06 11:12 ` Nelson Chu @ 2022-10-06 12:08 ` Tsukasa OI 0 siblings, 0 replies; 6+ messages in thread From: Tsukasa OI @ 2022-10-06 12:08 UTC (permalink / raw) To: Nelson Chu; +Cc: Binutils, Kito Cheng, Palmer Dabbelt On 2022/10/06 20:12, Nelson Chu wrote: > Combining the INSN_CLASS will make things complicated and may need > lots of special combine rules in the future. You probably can combine > INSN_CLASS_A and INSN_CLASS_B easier, but it is hard to combine > something like INSN_CLASS_A_OR_B, INSN_CLASS_C_AND_D, ... into one > CLASS, so if we choose your proposal, then we will need a lot of > special handlings in the future, and that's what I don't really hope > to see. Generally, consider we have, > > { insn, INSN_CLASS_A, ... }, > { insn, INSN_CLASS_B, ... }, > { insn, INSN_CLASS_C, ... }, > (may have more ... ) > > The current assembler just reports an error for the last entry. The > checking rule is, > INSN_CLASS_A? -> invalid operands of A -> INSN_CLASS_B? -> invalid > operands of B -> INSN_CLASS_C? -> invalid operands of C > > Too strict error report is a bit impractical, since it is difficult to > report accurately by the current mechanism. So two cases, > 1. Once any of INSN_CLASS_* is passed, just reporting "illegal > operands" should be enough. Or you probably want to report more, > something like "illegal operands for extension A...", but in fact you > may have a chance to see that all ABC extensions (or both A and C) are > enabled, so which one you want to report? So that's why I said, just > reporting "illegal operands" is acceptable and correct. > > 2. we don't have any illegal operands since all of the INSN_CLASS* are > not passed. Assume the strings of INSN_CLASS_A/B/C from > riscv_multi_subset_supports_ext are string_A/B/C, then you probably > can just report "extension string_A or string_B or string_C is/are > required". Since we have the beginning entry of riscv_opcode from > str_hash_find, it should be easy to get the whole INSN_CLASS* and > their error string easier. > > Anyway, I hope we can try the more general and compromise solutions > for every case, rather than make things too complicated and specially. Case 2 is what I partially agree but case 1 is ABSOLUTELY NOT. I admit that my proposal is not *very* extensible but I think this is sufficient for a while. I don't think my proposal will break in a few years. Long-term solution would be a list of instruction classes (and optional failure reasons appended when not matched). That's what you pointed in the case 2, right? I AM trying compromise mid-term solution (to fix the problems WE EASILY FACE) and I don't think this is too complicated and specialized. > > > Thanks > Nelson > > On Sat, Oct 1, 2022 at 4:26 PM Tsukasa OI <research_trasio@irq.a4lg.com> wrote: >> >> On 2022/10/01 16:21, Nelson Chu wrote: >>> On Sat, Oct 1, 2022 at 1:27 PM Tsukasa OI <research_trasio@irq.a4lg.com> wrote: >>>> >>>> Hi all RISC-V folks, >>>> >>>> GitHub tracker: >>>> <https://github.com/a4lg/binutils-gdb/wiki/riscv_insn_ext_variants> >>>> >>>> While investigating possible implementation of both 'Z[fdq]inx' and 'P' >>>> implementations, I found something common (not just register pairs). >>>> >>>> Here, this patch implements what I call "extension variants". >>>> It was formerly a part of 'Zfinx' fixes (formerly the flag was named >>>> "INSN_F_OR_X"). >>>> >>>> If there is a instruction with multiple variants with different requirements >>>> and the assembler fails to parse all variants, there is a case that needs to >>>> refer ALL variants to generate proper diagnostics. >>>> >>>> If an instruction with "INSN_HAS_EXT_VARS" fails on all variants, the >>>> assembler now has a chance to modify the instruction class for proper >>>> diagnostics. A typical use of this feature is to select wider instruction >>>> class when necessary. >>>> >>>> >>>> [Usage: CLZ instruction ('Zbpbo')] >>>> >>>> To implement proposed 'Zbpbo' extension, updating instruction class for >>>> CLZ instruction is not enough. If we change CLZ instruction class from >>>> "INSN_CLASS_ZBB" to "INSN_CLASS_ZBB_OR_ZBPBO", we will **mistakenly** >>>> support that instruction on RV64_Zbpbo because CLZ on 'Zbpbo' is RV32-only. >>>> >>>> Possible use with "INSN_HAS_EXT_VARS" is to define two variants of CLZ >>>> instruction with that flag: >>>> >>>> 1. 'Zbb' (RV32/RV64) >>>> 2. 'Zbpbo' (RV32) >>> >>> Will we write an extra entry for clz can resolve the problem? >>> >>> {"clz", 32, INSN_CLASS_ZBPBO, ... >>> {"clz", 0, INSN_CLASS_ZBB, ... >>> >>> Nelson >> >> I initially thought so but it turned out that it cannot. I learned it >> when I splitted D/Zdinx and Q/Zqinx. >> >> If an instruction entry is not matched because an extension is missing, >> information to store missing extension is stored before trying the next >> variant. The point here is, this information is _overwritten_ when not >> matched and only the last one is used to generate the error message when >> all variants are not matched. >> >> So (on environment with -march=rv32i with no 'Zbb' or 'Zbpbo'), just >> placing two entries like that results in the message saying 'Zbb' is >> required ('Zbpbo' is going to be ignored). >> >> That's why something has to change (and I think that this is the one of >> the simplest solution). >> >> Thanks, >> Tsukasa >> >>> >>>> If both fails, we can widen the instruction class from "INSN_CLASS_ZBPBO" >>>> to "INSN_CLASS_ZBB_OR_ZBPBO" if XLEN is 32. >>>> After doing that, we will get following diagnostics: >>>> >>>> - On RV32: 'Zbb' or 'Zbpbo' is required >>>> - On RV64: 'Zbb' is required >>>> >>>> >>>> [Usage: 'D'/'Zdinx' or 'Q'/'Zqinx'] >>>> >>>> Due to use of register pairs, we need to split 'D'/'Q' and 'Zdinx'/'Zqinx' >>>> variants. For instance, if parsing "fmin.d" fails on both 'D' and 'Zdinx' >>>> variants, we have to require 'D' or 'Zdinx', not just 'Zdinx', the last >>>> "fmin.d" variant in riscv_opcodes. >>>> >>>> 1. 'D' >>>> 2. 'Zdinx' >>>> >>>> If both fails, we can widen the instruction class from "INSN_CLASS_ZDINX" to >>>> "INSN_CLASS_D_OR_ZDINX". >>>> After doing that, we will get diagnostics saying 'D' or 'Zdinx' is required. >>>> >>>> >>>> >>>> Thanks, >>>> Tsukasa >>>> >>>> >>>> >>>> >>>> Tsukasa OI (1): >>>> RISC-V: Implement extension variants >>>> >>>> gas/config/tc-riscv.c | 27 +++++++++++++++++++++++++-- >>>> include/opcode/riscv.h | 5 +++++ >>>> 2 files changed, 30 insertions(+), 2 deletions(-) >>>> >>>> >>>> base-commit: b4477c7f666bdeb7f8e998633c7b0cb62310b9ef >>>> -- >>>> 2.34.1 >>>> >>> > ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-10-06 12:08 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-10-01 5:27 [RFC PATCH 0/1] RISC-V: Implement "extension variants" for diagnostics Tsukasa OI 2022-10-01 5:27 ` [RFC PATCH 1/1] RISC-V: Implement extension variants Tsukasa OI 2022-10-01 7:21 ` [RFC PATCH 0/1] RISC-V: Implement "extension variants" for diagnostics Nelson Chu 2022-10-01 8:25 ` Tsukasa OI 2022-10-06 11:12 ` Nelson Chu 2022-10-06 12:08 ` Tsukasa OI
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).