Saturation remove set 0

[rez5427]

I ran the test for VNClip and noticed that the saturation function includes logic that sets vxsat to zero. This caused the test to fail. After reviewing the Spike implementation, I found that it does not set vxsat to zero. The spec only mentions setting it to one, without specifying any case where it should be reset to zero.

then saturates the result to fit into SEW bits. If the result causes saturation, the vxsat bit is set.

In vector instructions, if a situation causes saturation midway, vxsat is set to one but is later reset to zero. As a result, the information that vxsat was once set to one is discarded, as shown in the case in the picture.

Here is the spike.

[rez5427]

elem: 0x7fff triggers set vxsat to one btw.

[allenjbaum]

In IEEE FP there are exception bits that can be set by HW, but are only cleared by SW. One of them is an underflow exception bit (among others), indicating a denormalized result. It is possible for an FP op to yield a normalized result, AND to set the underflow exception bit. This can happen if an operation yields an denormalized result, but when rounded, becomes normalized again. It sounds like the P extension is doing something similar.

…

On Wed, Oct 9, 2024 at 10:49 AM guan jian ***@***.***> wrote: I ran the test for VNClip and noticed that the saturation function includes logic that sets vxsat to zero. This caused the test to fail. After reviewing the Spike implementation, I found that it does not set vxsat to zero. The spec <https://riscv-specs.timhutt.co.uk/spec/20240411/unpriv-isa-asciidoc.html#_vector_single_width_fractional_multiply_with_rounding_and_saturation> only mentions setting it to one, without specifying any case where it should be reset to zero. then saturates the result to fit into SEW bits. If the result causes saturation, the vxsat bit is set. In vector instructions, if a situation causes saturation midway, vxsat is set to one but is later reset to zero. As a result, the information that vxsat was once set to one is discarded, as shown in the case in the picture. image.png (view on web) <https://github.com/user-attachments/assets/add7c3f0-f73e-460a-a106-fb54e7bee581> Here is the spike. image.png (view on web) <https://github.com/user-attachments/assets/7eb80455-9cc5-49df-9ba0-61c5a89b4544> ------------------------------ You can view, comment on, or merge this pull request online at: #585 Commit Summary - 520f019 <520f019> Change sign to unsign in v extension - 5879615 <5879615> saturation remove set 0 File Changes (2 files <https://github.com/riscv/sail-riscv/pull/585/files>) - *M* model/riscv_insts_vext_mem.sail <https://github.com/riscv/sail-riscv/pull/585/files#diff-84b47c6339b1b12666f0a87710d2a9eecfbe0a063f2236a594a2192e9082c1f3> (8) - *M* model/riscv_insts_vext_utils.sail <https://github.com/riscv/sail-riscv/pull/585/files#diff-125cfce21b0630c7389c0d3807cca40495388de37e444e9bf35bd021bf43cb2d> (2) Patch Links: - https://github.com/riscv/sail-riscv/pull/585.patch - https://github.com/riscv/sail-riscv/pull/585.diff — Reply to this email directly, view it on GitHub <#585>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AHPXVJWZZPWG6GZENH77WT3Z2VUBBAVCNFSM6AAAAABPVB7W7WVHI2DSMVQWIX3LMV43ASLTON2WKOZSGU3TMNJWGE2DIMI> . You are receiving this because you are subscribed to this thread.Message ID: ***@***.***>

[github-actions]

Test Results

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

Results for commit 7ed3714. ± Comparison against base commit 87f8bb3.

[Timmmm]

I confirmed we have the same fix in our fork (sorry... I'll try to make a list of fixes at some point at least).