search

openhwgroup / cv32e40p

CV32E40P is an in-order 4-stage RISC-V RV32IMFCXpulp CPU based on RI5CY from PULP-Platform
https://docs.openhwgroup.org/projects/cv32e40p-user-manual/en/latest
Other
902 stars 399 forks source link

Clarification on cv.clipr signedness #967

Closed realqhc closed 3 months ago

realqhc commented 3 months ago

That is a fair point. I raised this issue with the hw group and we should have soon an update that clarifies this. I think that having bit 31 zero (so rs2 being positive) is a requirement for correctness of the operation. In that case we'd need a check on the operand value. I agree with separating this patch into two for now: intrinsics support and codegen.

_Originally posted by @PaoloS02 in https://github.com/llvm/llvm-project/pull/78138#discussion_r1524938040_

Does cv.clipr require bit 31 being zero to function correctly? or it automatically treats the bit 31 as zero even if it is 1 (e.g. it performs abs before actual operation)?

pascalgouedo commented 3 months ago

I am working on it since last week. Same pb than https://github.com/openhwgroup/cv32e40p/commit/1a58c7b7666ac693d936c45b4ed9dc0d762e1bf6#commitcomment-138230716

I have an RTL update (masking MSB directly in HW) for that to definitively resolve the pb for everyone. Need to fully verify it before changing User Manual clipr/clipur description and pushing it.

pascalgouedo commented 3 months ago

Corrected with PR #971