Conversation
There was a problem hiding this comment.
3 issues found across 5 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="tests/rvv-smoke.S">
<violation number="1" location="tests/rvv-smoke.S:2008">
P3: fail_vfrec7 assigns exit code 0xa0, which collides with the range the block comment above reserves for 'spec-aware mask/vstart tests' (0xa1-0xa6 are all mask/vstart tests). A future regression returning 0xa0 will be misread as a mask/vstart failure. Use a code outside the 0xaN range, e.g. 0x9c, to keep the documented grouping meaningful.</violation>
</file>
<file name="src/rv32_v_template.c">
<violation number="1" location="src/rv32_v_template.c:7039">
P2: When a masked unary instruction uses `vd=v0`, the loop overwrites mask bits while reading later lanes, making execution depend on earlier results. Reject a destination group overlapping `v0` when `vm=0` before entering the loop.</violation>
<violation number="2" location="src/rv32_v_template.c:7686">
P2: `vfrsqrt7.v` and `vfrec7.v` must not update `fflags`, but these helpers raise flags and this shared wrapper commits them after each vector. Separate the estimate instructions from the flag-committing wrapper while retaining `frm` selection for `vfrec7` overflow handling.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| rvv_set_elem(rv, dest, elem, 32, 0xFFFFFFFFU); | ||
| continue; | ||
| } | ||
| rvv_set_elem(rv, dest, elem, 32, |
There was a problem hiding this comment.
P2: When a masked unary instruction uses vd=v0, the loop overwrites mask bits while reading later lanes, making execution depend on earlier results. Reject a destination group overlapping v0 when vm=0 before entering the loop.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/rv32_v_template.c, line 7039:
<comment>When a masked unary instruction uses `vd=v0`, the loop overwrites mask bits while reading later lanes, making execution depend on earlier results. Reject a destination group overlapping `v0` when `vm=0` before entering the loop.</comment>
<file context>
@@ -6828,6 +7016,36 @@ static inline void rvv_exec_fp32_vf(riscv_t *rv,
+ rvv_set_elem(rv, dest, elem, 32, 0xFFFFFFFFU);
+ continue;
+ }
+ rvv_set_elem(rv, dest, elem, 32,
+ op(rvv_get_elem(rv, ir->vs2, elem, 32)));
+ }
</file context>
| }) | ||
|
|
||
| RVV_FP32_UNARY_OP(vfsqrt_v, rvv_fp_sqrt32, true); | ||
| RVV_FP32_UNARY_OP(vfrsqrt7_v, rvv_fp_rsqrt7_32, false); |
There was a problem hiding this comment.
P2: vfrsqrt7.v and vfrec7.v must not update fflags, but these helpers raise flags and this shared wrapper commits them after each vector. Separate the estimate instructions from the flag-committing wrapper while retaining frm selection for vfrec7 overflow handling.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/rv32_v_template.c, line 7686:
<comment>`vfrsqrt7.v` and `vfrec7.v` must not update `fflags`, but these helpers raise flags and this shared wrapper commits them after each vector. Separate the estimate instructions from the flag-committing wrapper while retaining `frm` selection for `vfrec7` overflow handling.</comment>
<file context>
@@ -7448,6 +7666,27 @@ static inline void rvv_exec_vfmv_v_f(riscv_t *rv,
+ })
+
+RVV_FP32_UNARY_OP(vfsqrt_v, rvv_fp_sqrt32, true);
+RVV_FP32_UNARY_OP(vfrsqrt7_v, rvv_fp_rsqrt7_32, false);
+RVV_FP32_UNARY_OP(vfrec7_v, rvv_fp_rec7_32, true);
+RVV_FP32_UNARY_OP(vfclass_v, rvv_fp_class32, false);
</file context>
| li a3, 0x9f | ||
| j write_and_exit_fail | ||
| fail_vfrec7: | ||
| li a3, 0xa0 |
There was a problem hiding this comment.
P3: fail_vfrec7 assigns exit code 0xa0, which collides with the range the block comment above reserves for 'spec-aware mask/vstart tests' (0xa1-0xa6 are all mask/vstart tests). A future regression returning 0xa0 will be misread as a mask/vstart failure. Use a code outside the 0xaN range, e.g. 0x9c, to keep the documented grouping meaningful.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At tests/rvv-smoke.S, line 2008:
<comment>fail_vfrec7 assigns exit code 0xa0, which collides with the range the block comment above reserves for 'spec-aware mask/vstart tests' (0xa1-0xa6 are all mask/vstart tests). A future regression returning 0xa0 will be misread as a mask/vstart failure. Use a code outside the 0xaN range, e.g. 0x9c, to keep the documented grouping meaningful.</comment>
<file context>
@@ -1911,6 +1995,18 @@ fail_vsmul4:
+ li a3, 0x9f
+ j write_and_exit_fail
+fail_vfrec7:
+ li a3, 0xa0
+ j write_and_exit_fail
fail_vwmulu:
</file context>
| li a3, 0xa0 | |
| li a3, 0x9c |
Add vfsqrt.v, vfrsqrt7.v, vfrec7.v and vfclass.v (V 1.0 sections 13.8-13.14) at the VFUNARY1 decode site. vfrsqrt7 and vfrec7 are table-driven estimates; the two 128-entry tables are transcribed from the specification and kept in the specification's row layout with clang-format disabled around them. Both worked examples given in the specification are covered by the smoke test, along with an even-exponent input, because the two published examples share an odd normalized exponent and cannot distinguish an off-by-one in the output-exponent formula.
Implements the last of the five families in #734.
Adds
vfsqrt.v,vfrsqrt7.v,vfrec7.vandvfclass.v(V 1.0 §13.8-13.14)at the VFUNARY1 decode site (funct6=0x13, funct3=0x1), previously rejected as
reserved.
vfrsqrt7andvfrec7are the table-driven 7-bit estimates. The two128-entry tables are transcribed from the specification's
vfrsqrt7.adocandvfrec7.adoc, and kept in the specification's row layout by disablingclang-format around them, as several existing files already do.
Both instructions were developed against the worked examples in the spec
before being written in C:
vfrsqrt7(0x00718abc) = 0x5f080000,vfrsqrt7(0x7f765432) = 0x1f820000vfrec7(0x00718abc) = 0x7e900000,vfrec7(0x7f765432) = 0x00214000All four are covered by the smoke test. Two details that cost some time and
may be worth noting for review:
vfrec7output the implicit leading one has to berestored before the right shift, otherwise the low bit of the result is
lost (the second worked example catches this);
vfrsqrt7examples both have an odd normalizedexponent, so the floor in
(3*B - 1 - e) / 2is absorbed by integerdivision and an off-by-one there passes both of them. The smoke test adds
an even-exponent input (4.0 -> 0x3eff0000) which does catch it.
make checkpasses; the build was checked in the default,EXT_V,JIT+
EXT_VandEXT_V-without-EXT_Fconfigurations. clang-format 20.1.7clean.
Summary by cubic
Implements the last of the five RVV floating-point unary instruction families in #734 by adding
vfsqrt.v,vfrsqrt7.v,vfrec7.v, andvfclass.v(V 1.0 §13.8–13.14) at the VFUNARY1 decode site, which previously rejected the encoding as reserved.vs1field (00000, 00100, 00101, 10000); both masked and unmasked forms are permitted.vfrsqrt7input (4.0 → 0x3eff0000) is added because both published examples share an odd normalized exponent and would not catch a floor off-by-one in the output-exponent formula.vfrec7output restores the implicit leading one before the right shift, otherwise the low bit is lost (the second worked example exercises this path).vfrsqrt7results are independent of the dynamic rounding mode; onlyvfrec7overflow depends onfrm, so onlyvfsqrtandvfrec7invokervv_fp_begin_round.V_NOPwithoutEXT_V, andmake checkpasses in default,EXT_V, JIT+EXT_V, andEXT_V-without-EXT_Fbuilds.Written for commit 08bdc4d. Summary will update on new commits.