chipsalliance / chipsalliance/riscv-vector-tests

Generator silently drops declared `sew*` blocks — 19 instructions across two files

Open Beginner friendly
#94 2 comments 0 reactions 0 assignees View on GitHub
Dominant language
Go
Stars
118
Forks
40
PR merge metrics
No merged PRs in 30d

Description

## Summary

Two generator files choose a SEW list narrower than the instructions permit, so nineteen
instructions never run at a SEW their own config declares operand vectors for. Nothing reports it:
the configs look covered, the suite passes, and the coverage is absent.

Verified on `main` @ `f76bff1`. **Both are one-line changes**, each aligning a file with a sibling
that already handles the same constraint correctly; diffs below.

## What is missing

Measured on `main` @ `f76bff1`, `VLEN=256 XLEN=64`. **Legal** is what the spec permits at ELEN=64;
**generated** is what the generator emits today:

| instructions | legal SEWs | generated SEWs | missing |
|---|---|---|---|
| `vsll.vi` `vsra.vi` `vsrl.vi` `vssra.vi` `vssrl.vi` `vror.vi` `vrgather.vi` `vslideup.vi` `vslidedown.vi` | 8 16 32 64 | 8 16 32 | **64** |
| `vnsra.wi` `vnsrl.wi` `vnclip.wi` `vnclipu.wi` | 8 16 32 | 8 16 | **32** |
| `vbrev.v` `vbrev8.v` `vclz.v` `vcpop.v` `vctz.v` `vrev8.v` | 8 16 32 64 | 16 32 64 | **8** |

Each of these declares operand vectors for the missing SEW in its config and never runs at it.

The **legal** column is the spec's element-width limit applied to each group. The first and third are
plain `SEW <= ELEN`. The `.wi` forms are narrowing — `vs2` is 2xSEW — so they are bounded by
`2*SEW <= ELEN`, which permits 32 at ELEN=64 and correctly excludes 64.

## Reproduce it

Stage-1 generation only, no simulator needed:

```console
# main @ f76bff1
make generate-stage1 VLEN=256 XLEN=64
grep -h 'SEW: e' out/v256x64machine/tests/stage1/vsll_vi-*.S | sort -u # no e64
grep -h 'SEW: e' out/v256x64machine/tests/stage1/vnsra_wi-*.S | sort -u # no e32
grep -h 'SEW: e' out/v256x64machine/tests/stage1/vclz_v-*.S | sort -u # no e8
```

`configs/v/vsll.vi.toml` declares a `sew64` block that is never used.

## Where it comes from

Two files...

### `generator/insn_vdvs2uimmvm.go` — the 13 above

```go
// generator/insn_vdvs2uimmvm.go:12 — main @ f76bff1 (2026-05-24), current code
sews := iff(vs2Widening, allSEWs[:len(allSEWs)-2], allSEWs[:len(allSEWs)-1])
```

Both branches truncate by one too many:

* **else branch** → `{8,16,32}` for instructions where neither operand is widened, **9 lose
SEW=64**: `vsll.vi` `vsra.vi` `vsrl.vi` `vssra.vi` `vssrl.vi` `vror.vi` `vrgather.vi`
`vslideup.vi` `vslidedown.vi`
* **then branch** → `{8,16}` for the `.wi` forms, **4 lose SEW=32**: `vnsra.wi` `vnsrl.wi` `vnclip.wi` `vnclipu.wi`

`vwsll.vi` is correct only incidentally: it widens `vd`, so it takes the else branch and gets the
`{8,16,32}` the `.wi` forms should also have had.

The generator already emits those SEWs for the same instructions in their other operand forms.

| instruction group | `.vv` / `.wv` | `.vx` / `.wx` | immediate form |
|---|---|---|---|
| the 7 shifts, rotates and `vrgather` | 8 16 32 **64** | 8 16 32 **64** | 8 16 32 |
| `vslideup` `vslidedown` | *(no `.vv` form)* | 8 16 32 **64** | 8 16 32 |
| the 4 narrowing `.w*` | 8 16 **32** | 8 16 **32** | 8 16 |

