Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Clarification on cv.clipr signedness #967

Closed
realqhc opened this issue Mar 22, 2024 · 2 comments
Closed

Clarification on cv.clipr signedness #967

realqhc opened this issue Mar 22, 2024 · 2 comments
Assignees
Labels
Type:Question For general questions

Comments

@realqhc
Copy link

realqhc commented Mar 22, 2024

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 llvm/llvm-project#78138 (comment)

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
Copy link

pascalgouedo commented Mar 22, 2024

I am working on it since last week.
Same pb than 1a58c7b#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 pushed a commit to pascalgouedo/cv32e40p that referenced this issue Mar 25, 2024
davideschiavone added a commit that referenced this issue Mar 25, 2024
@pascalgouedo
Copy link

Corrected with PR #971

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type:Question For general questions
Projects
None yet
Development

No branches or pull requests

4 participants