Refactor vstart assignments to use set_vstart function

[rez5427]

#623

[github-actions]

Test Results

396 tests  ±0   396 ✅ ±0   0s ⏱️ ±0s
  4 suites ±0     0 💤 ±0 
  1 files   ±0     0 ❌ ±0 

Results for commit 141b009. ± Comparison against base commit 70edcb0.

♻️ This comment has been updated with latest results.

[Timmmm]

Nice work. I think there are a couple of things to change though:

1. set_vstart() needs to call dirty_v_context().
2. function clause write_CSR(0x008, value) should use set_vstart().
3. I'm not sure about this but the CSR writing function truncates the written value to get_vlen_pow() bits. That isn't done by the places that call set_vstart(to_bits(16...). Should it? Or do we know that that truncation would never make a difference?

[rez5427]

Is this okay? @Timmmm, it seems like the length of vstart depends on vlen. I checked Spike, and it uses typedef uint64_t reg_t; to define vstart and writes vstart using a mask of vlen - 1. Maybe setting a dynamic length for vstart based on vlen would be better?

[Timmmm]

I don't think this is quite right either (but it is closer). I took the drastic action of actually reading the spec. These are the relevant bits:

The vstart CSR is defined to have only enough writable bits to hold the largest element index (one less than the maximum VLMAX).

The maximum vector length is obtained with the largest LMUL setting (8) and the smallest SEW setting (8), so VLMAX_max = 8*VLEN/8 = VLEN. For example, for VLEN=256, vstart would have 8 bits to represent indices from 0 through 255.

The use of vstart values greater than the largest element index for the current vtype setting is reserved.

It is recommended that implementations trap if vstart is out of bounds. It is not required to trap, as a possible future use of upper vstart bits is to store imprecise trap information.

Implementations are permitted to raise illegal instruction exceptions when attempting to execute a vector instruction with a value of vstart that the implementation can never produce when executing that same instruction with the same vtype setting.

Also:

The upper limit on VLEN allows software to know that indices will fit into 16 bits (largest VLMAX of 65,536 occurs for LMUL=8 and SEW=8 with VLEN=65,536).

So I think that's why the old code does tobits(16, ...) and vstart is a 16-bit register.

It does not look like the existing implementation traps when vstart is out of bounds; presumably because the current Sail CSR functions do not support trapping based on the value. Probably best to leave that for now. Maybe add a note.

So I think you want to change the xlen back to bits(16), and then take the lowest 16 bits of the write value for the CSR write clause.

[rez5427]

Is this okay? @Timmmm

[Timmmm]

Apart from that minor comment suggestion this LGTM. I'm still not 100% convinced by the original tobits(16, .. - it could probably do with fancier types so we can guarantee it doesn't truncate. But anyway this PR doesn't change that so it's fine.