Thirteen for thirteen. Since where the shift or index amount comes from has no bearing on the
element-width limit, there is no reason for the immediate form to differ — and restoring it brings
these files into line with what the generator already does everywhere else.

### `generator/insn_vdvs2vm.go` — the 6 single-operand instructions

```go
// generator/insn_vdvs2vm.go:10,19,46 — main @ f76bff1 (2026-05-24), current code
float := strings.HasPrefix(i.Name, "vf") // :10
...
sews := iff(vdWidening || vdNarrowing, // :19 float not consulted
i.floatSEWs()[:len(i.floatSEWs())-1], i.floatSEWs())
...
builder.WriteString(i.gWriteTestData(float, ...)) // :46 float consulted here
```

So the six integer members are given the float SEW list and never run at SEW=8.

The sibling `insn_vdvs2vs1vm.go` consults `float` in both places — `:21` for the SEW list and
`:96`/`:99` for the data — which is the behaviour proposed below. These six are single-operand, so
unlike the thirteen above they have no sibling *instruction* forms to compare against.

## Proposed fix

**File 1** — both widening cases share a cap, so they collapse:

```diff
generator/insn_vdvs2uimmvm.go:12
- sews := iff(vs2Widening, allSEWs[:len(allSEWs)-2], allSEWs[:len(allSEWs)-1])
+ sews := iff(vs2Widening || vdWidening, allSEWs[:len(allSEWs)-1], allSEWs)
```

This matches `insn_vdvs2rs1vm.go:17`, which handles the identical constraint this way today.

Alternatively, drop the pre-truncation entirely and rely on the existing
`vdEEW > XLEN || vs2EEW > XLEN` guard, which every one of these files already has — that is what
`insn_vdvs2vs1vm.go:21` does, and it is also correct:

```diff
generator/insn_vdvs2uimmvm.go:12
- sews := iff(vs2Widening, allSEWs[:len(allSEWs)-2], allSEWs[:len(allSEWs)-1])
+ sews := allSEWs
```
Measured: it produces the same SEW sets, at the
cost of two extra output files, because a guard-rejected combination still occupies a chunk slot.
Happy to submit whichever you prefer.

**File 2** — consult the `float` flag that is already computed:

```diff
generator/insn_vdvs2vm.go:19
- sews := iff(vdWidening || vdNarrowing, i.floatSEWs()[:len(i.floatSEWs())-1], i.floatSEWs())
+ sews := iff(vdWidening || vdNarrowing,
+ i.floatSEWs()[:len(i.floatSEWs())-1],
+ iff(float, i.floatSEWs(), allSEWs))
```

## Validation

At `VLEN=256 XLEN=64 SPLIT=10000` on `f76bff1`: 3,043 → 3,064 generated files, `+21` chunks across
the 13 instructions in file 1, with no other chunk count changing. The 6 instructions in file 2
gain SEW=8 within their existing chunks.

All newly emitted tests build and pass their Spike self-check, and pass on Saturn RTL
(`REFV256D128Rocket`, VLEN=256). Float members of `vd,vs2,vm` are unchanged (`vfsqrt.v`,
`vfclass.v` remain `{16,32,64}`).

One newly emitted case does fail on Saturn — `vror.vi` at SEW=64 — which is a decode
bug in the implementation, not in these tests, and is being reported separately to its maintainers.

## Relation to #72

Same class — a valid coverage dimension the generator does not emit — but a different code path and
a disjoint instruction set, so the two do not conflict.

Contributor guide

No contributing guide indexed for this repository

Research direction

Read `generator/insn_vdvs2uimmvm.go` and `generator/insn_vdvs2vm.go`, comparing the relevant SEW selection with the sibling implementations cited in the issue. Run `make generate-stage1 VLEN=256 XLEN=64` and inspect the listed generated files for the missing SEWs. Done when the expected SEWs appear for all affected instructions and the newly emitted tests build and pass their self-check.

Written by the indexing model from the issue text.

Assessment

Tech stack
go
Domain
compilers, testing
Issue type
Bug
Difficulty
2/5
Estimated time
1-3 hours
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
72/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.