Conversation
There was a problem hiding this comment.
2 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="src/rv32_v_template.c">
<violation number="1" location="src/rv32_v_template.c:6536">
P1: When a masked extension uses `vd = v0`, the loop overwrites the mask register while reading it for later elements, producing incorrect results. Reject destination groups overlapping `v0` when `ir->vm == 0`.</violation>
</file>
<file name="tests/rvv-smoke.S">
<violation number="1" location="tests/rvv-smoke.S:1830">
P3: The new smoke cases never exercise the extension overlap rule: every source/destination pair is disjoint. An incorrect overlap check would pass these tests, so add a legal highest-part overlap case and verify reserved overlaps where possible.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
Comment on lines
+6536
to
+6538
| if (!rvv_eew_reg_span(rv, src_bits, &src_span) || | ||
| !rvv_validate_data_reg(rv->csr_vtype, ir->vd) || | ||
| !rvv_validate_eew_reg(rv, src_bits, ir->vs2)) |
There was a problem hiding this comment.
P1: When a masked extension uses vd = v0, the loop overwrites the mask register while reading it for later elements, producing incorrect results. Reject destination groups overlapping v0 when ir->vm == 0.
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 6536:
<comment>When a masked extension uses `vd = v0`, the loop overwrites the mask register while reading it for later elements, producing incorrect results. Reject destination groups overlapping `v0` when `ir->vm == 0`.</comment>
<file context>
@@ -6499,6 +6499,93 @@ RVOP(vid_v, {
+ */
+ if (src_bits < 8)
+ return rvv_trap_illegal_state(rv, 0);
+ if (!rvv_eew_reg_span(rv, src_bits, &src_span) ||
+ !rvv_validate_data_reg(rv->csr_vtype, ir->vd) ||
+ !rvv_validate_eew_reg(rv, src_bits, ir->vs2))
</file context>
Suggested change
| if (!rvv_eew_reg_span(rv, src_bits, &src_span) || | |
| !rvv_validate_data_reg(rv->csr_vtype, ir->vd) || | |
| !rvv_validate_eew_reg(rv, src_bits, ir->vs2)) | |
| if (!rvv_eew_reg_span(rv, src_bits, &src_span) || | |
| !rvv_validate_data_reg(rv->csr_vtype, ir->vd) || | |
| !rvv_validate_eew_reg(rv, src_bits, ir->vs2) || | |
| (!ir->vm && rvv_reg_spans_overlap(ir->vd, dest_span, 0, 1))) |
| vle8.v v20, (a1) | ||
|
|
||
| # vzext.vf2: 0x7f,0x80,0x01,0xff -> zero-extended to 16-bit | ||
| vzext.vf2 v22, v20 |
There was a problem hiding this comment.
P3: The new smoke cases never exercise the extension overlap rule: every source/destination pair is disjoint. An incorrect overlap check would pass these tests, so add a legal highest-part overlap case and verify reserved overlaps where possible.
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 1830:
<comment>The new smoke cases never exercise the extension overlap rule: every source/destination pair is disjoint. An incorrect overlap check would pass these tests, so add a legal highest-part overlap case and verify reserved overlaps where possible.</comment>
<file context>
@@ -1813,6 +1819,82 @@ _start:
+ vle8.v v20, (a1)
+
+ # vzext.vf2: 0x7f,0x80,0x01,0xff -> zero-extended to 16-bit
+ vzext.vf2 v22, v20
+ la a1, ext_out
+ vse16.v v22, (a1)
</file context>
Add vzext.vf{2,4,8} and vsext.vf{2,4,8} (V 1.0 section 11.3), which widen
a narrow source element into a full SEW-wide destination element.
The overlap rule here is not the one implemented by
rvv_cross_eew_overlap_illegal(): for a destination whose EEW exceeds the
source EEW, section 5.2 requires the source EMUL to be at least 1 and the
overlap to sit in the highest-numbered part of the destination group. The
check is therefore spelled out in the execution helper.
Source EMUL is compared against 1 through rvv_lmul_ratio() rather than the
register span, because rvv_eew_reg_span() clamps a fractional EMUL to a
span of one register.
alanhc
force-pushed
the
rvv-vzext-vsext
branch
from
September 10, 2026 23:13
005c47c to
15453a8
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Implements the integer-extension family from #734.
Adds
vzext.vf{2,4,8}andvsext.vf{2,4,8}(V 1.0 §11.3) at the VXUNARY0decode site, which was previously rejected with a FIXME.
The one non-obvious part is the register-overlap rule.
vzext/vsexthavea destination EEW greater than the source EEW, and §5.2 requires the source
EMUL to be at least 1 and the overlap to sit in the highest-numbered
part of the destination group. The spec's own example is that at LMUL=8,
vzext.vf4 v0, v6is legal while a source ofv0,v2orv4is not.That is not what
rvv_cross_eew_overlap_illegal()implements — it requiresboth groups to share a base register, which matches the narrowing direction
instead and gets these two cases exactly backwards. The check is therefore
spelled out in the execution helper with a comment explaining why the shared
predicate is not used.
Source EMUL is compared against 1 via
rvv_lmul_ratio()rather than theregister span, because
rvv_eew_reg_span()clamps a fractional EMUL to aspan of one register and the information is lost.
Encodings were checked against riscv-opcodes (funct6=0x12, funct3=0x2,
vs1 = 2..7); reserved
vs1values are rejected. Smoke coverage added totests/rvv-smoke.Sfor vf2/vf4 zero- and sign-extension plus a masked case.make checkpasses withEXT_Venabled, and the build was checked in thedefault,
EXT_V, JIT+EXT_VandEXT_V-without-EXT_Fconfigurations.clang-format 20.1.7 clean.
Summary by cubic
Implements the integer-extension instructions
vzext.vf{2,4,8}andvsext.vf{2,4,8}(V 1.0 §11.3), which were previously rejected as unimplemented. They are now decoded and executed, including masked forms and tail handling; the widening register-overlap rule follows §5.2 rather than the existingrvv_cross_eew_overlap_illegal()predicate.Written for commit 15453a8. Summary will update on new commits.