* [PATCH 0/3] RISC-V: Support non-standard encodings (on widening FP ops)
@ 2022-11-28 6:39 Tsukasa OI
2022-11-28 6:39 ` [PATCH 1/3] RISC-V: Allocate "various" operand type Tsukasa OI
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Tsukasa OI @ 2022-11-28 6:39 UTC (permalink / raw)
To: Tsukasa OI, Nelson Chu, Kito Cheng, Palmer Dabbelt; +Cc: binutils
Hello,
Some of the floating point instructions does not depend on the rounding mode
despite the existence of rm (rounding mode) field. Such examples are
widening conversion instructions.
Quoting "11.2 Floating-Point Control and Status Register" from
the RISC-V ISA Manual (version 20191213):
> Some instructions, including widening conversions, have the rm field but
> are nevertheless unaffected by the rounding mode; software should set
> their rm field to RNE (000).
The latest draft of the ISA Manual is clarified further:
Quoting "13.2 Floating-Point Control and Status Register" from
the RISC-V ISA Manual (version draft-20221119-5234c63):
> Some instructions, including widening conversions, have the rm field but
> are nevertheless mathematically unaffected by the rounding mode; software
> should set their rm field to RNE (000) but implementations must treat the
> rm field as usual (in particular, with regard to decoding legal vs.
> reserved encodings).
For instance, to encode a FCVT.D.S instruction, we should set its rm field
to RNE (0b000). However, FCVT.D.S instruction with non-RNE rm field is
still a valid instruction (despite that GAS does not allow specifying any
rounding modes on FCVT.D.S) and must handle as a valid instruction when
disassembled unless an invalid rounding mode is specified.
However, current GNU Binutils only supports disassembling widening
conversion instructions with rm field of RNE (0b000) except FCVT.Q.L and
FCVT.Q.LU instructions (two instructions supported specifying rounding
modes for historical reasons).
This patchset (in specific, PATCH 3/3) enables special handling of such
instructions by adding two new operand types:
1. "WfM": optional rounding mode where specifying rounding mode is not
supported in the past.
2. "Wfm": optional rounding mode where specifying rounding mode is
supported in the past (used in FCVT.Q.L and FCVT.Q.LU).
I designed this patchset to be configurable (allow implementing S Pawan
Kumar's proposal if needed) but the behavior in this patchset is as follows:
Disassembler:
Optional (non-RNE [!= 0b000]) rounding mode is printed only if:
(a) "no-aliases" disassembler option is specified, or
(b) the rounding mode is invalid (0b101 / 0b110).
I think removing condition (a) might be an option.
Because, despite that we can now see the actual rounding mode with
condition (a), it's not valid as an assembler mnemonic.
Condition (b) is an intentional choice to detect invalid encodings.
Still, it could be removed, too (I don't recommend though).
Assembler:
Specifying optional rounding mode is prohibited (except FCVT.Q.L and
FCVT.Q.LU) or accepted with a warning (FCVT.Q.L and FCVT.Q.LU).
c.f. S Pawan Kumar's proposal:
<https://github.com/riscv-collab/riscv-gnu-toolchain/issues/1089>
# Before this patchset: objdump -d (with my comment)
8000002c: 42058553 fcvt.d.s fa0,fa1
80000030: 42059553 .4byte 0x42059553 # Valid (but not recommended) encoding of FCVT.D.S
80000034: 4205a553 .4byte 0x4205a553 # Valid (but not recommended) encoding of FCVT.D.S
80000038: 4205b553 .4byte 0x4205b553 # Valid (but not recommended) encoding of FCVT.D.S
8000003c: 4205c553 .4byte 0x4205c553 # Valid (but not recommended) encoding of FCVT.D.S
80000040: 4205f553 .4byte 0x4205f553 # Valid (but not recommended) encoding of FCVT.D.S
80000044: 4205d553 .4byte 0x4205d553 # Invalid FCVT.D.S (reserved rounding mode 0b101)
80000048: 4205e553 .4byte 0x4205e553 # Invalid FCVT.D.S (reserved rounding mode 0b110)
# After this patchset: objdump -d
8000002c: 42058553 fcvt.d.s fa0,fa1
80000030: 42059553 fcvt.d.s fa0,fa1
80000034: 4205a553 fcvt.d.s fa0,fa1
80000038: 4205b553 fcvt.d.s fa0,fa1
8000003c: 4205c553 fcvt.d.s fa0,fa1
80000040: 4205f553 fcvt.d.s fa0,fa1
80000044: 4205d553 fcvt.d.s fa0,fa1,unknown
80000048: 4205e553 fcvt.d.s fa0,fa1,unknown
# After this patchset: objdump -M no-aliases -d
8000002c: 42058553 fcvt.d.s fa0,fa1
80000030: 42059553 fcvt.d.s fa0,fa1,rtz
80000034: 4205a553 fcvt.d.s fa0,fa1,rdn
80000038: 4205b553 fcvt.d.s fa0,fa1,rup
8000003c: 4205c553 fcvt.d.s fa0,fa1,rmm
80000040: 4205f553 fcvt.d.s fa0,fa1,dyn
80000044: 4205d553 fcvt.d.s fa0,fa1,unknown
80000048: 4205e553 fcvt.d.s fa0,fa1,unknown
Due to my development process, it now depends on the commit
("RISC-V: Allocate "various" operand type": PATCH 1/3) which adds "Wif"
operand type (not just "WfM" and "Wfm" which is added by PATCH 3/3).
PATCH 1/3 is the same patch as this:
<https://sourceware.org/pipermail/binutils/2022-October/123884.html>
If requested, I can remove the dependency to PATCH 1/3 (in that case my
local development branches will be a bit complex but nothing else happens).
Thanks,
Tsukasa
Tsukasa OI (3):
RISC-V: Allocate "various" operand type
RISC-V: Reorganize invalid rounding mode test
RISC-V: Rounding mode on widening instructions
gas/config/tc-riscv.c | 107 +++++++++++++++---
gas/testsuite/gas/riscv/rouding-fail.d | 3 -
gas/testsuite/gas/riscv/rouding-fail.l | 3 -
gas/testsuite/gas/riscv/rouding-fail.s | 3 -
.../gas/riscv/rounding-dis-widening-noalias.d | 13 +++
.../gas/riscv/rounding-dis-widening.d | 13 +++
.../gas/riscv/rounding-dis-widening.s | 8 ++
gas/testsuite/gas/riscv/rounding-fail.d | 3 +
gas/testsuite/gas/riscv/rounding-fail.l | 16 +++
gas/testsuite/gas/riscv/rounding-fail.s | 22 ++++
.../gas/riscv/rounding-fcvt.q.l-noalias.d | 15 +++
gas/testsuite/gas/riscv/rounding-fcvt.q.l.d | 15 +++
gas/testsuite/gas/riscv/rounding-fcvt.q.l.l | 3 +
gas/testsuite/gas/riscv/rounding-fcvt.q.l.s | 5 +
opcodes/riscv-dis.c | 48 +++++++-
opcodes/riscv-opc.c | 32 +++---
16 files changed, 261 insertions(+), 48 deletions(-)
delete mode 100644 gas/testsuite/gas/riscv/rouding-fail.d
delete mode 100644 gas/testsuite/gas/riscv/rouding-fail.l
delete mode 100644 gas/testsuite/gas/riscv/rouding-fail.s
create mode 100644 gas/testsuite/gas/riscv/rounding-dis-widening-noalias.d
create mode 100644 gas/testsuite/gas/riscv/rounding-dis-widening.d
create mode 100644 gas/testsuite/gas/riscv/rounding-dis-widening.s
create mode 100644 gas/testsuite/gas/riscv/rounding-fail.d
create mode 100644 gas/testsuite/gas/riscv/rounding-fail.l
create mode 100644 gas/testsuite/gas/riscv/rounding-fail.s
create mode 100644 gas/testsuite/gas/riscv/rounding-fcvt.q.l-noalias.d
create mode 100644 gas/testsuite/gas/riscv/rounding-fcvt.q.l.d
create mode 100644 gas/testsuite/gas/riscv/rounding-fcvt.q.l.l
create mode 100644 gas/testsuite/gas/riscv/rounding-fcvt.q.l.s
base-commit: c341f4676af4f9922ca61e1b093d103ed808ae6e
--
2.38.1
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 1/3] RISC-V: Allocate "various" operand type
2022-11-28 6:39 [PATCH 0/3] RISC-V: Support non-standard encodings (on widening FP ops) Tsukasa OI
@ 2022-11-28 6:39 ` Tsukasa OI
2022-11-29 2:42 ` Nelson Chu
2022-11-28 6:39 ` [PATCH 2/3] RISC-V: Reorganize invalid rounding mode test Tsukasa OI
2022-11-28 6:39 ` [PATCH 3/3] RISC-V: Rounding mode on widening instructions Tsukasa OI
2 siblings, 1 reply; 6+ messages in thread
From: Tsukasa OI @ 2022-11-28 6:39 UTC (permalink / raw)
To: Tsukasa OI, Nelson Chu, Kito Cheng, Palmer Dabbelt; +Cc: binutils
From: Tsukasa OI <research_trasio@irq.a4lg.com>
This commit intends to move operands that require very special handling or
operand types that are so minor (e.g. only useful on a few instructions)
under "W". I also intend this "W" to be "temporary" operand storage until
we can find good two character (or less) operand type.
In this commit, prefetch offset operand "f" for 'Zicbop' extension is moved
to "Wif" because of its special handling (and allocating single character
"f" for this operand type seemed too much).
Current expected allocation guideline is as follows:
1. 'W'
2. The most closely related single-letter extension in lowercase
(strongly recommended but not mandatory)
3. Identify operand type
The author currently plans to allocate following three-character operand
types (for operands including instructions from unratified extensions).
1. "Wif" ('Zicbop': fetch offset)
2. "Wfv" (unratified 'Zfa': value operand from FLI.[HSDQ] instructions)
3. "Wfm" / "WfM"
'Zfh', 'F', 'D', 'Q': rounding modes "m" with special handling
solely for widening conversion instructions.
gas/ChangeLog:
* config/tc-riscv.c (validate_riscv_insn, riscv_ip): Move from
"f" to "Wif".
opcodes/ChangeLog:
* riscv-dis.c (print_insn_args): Move from "f" to "Wif".
* riscv-opc.c (riscv_opcodes): Reflect new operand type.
---
gas/config/tc-riscv.c | 64 +++++++++++++++++++++++++++++++------------
opcodes/riscv-dis.c | 26 ++++++++++++++----
opcodes/riscv-opc.c | 6 ++--
3 files changed, 71 insertions(+), 25 deletions(-)
diff --git a/gas/config/tc-riscv.c b/gas/config/tc-riscv.c
index 0682eb355241..bb0e18ac8d52 100644
--- a/gas/config/tc-riscv.c
+++ b/gas/config/tc-riscv.c
@@ -1359,7 +1359,6 @@ validate_riscv_insn (const struct riscv_opcode *opc, int length)
case 'j': used_bits |= ENCODE_ITYPE_IMM (-1U); break;
case 'a': used_bits |= ENCODE_JTYPE_IMM (-1U); break;
case 'p': used_bits |= ENCODE_BTYPE_IMM (-1U); break;
- case 'f': /* Fall through. */
case 'q': used_bits |= ENCODE_STYPE_IMM (-1U); break;
case 'u': used_bits |= ENCODE_UTYPE_IMM (-1U); break;
case 'z': break; /* Zero immediate. */
@@ -1386,6 +1385,21 @@ validate_riscv_insn (const struct riscv_opcode *opc, int length)
goto unknown_validate_operand;
}
break;
+ case 'W': /* Various operands. */
+ switch (*++oparg)
+ {
+ case 'i':
+ switch (*++oparg)
+ {
+ case 'f': used_bits |= ENCODE_STYPE_IMM (-1U); break;
+ default:
+ goto unknown_validate_operand;
+ }
+ break;
+ default:
+ goto unknown_validate_operand;
+ }
+ break;
case 'X': /* Integer immediate. */
{
size_t n;
@@ -3401,22 +3415,37 @@ riscv_ip (char *str, struct riscv_cl_insn *ip, expressionS *imm_expr,
imm_expr->X_op = O_absent;
continue;
- case 'f': /* Prefetch offset, pseudo S-type but lower 5-bits zero. */
- if (riscv_handle_implicit_zero_offset (imm_expr, asarg))
- continue;
- my_getExpression (imm_expr, asarg);
- check_absolute_expr (ip, imm_expr, false);
- if (((unsigned) (imm_expr->X_add_number) & 0x1fU)
- || imm_expr->X_add_number >= (signed) RISCV_IMM_REACH / 2
- || imm_expr->X_add_number < -(signed) RISCV_IMM_REACH / 2)
- as_bad (_("improper prefetch offset (%ld)"),
- (long) imm_expr->X_add_number);
- ip->insn_opcode |=
- ENCODE_STYPE_IMM ((unsigned) (imm_expr->X_add_number) &
- ~ 0x1fU);
- imm_expr->X_op = O_absent;
- asarg = expr_end;
- continue;
+ case 'W': /* Various operands. */
+ switch (*++oparg)
+ {
+ case 'i':
+ switch (*++oparg)
+ {
+ case 'f':
+ /* Prefetch offset for 'Zicbop' extension.
+ pseudo S-type but lower 5-bits zero. */
+ if (riscv_handle_implicit_zero_offset (imm_expr, asarg))
+ continue;
+ my_getExpression (imm_expr, asarg);
+ check_absolute_expr (ip, imm_expr, false);
+ if (((unsigned) (imm_expr->X_add_number) & 0x1fU)
+ || imm_expr->X_add_number >= RISCV_IMM_REACH / 2
+ || imm_expr->X_add_number < -RISCV_IMM_REACH / 2)
+ as_bad (_ ("improper prefetch offset (%ld)"),
+ (long) imm_expr->X_add_number);
+ ip->insn_opcode |= ENCODE_STYPE_IMM (
+ (unsigned) (imm_expr->X_add_number) & ~0x1fU);
+ imm_expr->X_op = O_absent;
+ asarg = expr_end;
+ continue;
+ default:
+ goto unknown_riscv_ip_operand;
+ }
+ break;
+ default:
+ goto unknown_riscv_ip_operand;
+ }
+ break;
case 'X': /* Integer immediate. */
{
@@ -3469,6 +3498,7 @@ riscv_ip (char *str, struct riscv_cl_insn *ip, expressionS *imm_expr,
}
}
break;
+
default:
unknown_riscv_ip_operand:
as_fatal (_("internal: unknown argument type `%s'"),
diff --git a/opcodes/riscv-dis.c b/opcodes/riscv-dis.c
index 0e1f3b4610aa..1e6716e8e58c 100644
--- a/opcodes/riscv-dis.c
+++ b/opcodes/riscv-dis.c
@@ -473,11 +473,6 @@ print_insn_args (const char *oparg, insn_t l, bfd_vma pc, disassemble_info *info
(int)EXTRACT_STYPE_IMM (l));
break;
- case 'f':
- print (info->stream, dis_style_address_offset, "%d",
- (int)EXTRACT_STYPE_IMM (l));
- break;
-
case 'a':
info->target = EXTRACT_JTYPE_IMM (l) + pc;
(*info->print_address_func) (info->target, info);
@@ -582,6 +577,27 @@ print_insn_args (const char *oparg, insn_t l, bfd_vma pc, disassemble_info *info
print (info->stream, dis_style_immediate, "%d", rs1);
break;
+ case 'W': /* Various operands. */
+ {
+ switch (*++oparg)
+ {
+ case 'i':
+ switch (*++oparg)
+ {
+ case 'f':
+ print (info->stream, dis_style_address_offset, "%d",
+ (int) EXTRACT_STYPE_IMM (l));
+ break;
+ default:
+ goto undefined_modifier;
+ }
+ break;
+ default:
+ goto undefined_modifier;
+ }
+ }
+ break;
+
case 'X': /* Integer immediate. */
{
size_t n;
diff --git a/opcodes/riscv-opc.c b/opcodes/riscv-opc.c
index 0e691544f9bc..653eb60f2a58 100644
--- a/opcodes/riscv-opc.c
+++ b/opcodes/riscv-opc.c
@@ -313,9 +313,9 @@ const struct riscv_opcode riscv_opcodes[] =
/* name, xlen, isa, operands, match, mask, match_func, pinfo. */
/* Standard hints. */
-{"prefetch.i", 0, INSN_CLASS_ZICBOP, "f(s)", MATCH_PREFETCH_I, MASK_PREFETCH_I, match_opcode, 0 },
-{"prefetch.r", 0, INSN_CLASS_ZICBOP, "f(s)", MATCH_PREFETCH_R, MASK_PREFETCH_R, match_opcode, 0 },
-{"prefetch.w", 0, INSN_CLASS_ZICBOP, "f(s)", MATCH_PREFETCH_W, MASK_PREFETCH_W, match_opcode, 0 },
+{"prefetch.i", 0, INSN_CLASS_ZICBOP, "Wif(s)", MATCH_PREFETCH_I, MASK_PREFETCH_I, match_opcode, 0 },
+{"prefetch.r", 0, INSN_CLASS_ZICBOP, "Wif(s)", MATCH_PREFETCH_R, MASK_PREFETCH_R, match_opcode, 0 },
+{"prefetch.w", 0, INSN_CLASS_ZICBOP, "Wif(s)", MATCH_PREFETCH_W, MASK_PREFETCH_W, match_opcode, 0 },
{"pause", 0, INSN_CLASS_ZIHINTPAUSE, "", MATCH_PAUSE, MASK_PAUSE, match_opcode, 0 },
/* Basic RVI instructions and aliases. */
--
2.38.1
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 2/3] RISC-V: Reorganize invalid rounding mode test
2022-11-28 6:39 [PATCH 0/3] RISC-V: Support non-standard encodings (on widening FP ops) Tsukasa OI
2022-11-28 6:39 ` [PATCH 1/3] RISC-V: Allocate "various" operand type Tsukasa OI
@ 2022-11-28 6:39 ` Tsukasa OI
2022-11-28 6:39 ` [PATCH 3/3] RISC-V: Rounding mode on widening instructions Tsukasa OI
2 siblings, 0 replies; 6+ messages in thread
From: Tsukasa OI @ 2022-11-28 6:39 UTC (permalink / raw)
To: Tsukasa OI, Nelson Chu, Kito Cheng, Palmer Dabbelt; +Cc: binutils
From: Tsukasa OI <research_trasio@irq.a4lg.com>
This commit reorganizes and adds testcases to invalid rounding mode
operand test. It also fixes a typo in the filename.
gas/ChangeLog:
* testsuite/gas/riscv/rounding-fail.d: Rename from
rouding-fail. Add some testcases.
* testsuite/gas/riscv/rounding-fail.s: Likewise.
* testsuite/gas/riscv/rounding-fail.l: Likewise.
---
gas/testsuite/gas/riscv/rouding-fail.d | 3 ---
gas/testsuite/gas/riscv/rouding-fail.s | 3 ---
gas/testsuite/gas/riscv/rounding-fail.d | 3 +++
gas/testsuite/gas/riscv/{rouding-fail.l => rounding-fail.l} | 2 ++
gas/testsuite/gas/riscv/rounding-fail.s | 6 ++++++
5 files changed, 11 insertions(+), 6 deletions(-)
delete mode 100644 gas/testsuite/gas/riscv/rouding-fail.d
delete mode 100644 gas/testsuite/gas/riscv/rouding-fail.s
create mode 100644 gas/testsuite/gas/riscv/rounding-fail.d
rename gas/testsuite/gas/riscv/{rouding-fail.l => rounding-fail.l} (52%)
create mode 100644 gas/testsuite/gas/riscv/rounding-fail.s
diff --git a/gas/testsuite/gas/riscv/rouding-fail.d b/gas/testsuite/gas/riscv/rouding-fail.d
deleted file mode 100644
index 9827b11446db..000000000000
--- a/gas/testsuite/gas/riscv/rouding-fail.d
+++ /dev/null
@@ -1,3 +0,0 @@
-#as: -march=rv32ifd
-#source: rouding-fail.s
-#error_output: rouding-fail.l
diff --git a/gas/testsuite/gas/riscv/rouding-fail.s b/gas/testsuite/gas/riscv/rouding-fail.s
deleted file mode 100644
index d18f53efb503..000000000000
--- a/gas/testsuite/gas/riscv/rouding-fail.s
+++ /dev/null
@@ -1,3 +0,0 @@
-target:
- fadd.s fa1,fa1,fa1,
- fadd.d fa1,fa1,fa1,
diff --git a/gas/testsuite/gas/riscv/rounding-fail.d b/gas/testsuite/gas/riscv/rounding-fail.d
new file mode 100644
index 000000000000..0d0a55818caf
--- /dev/null
+++ b/gas/testsuite/gas/riscv/rounding-fail.d
@@ -0,0 +1,3 @@
+#as: -march=rv32ifd
+#source: rounding-fail.s
+#error_output: rounding-fail.l
diff --git a/gas/testsuite/gas/riscv/rouding-fail.l b/gas/testsuite/gas/riscv/rounding-fail.l
similarity index 52%
rename from gas/testsuite/gas/riscv/rouding-fail.l
rename to gas/testsuite/gas/riscv/rounding-fail.l
index ea46e7c2d5aa..00d4d8e40fa6 100644
--- a/gas/testsuite/gas/riscv/rouding-fail.l
+++ b/gas/testsuite/gas/riscv/rounding-fail.l
@@ -1,3 +1,5 @@
.*: Assembler messages:
.*: Error: illegal operands `fadd.s fa1,fa1,fa1,'
.*: Error: illegal operands `fadd.d fa1,fa1,fa1,'
+.*: Error: illegal operands `fadd.s fa1,fa1,fa1,unknown'
+.*: Error: illegal operands `fadd.d fa1,fa1,fa1,unknown'
diff --git a/gas/testsuite/gas/riscv/rounding-fail.s b/gas/testsuite/gas/riscv/rounding-fail.s
new file mode 100644
index 000000000000..6e05cbd410c9
--- /dev/null
+++ b/gas/testsuite/gas/riscv/rounding-fail.s
@@ -0,0 +1,6 @@
+target:
+ # Invalid rounding modes
+ fadd.s fa1,fa1,fa1,
+ fadd.d fa1,fa1,fa1,
+ fadd.s fa1,fa1,fa1,unknown
+ fadd.d fa1,fa1,fa1,unknown
--
2.38.1
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 3/3] RISC-V: Rounding mode on widening instructions
2022-11-28 6:39 [PATCH 0/3] RISC-V: Support non-standard encodings (on widening FP ops) Tsukasa OI
2022-11-28 6:39 ` [PATCH 1/3] RISC-V: Allocate "various" operand type Tsukasa OI
2022-11-28 6:39 ` [PATCH 2/3] RISC-V: Reorganize invalid rounding mode test Tsukasa OI
@ 2022-11-28 6:39 ` Tsukasa OI
2 siblings, 0 replies; 6+ messages in thread
From: Tsukasa OI @ 2022-11-28 6:39 UTC (permalink / raw)
To: Tsukasa OI, Nelson Chu, Kito Cheng, Palmer Dabbelt
Cc: binutils, S Pawan Kumar
From: Tsukasa OI <research_trasio@irq.a4lg.com>
This commit adds support for rounding modes on widening instructions to
the assembler/disassembler.
On the disassembler, non-default rounding mode is displayed when
"no-aliases" option is given or the rounding mode itself is invalid.
On the assembler, specifying such rounding modes is prohibited unless
we have supported in the past.
gas/ChangeLog:
* config/tc-riscv.c (validate_riscv_insn): Add rounding mode
support to widening instructions.
(riscv_ip): Likewise.
* testsuite/gas/riscv/rounding-dis-widening.d: New disasm test.
* testsuite/gas/riscv/rounding-dis-widening.s: Likewise.
* testsuite/gas/riscv/rounding-dis-widening-noalias.d: Likewise.
* testsuite/gas/riscv/rounding-fail.d: Add testcases for widening
instructions.
* testsuite/gas/riscv/rounding-fail.l: Likewise.
* testsuite/gas/riscv/rounding-fail.s: Likewise.
* testsuite/gas/riscv/rounding-fcvt.q.l.d: New test.
* testsuite/gas/riscv/rounding-fcvt.q.l.l: Likewise.
* testsuite/gas/riscv/rounding-fcvt.q.l.s: Likewise.
* testsuite/gas/riscv/rounding-fcvt.q.l-noalias.d: Likewise.
opcodes/ChangeLog:
* riscv-dis.c (print_insn_args): Add rounding mode support to
widening instructions.
* riscv-opc.c (riscv_opcodes): Use new operand types.
Idea-by: Tsukasa OI <research_trasio@irq.a4lg.com>
Idea-by: S Pawan Kumar <pawan.kumar@incoresemi.com>
---
gas/config/tc-riscv.c | 43 +++++++++++++++++++
.../gas/riscv/rounding-dis-widening-noalias.d | 13 ++++++
.../gas/riscv/rounding-dis-widening.d | 13 ++++++
.../gas/riscv/rounding-dis-widening.s | 8 ++++
gas/testsuite/gas/riscv/rounding-fail.d | 2 +-
gas/testsuite/gas/riscv/rounding-fail.l | 11 +++++
gas/testsuite/gas/riscv/rounding-fail.s | 16 +++++++
.../gas/riscv/rounding-fcvt.q.l-noalias.d | 15 +++++++
gas/testsuite/gas/riscv/rounding-fcvt.q.l.d | 15 +++++++
gas/testsuite/gas/riscv/rounding-fcvt.q.l.l | 3 ++
gas/testsuite/gas/riscv/rounding-fcvt.q.l.s | 5 +++
opcodes/riscv-dis.c | 22 ++++++++++
opcodes/riscv-opc.c | 26 ++++++-----
13 files changed, 177 insertions(+), 15 deletions(-)
create mode 100644 gas/testsuite/gas/riscv/rounding-dis-widening-noalias.d
create mode 100644 gas/testsuite/gas/riscv/rounding-dis-widening.d
create mode 100644 gas/testsuite/gas/riscv/rounding-dis-widening.s
create mode 100644 gas/testsuite/gas/riscv/rounding-fcvt.q.l-noalias.d
create mode 100644 gas/testsuite/gas/riscv/rounding-fcvt.q.l.d
create mode 100644 gas/testsuite/gas/riscv/rounding-fcvt.q.l.l
create mode 100644 gas/testsuite/gas/riscv/rounding-fcvt.q.l.s
diff --git a/gas/config/tc-riscv.c b/gas/config/tc-riscv.c
index bb0e18ac8d52..f0f531039415 100644
--- a/gas/config/tc-riscv.c
+++ b/gas/config/tc-riscv.c
@@ -1396,6 +1396,15 @@ validate_riscv_insn (const struct riscv_opcode *opc, int length)
goto unknown_validate_operand;
}
break;
+ case 'f':
+ switch (*++oparg)
+ {
+ case 'M': /* Fall through. */
+ case 'm': USE_BITS (OP_MASK_RM, OP_SH_RM); break;
+ default:
+ goto unknown_validate_operand;
+ }
+ break;
default:
goto unknown_validate_operand;
}
@@ -3442,6 +3451,40 @@ riscv_ip (char *str, struct riscv_cl_insn *ip, expressionS *imm_expr,
goto unknown_riscv_ip_operand;
}
break;
+ case 'f':
+ switch (*++oparg)
+ {
+ case 'M':
+ case 'm':
+ /* Optional rounding mode (widening conversion)
+ 'M': operand either disallowed or not recommended
+ (considered to be non-useful to normal software).
+ 'm': operand allowed for compatibility reasons
+ (display a warning instead). */
+ if (*asarg == '\0')
+ {
+ INSERT_OPERAND (RM, *ip, 0);
+ continue;
+ }
+ else if (*asarg == ',' && asarg++
+ && arg_lookup (&asarg, riscv_rm,
+ ARRAY_SIZE (riscv_rm), ®no))
+ {
+ INSERT_OPERAND (RM, *ip, regno);
+ if (*oparg == 'M')
+ as_bad (_ ("rounding mode cannot be specified "
+ "on widening conversion"));
+ else
+ as_warn (
+ _ ("specifying a rounding mode is strongly "
+ "discourged on widening conversion"));
+ continue;
+ }
+ break;
+ default:
+ goto unknown_riscv_ip_operand;
+ }
+ break;
default:
goto unknown_riscv_ip_operand;
}
diff --git a/gas/testsuite/gas/riscv/rounding-dis-widening-noalias.d b/gas/testsuite/gas/riscv/rounding-dis-widening-noalias.d
new file mode 100644
index 000000000000..3330b1db83db
--- /dev/null
+++ b/gas/testsuite/gas/riscv/rounding-dis-widening-noalias.d
@@ -0,0 +1,13 @@
+#as: -march=rv32ifd
+#source: rounding-dis-widening.s
+#objdump: -d -M no-aliases
+
+.*:[ ]+file format .*
+
+
+Disassembly of section .text:
+
+0+000 <target>:
+[ ]+[0-9a-f]+:[ ]+420100d3[ ]+fcvt\.d\.s[ ]+ft1,ft2
+[ ]+[0-9a-f]+:[ ]+420100d3[ ]+fcvt\.d\.s[ ]+ft1,ft2
+[ ]+[0-9a-f]+:[ ]+420170d3[ ]+fcvt\.d\.s[ ]+ft1,ft2,dyn
diff --git a/gas/testsuite/gas/riscv/rounding-dis-widening.d b/gas/testsuite/gas/riscv/rounding-dis-widening.d
new file mode 100644
index 000000000000..8fb31ab39efa
--- /dev/null
+++ b/gas/testsuite/gas/riscv/rounding-dis-widening.d
@@ -0,0 +1,13 @@
+#as: -march=rv32ifd
+#source: rounding-dis-widening.s
+#objdump: -d
+
+.*:[ ]+file format .*
+
+
+Disassembly of section .text:
+
+0+000 <target>:
+[ ]+[0-9a-f]+:[ ]+420100d3[ ]+fcvt\.d\.s[ ]+ft1,ft2
+[ ]+[0-9a-f]+:[ ]+420100d3[ ]+fcvt\.d\.s[ ]+ft1,ft2
+[ ]+[0-9a-f]+:[ ]+420170d3[ ]+fcvt\.d\.s[ ]+ft1,ft2
diff --git a/gas/testsuite/gas/riscv/rounding-dis-widening.s b/gas/testsuite/gas/riscv/rounding-dis-widening.s
new file mode 100644
index 000000000000..17443b0a0343
--- /dev/null
+++ b/gas/testsuite/gas/riscv/rounding-dis-widening.s
@@ -0,0 +1,8 @@
+target:
+ fcvt.d.s ft1, ft2
+ # Standard encoding:
+ # - 2nd operand is the rounding mode (RNE [0b000] is preferred).
+ # - 6th operand (additional function) is zero for FCVT.D.S.
+ .insn r OP_FP, 0x0, 0x21, ft1, ft2, f0
+ # Non-standard encoding
+ .insn r OP_FP, 0x7, 0x21, ft1, ft2, f0
diff --git a/gas/testsuite/gas/riscv/rounding-fail.d b/gas/testsuite/gas/riscv/rounding-fail.d
index 0d0a55818caf..7857ab75145c 100644
--- a/gas/testsuite/gas/riscv/rounding-fail.d
+++ b/gas/testsuite/gas/riscv/rounding-fail.d
@@ -1,3 +1,3 @@
-#as: -march=rv32ifd
+#as: -march=rv32ifdq_zfh
#source: rounding-fail.s
#error_output: rounding-fail.l
diff --git a/gas/testsuite/gas/riscv/rounding-fail.l b/gas/testsuite/gas/riscv/rounding-fail.l
index 00d4d8e40fa6..69d359a0eba4 100644
--- a/gas/testsuite/gas/riscv/rounding-fail.l
+++ b/gas/testsuite/gas/riscv/rounding-fail.l
@@ -3,3 +3,14 @@
.*: Error: illegal operands `fadd.d fa1,fa1,fa1,'
.*: Error: illegal operands `fadd.s fa1,fa1,fa1,unknown'
.*: Error: illegal operands `fadd.d fa1,fa1,fa1,unknown'
+.*: Error: rounding mode cannot be specified on widening conversion
+.*: Error: rounding mode cannot be specified on widening conversion
+.*: Error: rounding mode cannot be specified on widening conversion
+.*: Error: rounding mode cannot be specified on widening conversion
+.*: Error: rounding mode cannot be specified on widening conversion
+.*: Error: rounding mode cannot be specified on widening conversion
+.*: Error: rounding mode cannot be specified on widening conversion
+.*: Error: rounding mode cannot be specified on widening conversion
+.*: Error: rounding mode cannot be specified on widening conversion
+.*: Error: rounding mode cannot be specified on widening conversion
+.*: Error: illegal operands `fcvt\.q\.wu ft1,t0,unknown'
diff --git a/gas/testsuite/gas/riscv/rounding-fail.s b/gas/testsuite/gas/riscv/rounding-fail.s
index 6e05cbd410c9..75f9fe1965d1 100644
--- a/gas/testsuite/gas/riscv/rounding-fail.s
+++ b/gas/testsuite/gas/riscv/rounding-fail.s
@@ -4,3 +4,19 @@ target:
fadd.d fa1,fa1,fa1,
fadd.s fa1,fa1,fa1,unknown
fadd.d fa1,fa1,fa1,unknown
+
+ # Rounding mode cannot be specified on widening conversion
+ # unless we have supported in the past.
+ fcvt.s.h ft1,ft2,dyn
+ fcvt.d.h ft1,ft2,dyn
+ fcvt.q.h ft1,ft2,dyn
+ fcvt.d.s ft1,ft2,dyn
+ fcvt.q.s ft1,ft2,dyn
+ fcvt.q.d ft1,ft2,dyn
+ fcvt.d.w ft1,t0,dyn
+ fcvt.d.wu ft1,t0,dyn
+ fcvt.q.w ft1,t0,dyn
+ fcvt.q.wu ft1,t0,dyn
+
+ # Different error message because of an invalid rounding mode
+ fcvt.q.wu ft1,t0,unknown
diff --git a/gas/testsuite/gas/riscv/rounding-fcvt.q.l-noalias.d b/gas/testsuite/gas/riscv/rounding-fcvt.q.l-noalias.d
new file mode 100644
index 000000000000..6f7a10f6c755
--- /dev/null
+++ b/gas/testsuite/gas/riscv/rounding-fcvt.q.l-noalias.d
@@ -0,0 +1,15 @@
+#as: -march=rv64ifdq
+#source: rounding-fcvt.q.l.s
+#warning_output: rounding-fcvt.q.l.l
+#objdump: -d -M no-aliases
+
+.*:[ ]+file format .*
+
+
+Disassembly of section .text:
+
+0+000 <target>:
+[ ]+[0-9a-f]+:[ ]+d62280d3[ ]+fcvt\.q\.l[ ]+ft1,t0
+[ ]+[0-9a-f]+:[ ]+d622f0d3[ ]+fcvt\.q\.l[ ]+ft1,t0,dyn
+[ ]+[0-9a-f]+:[ ]+d63280d3[ ]+fcvt\.q\.lu[ ]+ft1,t0
+[ ]+[0-9a-f]+:[ ]+d632f0d3[ ]+fcvt\.q\.lu[ ]+ft1,t0,dyn
diff --git a/gas/testsuite/gas/riscv/rounding-fcvt.q.l.d b/gas/testsuite/gas/riscv/rounding-fcvt.q.l.d
new file mode 100644
index 000000000000..80b6320e873f
--- /dev/null
+++ b/gas/testsuite/gas/riscv/rounding-fcvt.q.l.d
@@ -0,0 +1,15 @@
+#as: -march=rv64ifdq
+#source: rounding-fcvt.q.l.s
+#warning_output: rounding-fcvt.q.l.l
+#objdump: -d
+
+.*:[ ]+file format .*
+
+
+Disassembly of section .text:
+
+0+000 <target>:
+[ ]+[0-9a-f]+:[ ]+d62280d3[ ]+fcvt\.q\.l[ ]+ft1,t0
+[ ]+[0-9a-f]+:[ ]+d622f0d3[ ]+fcvt\.q\.l[ ]+ft1,t0
+[ ]+[0-9a-f]+:[ ]+d63280d3[ ]+fcvt\.q\.lu[ ]+ft1,t0
+[ ]+[0-9a-f]+:[ ]+d632f0d3[ ]+fcvt\.q\.lu[ ]+ft1,t0
diff --git a/gas/testsuite/gas/riscv/rounding-fcvt.q.l.l b/gas/testsuite/gas/riscv/rounding-fcvt.q.l.l
new file mode 100644
index 000000000000..22df262891e8
--- /dev/null
+++ b/gas/testsuite/gas/riscv/rounding-fcvt.q.l.l
@@ -0,0 +1,3 @@
+.*: Assembler messages:
+.*: Warning: specifying a rounding mode is strongly discourged on widening conversion
+.*: Warning: specifying a rounding mode is strongly discourged on widening conversion
diff --git a/gas/testsuite/gas/riscv/rounding-fcvt.q.l.s b/gas/testsuite/gas/riscv/rounding-fcvt.q.l.s
new file mode 100644
index 000000000000..ec60e53f7ad1
--- /dev/null
+++ b/gas/testsuite/gas/riscv/rounding-fcvt.q.l.s
@@ -0,0 +1,5 @@
+target:
+ fcvt.q.l ft1,t0
+ fcvt.q.l ft1,t0,dyn
+ fcvt.q.lu ft1,t0
+ fcvt.q.lu ft1,t0,dyn
diff --git a/opcodes/riscv-dis.c b/opcodes/riscv-dis.c
index 1e6716e8e58c..e4d966a118df 100644
--- a/opcodes/riscv-dis.c
+++ b/opcodes/riscv-dis.c
@@ -592,6 +592,27 @@ print_insn_args (const char *oparg, insn_t l, bfd_vma pc, disassemble_info *info
goto undefined_modifier;
}
break;
+ case 'f':
+ switch (*++oparg)
+ {
+ case 'M': /* Fall through. */
+ case 'm':
+ /* Optional rounding mode (widening conversion)
+ which defaults to RNE (0b000).
+ Display non-default rounding mode if:
+ 1. rounding mode is invalid or
+ 2. 'no-aliases' option is specified. */
+ if (EXTRACT_OPERAND (RM, l) == 0
+ || (!no_aliases && riscv_rm[EXTRACT_OPERAND (RM, l)]))
+ break;
+ print (info->stream, dis_style_text, ",");
+ arg_print (info, EXTRACT_OPERAND (RM, l), riscv_rm,
+ ARRAY_SIZE (riscv_rm));
+ break;
+ default:
+ goto undefined_modifier;
+ }
+ break;
default:
goto undefined_modifier;
}
@@ -640,6 +661,7 @@ print_insn_args (const char *oparg, insn_t l, bfd_vma pc, disassemble_info *info
}
}
break;
+
default:
undefined_modifier:
/* xgettext:c-format */
diff --git a/opcodes/riscv-opc.c b/opcodes/riscv-opc.c
index 653eb60f2a58..dbfb4f3918b2 100644
--- a/opcodes/riscv-opc.c
+++ b/opcodes/riscv-opc.c
@@ -659,9 +659,9 @@ const struct riscv_opcode riscv_opcodes[] =
{"fcvt.h.w", 0, INSN_CLASS_ZFH_INX, "D,s,m", MATCH_FCVT_H_W, MASK_FCVT_H_W, match_opcode, 0 },
{"fcvt.h.wu", 0, INSN_CLASS_ZFH_INX, "D,s", MATCH_FCVT_H_WU|MASK_RM, MASK_FCVT_H_WU|MASK_RM, match_opcode, 0 },
{"fcvt.h.wu", 0, INSN_CLASS_ZFH_INX, "D,s,m", MATCH_FCVT_H_WU, MASK_FCVT_H_WU, match_opcode, 0 },
-{"fcvt.s.h", 0, INSN_CLASS_ZFHMIN_INX, "D,S", MATCH_FCVT_S_H, MASK_FCVT_S_H|MASK_RM, match_opcode, 0 },
-{"fcvt.d.h", 0, INSN_CLASS_ZFHMIN_AND_D_INX, "D,S", MATCH_FCVT_D_H, MASK_FCVT_D_H|MASK_RM, match_opcode, 0 },
-{"fcvt.q.h", 0, INSN_CLASS_ZFHMIN_AND_Q_INX, "D,S", MATCH_FCVT_Q_H, MASK_FCVT_Q_H|MASK_RM, match_opcode, 0 },
+{"fcvt.s.h", 0, INSN_CLASS_ZFHMIN_INX, "D,SWfM", MATCH_FCVT_S_H, MASK_FCVT_S_H, match_opcode, 0 },
+{"fcvt.d.h", 0, INSN_CLASS_ZFHMIN_AND_D_INX, "D,SWfM", MATCH_FCVT_D_H, MASK_FCVT_D_H, match_opcode, 0 },
+{"fcvt.q.h", 0, INSN_CLASS_ZFHMIN_AND_Q_INX, "D,SWfM", MATCH_FCVT_Q_H, MASK_FCVT_Q_H, match_opcode, 0 },
{"fcvt.h.s", 0, INSN_CLASS_ZFHMIN_INX, "D,S", MATCH_FCVT_H_S|MASK_RM, MASK_FCVT_H_S|MASK_RM, match_opcode, 0 },
{"fcvt.h.s", 0, INSN_CLASS_ZFHMIN_INX, "D,S,m", MATCH_FCVT_H_S, MASK_FCVT_H_S, match_opcode, 0 },
{"fcvt.h.d", 0, INSN_CLASS_ZFHMIN_AND_D_INX, "D,S", MATCH_FCVT_H_D|MASK_RM, MASK_FCVT_H_D|MASK_RM, match_opcode, 0 },
@@ -800,9 +800,9 @@ const struct riscv_opcode riscv_opcodes[] =
{"fcvt.w.d", 0, INSN_CLASS_D_INX, "d,S,m", MATCH_FCVT_W_D, MASK_FCVT_W_D, match_opcode, 0 },
{"fcvt.wu.d", 0, INSN_CLASS_D_INX, "d,S", MATCH_FCVT_WU_D|MASK_RM, MASK_FCVT_WU_D|MASK_RM, match_opcode, 0 },
{"fcvt.wu.d", 0, INSN_CLASS_D_INX, "d,S,m", MATCH_FCVT_WU_D, MASK_FCVT_WU_D, match_opcode, 0 },
-{"fcvt.d.w", 0, INSN_CLASS_D_INX, "D,s", MATCH_FCVT_D_W, MASK_FCVT_D_W|MASK_RM, match_opcode, 0 },
-{"fcvt.d.wu", 0, INSN_CLASS_D_INX, "D,s", MATCH_FCVT_D_WU, MASK_FCVT_D_WU|MASK_RM, match_opcode, 0 },
-{"fcvt.d.s", 0, INSN_CLASS_D_INX, "D,S", MATCH_FCVT_D_S, MASK_FCVT_D_S|MASK_RM, match_opcode, 0 },
+{"fcvt.d.w", 0, INSN_CLASS_D_INX, "D,sWfM", MATCH_FCVT_D_W, MASK_FCVT_D_W, match_opcode, 0 },
+{"fcvt.d.wu", 0, INSN_CLASS_D_INX, "D,sWfM", MATCH_FCVT_D_WU, MASK_FCVT_D_WU, match_opcode, 0 },
+{"fcvt.d.s", 0, INSN_CLASS_D_INX, "D,SWfM", MATCH_FCVT_D_S, MASK_FCVT_D_S, match_opcode, 0 },
{"fcvt.s.d", 0, INSN_CLASS_D_INX, "D,S", MATCH_FCVT_S_D|MASK_RM, MASK_FCVT_S_D|MASK_RM, match_opcode, 0 },
{"fcvt.s.d", 0, INSN_CLASS_D_INX, "D,S,m", MATCH_FCVT_S_D, MASK_FCVT_S_D, match_opcode, 0 },
{"fclass.d", 0, INSN_CLASS_D_INX, "d,S", MATCH_FCLASS_D, MASK_FCLASS_D, match_opcode, 0 },
@@ -857,10 +857,10 @@ const struct riscv_opcode riscv_opcodes[] =
{"fcvt.w.q", 0, INSN_CLASS_Q_INX, "d,S,m", MATCH_FCVT_W_Q, MASK_FCVT_W_Q, match_opcode, 0 },
{"fcvt.wu.q", 0, INSN_CLASS_Q_INX, "d,S", MATCH_FCVT_WU_Q|MASK_RM, MASK_FCVT_WU_Q|MASK_RM, match_opcode, 0 },
{"fcvt.wu.q", 0, INSN_CLASS_Q_INX, "d,S,m", MATCH_FCVT_WU_Q, MASK_FCVT_WU_Q, match_opcode, 0 },
-{"fcvt.q.w", 0, INSN_CLASS_Q_INX, "D,s", MATCH_FCVT_Q_W, MASK_FCVT_Q_W|MASK_RM, match_opcode, 0 },
-{"fcvt.q.wu", 0, INSN_CLASS_Q_INX, "D,s", MATCH_FCVT_Q_WU, MASK_FCVT_Q_WU|MASK_RM, match_opcode, 0 },
-{"fcvt.q.s", 0, INSN_CLASS_Q_INX, "D,S", MATCH_FCVT_Q_S, MASK_FCVT_Q_S|MASK_RM, match_opcode, 0 },
-{"fcvt.q.d", 0, INSN_CLASS_Q_INX, "D,S", MATCH_FCVT_Q_D, MASK_FCVT_Q_D|MASK_RM, match_opcode, 0 },
+{"fcvt.q.w", 0, INSN_CLASS_Q_INX, "D,sWfM", MATCH_FCVT_Q_W, MASK_FCVT_Q_W, match_opcode, 0 },
+{"fcvt.q.wu", 0, INSN_CLASS_Q_INX, "D,sWfM", MATCH_FCVT_Q_WU, MASK_FCVT_Q_WU, match_opcode, 0 },
+{"fcvt.q.s", 0, INSN_CLASS_Q_INX, "D,SWfM", MATCH_FCVT_Q_S, MASK_FCVT_Q_S, match_opcode, 0 },
+{"fcvt.q.d", 0, INSN_CLASS_Q_INX, "D,SWfM", MATCH_FCVT_Q_D, MASK_FCVT_Q_D, match_opcode, 0 },
{"fcvt.s.q", 0, INSN_CLASS_Q_INX, "D,S", MATCH_FCVT_S_Q|MASK_RM, MASK_FCVT_S_Q|MASK_RM, match_opcode, 0 },
{"fcvt.s.q", 0, INSN_CLASS_Q_INX, "D,S,m", MATCH_FCVT_S_Q, MASK_FCVT_S_Q, match_opcode, 0 },
{"fcvt.d.q", 0, INSN_CLASS_Q_INX, "D,S", MATCH_FCVT_D_Q|MASK_RM, MASK_FCVT_D_Q|MASK_RM, match_opcode, 0 },
@@ -875,10 +875,8 @@ const struct riscv_opcode riscv_opcodes[] =
{"fcvt.l.q", 64, INSN_CLASS_Q_INX, "d,S,m", MATCH_FCVT_L_Q, MASK_FCVT_L_Q, match_opcode, 0 },
{"fcvt.lu.q", 64, INSN_CLASS_Q_INX, "d,S", MATCH_FCVT_LU_Q|MASK_RM, MASK_FCVT_LU_Q|MASK_RM, match_opcode, 0 },
{"fcvt.lu.q", 64, INSN_CLASS_Q_INX, "d,S,m", MATCH_FCVT_LU_Q, MASK_FCVT_LU_Q, match_opcode, 0 },
-{"fcvt.q.l", 64, INSN_CLASS_Q_INX, "D,s", MATCH_FCVT_Q_L, MASK_FCVT_Q_L|MASK_RM, match_opcode, 0 },
-{"fcvt.q.l", 64, INSN_CLASS_Q_INX, "D,s,m", MATCH_FCVT_Q_L, MASK_FCVT_Q_L, match_opcode, 0 },
-{"fcvt.q.lu", 64, INSN_CLASS_Q_INX, "D,s", MATCH_FCVT_Q_LU, MASK_FCVT_Q_LU|MASK_RM, match_opcode, 0 },
-{"fcvt.q.lu", 64, INSN_CLASS_Q_INX, "D,s,m", MATCH_FCVT_Q_LU, MASK_FCVT_Q_LU, match_opcode, 0 },
+{"fcvt.q.l", 64, INSN_CLASS_Q_INX, "D,sWfm", MATCH_FCVT_Q_L, MASK_FCVT_Q_L, match_opcode, 0 },
+{"fcvt.q.lu", 64, INSN_CLASS_Q_INX, "D,sWfm", MATCH_FCVT_Q_LU, MASK_FCVT_Q_LU, match_opcode, 0 },
/* Compressed instructions. */
{"c.unimp", 0, INSN_CLASS_C, "", 0, 0xffffU, match_opcode, 0 },
--
2.38.1
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/3] RISC-V: Allocate "various" operand type
2022-11-28 6:39 ` [PATCH 1/3] RISC-V: Allocate "various" operand type Tsukasa OI
@ 2022-11-29 2:42 ` Nelson Chu
2022-11-29 3:19 ` Tsukasa OI
0 siblings, 1 reply; 6+ messages in thread
From: Nelson Chu @ 2022-11-29 2:42 UTC (permalink / raw)
To: Tsukasa OI; +Cc: Kito Cheng, Palmer Dabbelt, binutils
On Mon, Nov 28, 2022 at 2:39 PM Tsukasa OI <research_trasio@irq.a4lg.com> wrote:
>
> From: Tsukasa OI <research_trasio@irq.a4lg.com>
>
> This commit intends to move operands that require very special handling or
> operand types that are so minor (e.g. only useful on a few instructions)
> under "W". I also intend this "W" to be "temporary" operand storage until
> we can find good two character (or less) operand type.
>
> In this commit, prefetch offset operand "f" for 'Zicbop' extension is moved
> to "Wif" because of its special handling (and allocating single character
> "f" for this operand type seemed too much).
>
> Current expected allocation guideline is as follows:
>
> 1. 'W'
> 2. The most closely related single-letter extension in lowercase
> (strongly recommended but not mandatory)
> 3. Identify operand type
>
> The author currently plans to allocate following three-character operand
> types (for operands including instructions from unratified extensions).
>
> 1. "Wif" ('Zicbop': fetch offset)
Maybe just "if" without the W? W seems redundant. If the offset is
immediate then using "i + <xxx>" seems more clear to understand.
> 2. "Wfv" (unratified 'Zfa': value operand from FLI.[HSDQ] instructions)
We probably also need to modify other old operand names to comply with
the new naming rules. But for now the current operand names seem
enough, maybe we can find better naming rules in the future, it's not
urgent.
> 3. "Wfm" / "WfM"
> 'Zfh', 'F', 'D', 'Q': rounding modes "m" with special handling
> solely for widening conversion instructions.
I still think these similar checks, including the rounding mode
checks, should be checked in different match_opcode functions, that
should be enough. There is no way to make assembly wrong until you
are using the .insn directives, and we don't need to check if the
operands are valid or not for them, since .insn is used to allow users
to write an unsupported instruction. Please don't continue trying to
add various operand names to support these, it is really hard to
maintain. I should have already said that, so this is the last time I
mention it.
Nelson
> gas/ChangeLog:
>
> * config/tc-riscv.c (validate_riscv_insn, riscv_ip): Move from
> "f" to "Wif".
>
> opcodes/ChangeLog:
>
> * riscv-dis.c (print_insn_args): Move from "f" to "Wif".
> * riscv-opc.c (riscv_opcodes): Reflect new operand type.
> ---
> gas/config/tc-riscv.c | 64 +++++++++++++++++++++++++++++++------------
> opcodes/riscv-dis.c | 26 ++++++++++++++----
> opcodes/riscv-opc.c | 6 ++--
> 3 files changed, 71 insertions(+), 25 deletions(-)
>
> diff --git a/gas/config/tc-riscv.c b/gas/config/tc-riscv.c
> index 0682eb355241..bb0e18ac8d52 100644
> --- a/gas/config/tc-riscv.c
> +++ b/gas/config/tc-riscv.c
> @@ -1359,7 +1359,6 @@ validate_riscv_insn (const struct riscv_opcode *opc, int length)
> case 'j': used_bits |= ENCODE_ITYPE_IMM (-1U); break;
> case 'a': used_bits |= ENCODE_JTYPE_IMM (-1U); break;
> case 'p': used_bits |= ENCODE_BTYPE_IMM (-1U); break;
> - case 'f': /* Fall through. */
> case 'q': used_bits |= ENCODE_STYPE_IMM (-1U); break;
> case 'u': used_bits |= ENCODE_UTYPE_IMM (-1U); break;
> case 'z': break; /* Zero immediate. */
> @@ -1386,6 +1385,21 @@ validate_riscv_insn (const struct riscv_opcode *opc, int length)
> goto unknown_validate_operand;
> }
> break;
> + case 'W': /* Various operands. */
> + switch (*++oparg)
> + {
> + case 'i':
> + switch (*++oparg)
> + {
> + case 'f': used_bits |= ENCODE_STYPE_IMM (-1U); break;
> + default:
> + goto unknown_validate_operand;
> + }
> + break;
> + default:
> + goto unknown_validate_operand;
> + }
> + break;
> case 'X': /* Integer immediate. */
> {
> size_t n;
> @@ -3401,22 +3415,37 @@ riscv_ip (char *str, struct riscv_cl_insn *ip, expressionS *imm_expr,
> imm_expr->X_op = O_absent;
> continue;
>
> - case 'f': /* Prefetch offset, pseudo S-type but lower 5-bits zero. */
> - if (riscv_handle_implicit_zero_offset (imm_expr, asarg))
> - continue;
> - my_getExpression (imm_expr, asarg);
> - check_absolute_expr (ip, imm_expr, false);
> - if (((unsigned) (imm_expr->X_add_number) & 0x1fU)
> - || imm_expr->X_add_number >= (signed) RISCV_IMM_REACH / 2
> - || imm_expr->X_add_number < -(signed) RISCV_IMM_REACH / 2)
> - as_bad (_("improper prefetch offset (%ld)"),
> - (long) imm_expr->X_add_number);
> - ip->insn_opcode |=
> - ENCODE_STYPE_IMM ((unsigned) (imm_expr->X_add_number) &
> - ~ 0x1fU);
> - imm_expr->X_op = O_absent;
> - asarg = expr_end;
> - continue;
> + case 'W': /* Various operands. */
> + switch (*++oparg)
> + {
> + case 'i':
> + switch (*++oparg)
> + {
> + case 'f':
> + /* Prefetch offset for 'Zicbop' extension.
> + pseudo S-type but lower 5-bits zero. */
> + if (riscv_handle_implicit_zero_offset (imm_expr, asarg))
> + continue;
> + my_getExpression (imm_expr, asarg);
> + check_absolute_expr (ip, imm_expr, false);
> + if (((unsigned) (imm_expr->X_add_number) & 0x1fU)
> + || imm_expr->X_add_number >= RISCV_IMM_REACH / 2
> + || imm_expr->X_add_number < -RISCV_IMM_REACH / 2)
> + as_bad (_ ("improper prefetch offset (%ld)"),
> + (long) imm_expr->X_add_number);
> + ip->insn_opcode |= ENCODE_STYPE_IMM (
> + (unsigned) (imm_expr->X_add_number) & ~0x1fU);
> + imm_expr->X_op = O_absent;
> + asarg = expr_end;
> + continue;
> + default:
> + goto unknown_riscv_ip_operand;
> + }
> + break;
> + default:
> + goto unknown_riscv_ip_operand;
> + }
> + break;
>
> case 'X': /* Integer immediate. */
> {
> @@ -3469,6 +3498,7 @@ riscv_ip (char *str, struct riscv_cl_insn *ip, expressionS *imm_expr,
> }
> }
> break;
> +
> default:
> unknown_riscv_ip_operand:
> as_fatal (_("internal: unknown argument type `%s'"),
> diff --git a/opcodes/riscv-dis.c b/opcodes/riscv-dis.c
> index 0e1f3b4610aa..1e6716e8e58c 100644
> --- a/opcodes/riscv-dis.c
> +++ b/opcodes/riscv-dis.c
> @@ -473,11 +473,6 @@ print_insn_args (const char *oparg, insn_t l, bfd_vma pc, disassemble_info *info
> (int)EXTRACT_STYPE_IMM (l));
> break;
>
> - case 'f':
> - print (info->stream, dis_style_address_offset, "%d",
> - (int)EXTRACT_STYPE_IMM (l));
> - break;
> -
> case 'a':
> info->target = EXTRACT_JTYPE_IMM (l) + pc;
> (*info->print_address_func) (info->target, info);
> @@ -582,6 +577,27 @@ print_insn_args (const char *oparg, insn_t l, bfd_vma pc, disassemble_info *info
> print (info->stream, dis_style_immediate, "%d", rs1);
> break;
>
> + case 'W': /* Various operands. */
> + {
> + switch (*++oparg)
> + {
> + case 'i':
> + switch (*++oparg)
> + {
> + case 'f':
> + print (info->stream, dis_style_address_offset, "%d",
> + (int) EXTRACT_STYPE_IMM (l));
> + break;
> + default:
> + goto undefined_modifier;
> + }
> + break;
> + default:
> + goto undefined_modifier;
> + }
> + }
> + break;
> +
> case 'X': /* Integer immediate. */
> {
> size_t n;
> diff --git a/opcodes/riscv-opc.c b/opcodes/riscv-opc.c
> index 0e691544f9bc..653eb60f2a58 100644
> --- a/opcodes/riscv-opc.c
> +++ b/opcodes/riscv-opc.c
> @@ -313,9 +313,9 @@ const struct riscv_opcode riscv_opcodes[] =
> /* name, xlen, isa, operands, match, mask, match_func, pinfo. */
>
> /* Standard hints. */
> -{"prefetch.i", 0, INSN_CLASS_ZICBOP, "f(s)", MATCH_PREFETCH_I, MASK_PREFETCH_I, match_opcode, 0 },
> -{"prefetch.r", 0, INSN_CLASS_ZICBOP, "f(s)", MATCH_PREFETCH_R, MASK_PREFETCH_R, match_opcode, 0 },
> -{"prefetch.w", 0, INSN_CLASS_ZICBOP, "f(s)", MATCH_PREFETCH_W, MASK_PREFETCH_W, match_opcode, 0 },
> +{"prefetch.i", 0, INSN_CLASS_ZICBOP, "Wif(s)", MATCH_PREFETCH_I, MASK_PREFETCH_I, match_opcode, 0 },
> +{"prefetch.r", 0, INSN_CLASS_ZICBOP, "Wif(s)", MATCH_PREFETCH_R, MASK_PREFETCH_R, match_opcode, 0 },
> +{"prefetch.w", 0, INSN_CLASS_ZICBOP, "Wif(s)", MATCH_PREFETCH_W, MASK_PREFETCH_W, match_opcode, 0 },
> {"pause", 0, INSN_CLASS_ZIHINTPAUSE, "", MATCH_PAUSE, MASK_PAUSE, match_opcode, 0 },
>
> /* Basic RVI instructions and aliases. */
> --
> 2.38.1
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/3] RISC-V: Allocate "various" operand type
2022-11-29 2:42 ` Nelson Chu
@ 2022-11-29 3:19 ` Tsukasa OI
0 siblings, 0 replies; 6+ messages in thread
From: Tsukasa OI @ 2022-11-29 3:19 UTC (permalink / raw)
To: Nelson Chu; +Cc: binutils
Nelson,
Before the real reply...
- The reply is going to be pretty long.
- I'm not going to reply to you today to stop myself from griping
(not to be led by my frustrations)
Meanwhile, reviewing
<https://sourceware.org/pipermail/binutils/2022-November/124693.html>
(at least, PATCH 1-7/11 should be easy to review and partial approval of
PATCH 1-7/11 is okay to me) would be nice.
Sincerely,
Tsukasa
On 2022/11/29 11:42, Nelson Chu wrote:
> On Mon, Nov 28, 2022 at 2:39 PM Tsukasa OI <research_trasio@irq.a4lg.com> wrote:
>>
>> From: Tsukasa OI <research_trasio@irq.a4lg.com>
>>
>> This commit intends to move operands that require very special handling or
>> operand types that are so minor (e.g. only useful on a few instructions)
>> under "W". I also intend this "W" to be "temporary" operand storage until
>> we can find good two character (or less) operand type.
>>
>> In this commit, prefetch offset operand "f" for 'Zicbop' extension is moved
>> to "Wif" because of its special handling (and allocating single character
>> "f" for this operand type seemed too much).
>>
>> Current expected allocation guideline is as follows:
>>
>> 1. 'W'
>> 2. The most closely related single-letter extension in lowercase
>> (strongly recommended but not mandatory)
>> 3. Identify operand type
>>
>> The author currently plans to allocate following three-character operand
>> types (for operands including instructions from unratified extensions).
>>
>> 1. "Wif" ('Zicbop': fetch offset)
>
> Maybe just "if" without the W? W seems redundant. If the offset is
> immediate then using "i + <xxx>" seems more clear to understand.
>
>> 2. "Wfv" (unratified 'Zfa': value operand from FLI.[HSDQ] instructions)
>
> We probably also need to modify other old operand names to comply with
> the new naming rules. But for now the current operand names seem
> enough, maybe we can find better naming rules in the future, it's not
> urgent.
>
>> 3. "Wfm" / "WfM"
>> 'Zfh', 'F', 'D', 'Q': rounding modes "m" with special handling
>> solely for widening conversion instructions.
>
> I still think these similar checks, including the rounding mode
> checks, should be checked in different match_opcode functions, that
> should be enough. There is no way to make assembly wrong until you
> are using the .insn directives, and we don't need to check if the
> operands are valid or not for them, since .insn is used to allow users
> to write an unsupported instruction. Please don't continue trying to
> add various operand names to support these, it is really hard to
> maintain. I should have already said that, so this is the last time I
> mention it.
>
> Nelson
>
>> gas/ChangeLog:
>>
>> * config/tc-riscv.c (validate_riscv_insn, riscv_ip): Move from
>> "f" to "Wif".
>>
>> opcodes/ChangeLog:
>>
>> * riscv-dis.c (print_insn_args): Move from "f" to "Wif".
>> * riscv-opc.c (riscv_opcodes): Reflect new operand type.
>> ---
>> gas/config/tc-riscv.c | 64 +++++++++++++++++++++++++++++++------------
>> opcodes/riscv-dis.c | 26 ++++++++++++++----
>> opcodes/riscv-opc.c | 6 ++--
>> 3 files changed, 71 insertions(+), 25 deletions(-)
>>
>> diff --git a/gas/config/tc-riscv.c b/gas/config/tc-riscv.c
>> index 0682eb355241..bb0e18ac8d52 100644
>> --- a/gas/config/tc-riscv.c
>> +++ b/gas/config/tc-riscv.c
>> @@ -1359,7 +1359,6 @@ validate_riscv_insn (const struct riscv_opcode *opc, int length)
>> case 'j': used_bits |= ENCODE_ITYPE_IMM (-1U); break;
>> case 'a': used_bits |= ENCODE_JTYPE_IMM (-1U); break;
>> case 'p': used_bits |= ENCODE_BTYPE_IMM (-1U); break;
>> - case 'f': /* Fall through. */
>> case 'q': used_bits |= ENCODE_STYPE_IMM (-1U); break;
>> case 'u': used_bits |= ENCODE_UTYPE_IMM (-1U); break;
>> case 'z': break; /* Zero immediate. */
>> @@ -1386,6 +1385,21 @@ validate_riscv_insn (const struct riscv_opcode *opc, int length)
>> goto unknown_validate_operand;
>> }
>> break;
>> + case 'W': /* Various operands. */
>> + switch (*++oparg)
>> + {
>> + case 'i':
>> + switch (*++oparg)
>> + {
>> + case 'f': used_bits |= ENCODE_STYPE_IMM (-1U); break;
>> + default:
>> + goto unknown_validate_operand;
>> + }
>> + break;
>> + default:
>> + goto unknown_validate_operand;
>> + }
>> + break;
>> case 'X': /* Integer immediate. */
>> {
>> size_t n;
>> @@ -3401,22 +3415,37 @@ riscv_ip (char *str, struct riscv_cl_insn *ip, expressionS *imm_expr,
>> imm_expr->X_op = O_absent;
>> continue;
>>
>> - case 'f': /* Prefetch offset, pseudo S-type but lower 5-bits zero. */
>> - if (riscv_handle_implicit_zero_offset (imm_expr, asarg))
>> - continue;
>> - my_getExpression (imm_expr, asarg);
>> - check_absolute_expr (ip, imm_expr, false);
>> - if (((unsigned) (imm_expr->X_add_number) & 0x1fU)
>> - || imm_expr->X_add_number >= (signed) RISCV_IMM_REACH / 2
>> - || imm_expr->X_add_number < -(signed) RISCV_IMM_REACH / 2)
>> - as_bad (_("improper prefetch offset (%ld)"),
>> - (long) imm_expr->X_add_number);
>> - ip->insn_opcode |=
>> - ENCODE_STYPE_IMM ((unsigned) (imm_expr->X_add_number) &
>> - ~ 0x1fU);
>> - imm_expr->X_op = O_absent;
>> - asarg = expr_end;
>> - continue;
>> + case 'W': /* Various operands. */
>> + switch (*++oparg)
>> + {
>> + case 'i':
>> + switch (*++oparg)
>> + {
>> + case 'f':
>> + /* Prefetch offset for 'Zicbop' extension.
>> + pseudo S-type but lower 5-bits zero. */
>> + if (riscv_handle_implicit_zero_offset (imm_expr, asarg))
>> + continue;
>> + my_getExpression (imm_expr, asarg);
>> + check_absolute_expr (ip, imm_expr, false);
>> + if (((unsigned) (imm_expr->X_add_number) & 0x1fU)
>> + || imm_expr->X_add_number >= RISCV_IMM_REACH / 2
>> + || imm_expr->X_add_number < -RISCV_IMM_REACH / 2)
>> + as_bad (_ ("improper prefetch offset (%ld)"),
>> + (long) imm_expr->X_add_number);
>> + ip->insn_opcode |= ENCODE_STYPE_IMM (
>> + (unsigned) (imm_expr->X_add_number) & ~0x1fU);
>> + imm_expr->X_op = O_absent;
>> + asarg = expr_end;
>> + continue;
>> + default:
>> + goto unknown_riscv_ip_operand;
>> + }
>> + break;
>> + default:
>> + goto unknown_riscv_ip_operand;
>> + }
>> + break;
>>
>> case 'X': /* Integer immediate. */
>> {
>> @@ -3469,6 +3498,7 @@ riscv_ip (char *str, struct riscv_cl_insn *ip, expressionS *imm_expr,
>> }
>> }
>> break;
>> +
>> default:
>> unknown_riscv_ip_operand:
>> as_fatal (_("internal: unknown argument type `%s'"),
>> diff --git a/opcodes/riscv-dis.c b/opcodes/riscv-dis.c
>> index 0e1f3b4610aa..1e6716e8e58c 100644
>> --- a/opcodes/riscv-dis.c
>> +++ b/opcodes/riscv-dis.c
>> @@ -473,11 +473,6 @@ print_insn_args (const char *oparg, insn_t l, bfd_vma pc, disassemble_info *info
>> (int)EXTRACT_STYPE_IMM (l));
>> break;
>>
>> - case 'f':
>> - print (info->stream, dis_style_address_offset, "%d",
>> - (int)EXTRACT_STYPE_IMM (l));
>> - break;
>> -
>> case 'a':
>> info->target = EXTRACT_JTYPE_IMM (l) + pc;
>> (*info->print_address_func) (info->target, info);
>> @@ -582,6 +577,27 @@ print_insn_args (const char *oparg, insn_t l, bfd_vma pc, disassemble_info *info
>> print (info->stream, dis_style_immediate, "%d", rs1);
>> break;
>>
>> + case 'W': /* Various operands. */
>> + {
>> + switch (*++oparg)
>> + {
>> + case 'i':
>> + switch (*++oparg)
>> + {
>> + case 'f':
>> + print (info->stream, dis_style_address_offset, "%d",
>> + (int) EXTRACT_STYPE_IMM (l));
>> + break;
>> + default:
>> + goto undefined_modifier;
>> + }
>> + break;
>> + default:
>> + goto undefined_modifier;
>> + }
>> + }
>> + break;
>> +
>> case 'X': /* Integer immediate. */
>> {
>> size_t n;
>> diff --git a/opcodes/riscv-opc.c b/opcodes/riscv-opc.c
>> index 0e691544f9bc..653eb60f2a58 100644
>> --- a/opcodes/riscv-opc.c
>> +++ b/opcodes/riscv-opc.c
>> @@ -313,9 +313,9 @@ const struct riscv_opcode riscv_opcodes[] =
>> /* name, xlen, isa, operands, match, mask, match_func, pinfo. */
>>
>> /* Standard hints. */
>> -{"prefetch.i", 0, INSN_CLASS_ZICBOP, "f(s)", MATCH_PREFETCH_I, MASK_PREFETCH_I, match_opcode, 0 },
>> -{"prefetch.r", 0, INSN_CLASS_ZICBOP, "f(s)", MATCH_PREFETCH_R, MASK_PREFETCH_R, match_opcode, 0 },
>> -{"prefetch.w", 0, INSN_CLASS_ZICBOP, "f(s)", MATCH_PREFETCH_W, MASK_PREFETCH_W, match_opcode, 0 },
>> +{"prefetch.i", 0, INSN_CLASS_ZICBOP, "Wif(s)", MATCH_PREFETCH_I, MASK_PREFETCH_I, match_opcode, 0 },
>> +{"prefetch.r", 0, INSN_CLASS_ZICBOP, "Wif(s)", MATCH_PREFETCH_R, MASK_PREFETCH_R, match_opcode, 0 },
>> +{"prefetch.w", 0, INSN_CLASS_ZICBOP, "Wif(s)", MATCH_PREFETCH_W, MASK_PREFETCH_W, match_opcode, 0 },
>> {"pause", 0, INSN_CLASS_ZIHINTPAUSE, "", MATCH_PAUSE, MASK_PAUSE, match_opcode, 0 },
>>
>> /* Basic RVI instructions and aliases. */
>> --
>> 2.38.1
>>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-11-29 3:19 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-11-28 6:39 [PATCH 0/3] RISC-V: Support non-standard encodings (on widening FP ops) Tsukasa OI
2022-11-28 6:39 ` [PATCH 1/3] RISC-V: Allocate "various" operand type Tsukasa OI
2022-11-29 2:42 ` Nelson Chu
2022-11-29 3:19 ` Tsukasa OI
2022-11-28 6:39 ` [PATCH 2/3] RISC-V: Reorganize invalid rounding mode test Tsukasa OI
2022-11-28 6:39 ` [PATCH 3/3] RISC-V: Rounding mode on widening instructions 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).