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

CHERI encodings and CSRs #214

Closed
wants to merge 8 commits into from
Closed

Conversation

tariqkurd-repo
Copy link

  • first attempt at CSRs and encodings for CHERI
  • note that lr/b etc will be a separate extension eventually

@tariqkurd-repo
Copy link
Author

This is my first attempt at reserving some encoding and CSR space for CHERI.
There are some limitations:

LR.B, LR.H, SC.B, SC.H need to be in a separate extension, but for use by CHERI software
All immediate values are 12-bit which is unlikely to be acceptable due to the encoding space requirement.
However, I'm interested in getting some feedback on the encodings in general, so I thought it was worth getting this PR ready sooner rather than later

Copy link
Member

@aswaterman aswaterman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using this repo to reserve opcode space far in advance of an extension's proposal isn't standard practice, and this is an even more unusual corner case since it's really a new base ISA. Still, I think it's helpful to leave this PR open so people can look at the concrete proposal.

@tariqkurd-repo
Copy link
Author

Our intention is to keep CHERI as compatible as possible with other extensions, and so tentatively reserving encoding space and CSR numbers help with that goal. When the TG gets going next year we'll make a more formal proposal.

@tariqkurd-repo
Copy link
Author

tariqkurd-repo commented Jan 4, 2024

@aswaterman this branch builds OK - but the cincoffsetimm encoding conflicts with slli. The conflict doesn't seem to be detected as the immediate values are different widths. Is this a known limitation?

@veselypeta
Copy link

LLVM also includes a mechanism for detecting encoding conflicts, but it too failed to identify this. Perhaps for the same reason stated by above.

@codecov-commenter
Copy link

codecov-commenter commented Jan 4, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (61d2ef4) 92.83% compared to head (6fefa7d) 92.83%.
Report is 9 commits behind head on master.

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #214   +/-   ##
=======================================
  Coverage   92.83%   92.83%           
=======================================
  Files           3        3           
  Lines         642      642           
=======================================
  Hits          596      596           
  Misses         46       46           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

arichardson pushed a commit to riscv/riscv-cheri that referenced this pull request Feb 27, 2024
I've changed this to the intended encoding, which is already in
riscv-opcodes (riscv/riscv-opcodes#214).
The previous one was a typo and conflicted with c.subw.
@tariqkurd-repo tariqkurd-repo marked this pull request as draft March 8, 2024 14:06
@tariqkurd-repo
Copy link
Author

See: #237

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants