sail-riscv icon indicating copy to clipboard operation
sail-riscv copied to clipboard

make sure vstart is zero when instruction early return

Open KotorinMinami opened this issue 3 months ago • 3 comments

for #1074

KotorinMinami avatar Aug 31 '25 17:08 KotorinMinami

Test Results

2 101 tests  ±0   2 101 ✅ ±0   20m 28s ⏱️ -2s     1 suites ±0       0 💤 ±0      1 files   ±0       0 ❌ ±0 

Results for commit d42b62dd. ± Comparison against base commit 0e96ba6e.

:recycle: This comment has been updated with latest results.

github-actions[bot] avatar Aug 31 '25 17:08 github-actions[bot]

Hmm thinking about this more I'm not sure about it. Maybe what would actually be least confusing is if do something like this:

val process_rfvv_single : (rfvvfunct6, bits(1), vregidx, vregidx, vregidx, nat1, sew_bitsize, range(-3, 3)) -> ExecutionResult
function process_rfvv_single(funct6, vm, vs2, vs1, vd, num_elem_vs, SEW, LMUL_pow) = {
  let rm_3b = fcsr[FRM];
  let num_elem_vd = get_num_elem(0, SEW); // vd regardless of LMUL setting

  if illegal_fp_reduction(SEW, rm_3b) then return Illegal_Instruction();
  assert(SEW != 8);

  if unsigned(vl) != 0 then {
    let 'n = num_elem_vs;
    let 'd = num_elem_vd;
    let 'm = SEW;

    let vm_val  : bits('n)     = read_vmask(num_elem_vs, vm, zvreg);
    let vd_val  : vector('d, bits('m)) = read_vreg(num_elem_vd, SEW, 0, vd);
    let vs2_val : vector('n, bits('m)) = read_vreg(num_elem_vs, SEW, LMUL_pow, vs2);
    let mask    : bits('n)     = match init_masked_source(num_elem_vs, LMUL_pow, vm_val) {
      Ok(v)   => v,
      Err(()) => return Illegal_Instruction()
    };

    var sum : bits('m) = read_single_element(SEW, 0, vs1); // vs1 regardless of LMUL setting
    foreach (i from 0 to (num_elem_vs - 1)) {
      if mask[i] == bitone then {
        sum = match funct6 {
          // currently ordered/unordered sum reductions do the same operations
          FVV_VFREDOSUM   => fp_add(rm_3b, sum, vs2_val[i]),
          FVV_VFREDUSUM   => fp_add(rm_3b, sum, vs2_val[i]),
          FVV_VFREDMAX    => fp_max(sum, vs2_val[i]),
          FVV_VFREDMIN    => fp_min(sum, vs2_val[i]),
          _               => internal_error(__FILE__, __LINE__, "Widening op unexpected")
        }
      }
    };

    write_single_element(SEW, 0, vd, sum);
    // other elements in vd are treated as tail elements, currently remain unchanged
    // TODO: configuration support for agnostic behavior
  };
  
  set_vstart(zeros());
  RETIRE_SUCCESS
}

Also Err(()) => return Illegal_Instruction() is only coincidentally correct because vstart must be zero. If we supported non-zero vstart then vstart=10, vl=0 presumably should raise an exception.

Maybe we should discuss this in the meeting.

Timmmm avatar Oct 09 '25 13:10 Timmmm

I prefer what you proposed, since I think it would be actually less confusing to have vstart zeroed at the end. When we support non-zero vstart, the execution clause will have to zero it at the end anyway.

Also Err(()) => return Illegal_Instruction() is only coincidentally correct because vstart must be zero. If we supported non-zero vstart then vstart=10, vl=0 presumably should raise an exception.

In this case (vstart=10,vl=0), won't the illegal_fp_reduction() check raise the exception?

pmundkur avatar Oct 29 '25 20:10 pmundkur