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

Cbuildcap and ctestsubset and the use of DDC #18

Closed
francislaus opened this issue Jan 23, 2024 · 8 comments · Fixed by #82
Closed

Cbuildcap and ctestsubset and the use of DDC #18

francislaus opened this issue Jan 23, 2024 · 8 comments · Fixed by #82
Labels
bug Something isn't working

Comments

@francislaus
Copy link
Contributor

Both cbuilcap and ctestsubset use DDC as a parameter if the cs1 == 0. However, both instructions only have Zcheri_purecap as their prerequisite. DDC is a register that is only defined in Zcheri_legacy, if I understand the spec correctly. Thus, there exists a case where the specified behaviour of the instruction is not possible.

These are the two straightforward solutions that came into my mind.

  1. Cbuildcap and ctestsubset use c0 if cs1 == 0
  2. DDC is defined in Zcheri_purecap

There probably are other solutions. Given the two I named, the first one seems way more clean. This would not optimally use the encoding space of the register index for cs1, but it would prevent DDC from being defined in purecap where it is not needed.

@andresag01 andresag01 added the bug Something isn't working label Jan 23, 2024
@andresag01
Copy link
Collaborator

@francislaus : Thanks for the feedback. You are absolutely right, the original specification for CBuildCap and CTestSubset in CHERI v9 does have a special case for DDC when cs1 == 0. This was not taken into account in the current definition of CBuildCap and CTestSubset and needs to be fixed.

Given the current split of extensions in the spec, option (1) seems is a better fit.

@tariqkurd-repo
Copy link
Collaborator

I also vote for option (1) - as for a purecap core we don't want to include DDC at all.
Can you file a PR @francislaus with the fix? thanks!

@francislaus
Copy link
Contributor Author

@tariqkurd-repo @andresag01 Sounds good. I will file a PR this afternoon.

@arichardson
Copy link
Collaborator

I think using cnull instead of DDC should be fine. The use of DDC is a carryover from the MIPS spec where we also had that behaviour for other instructions (loads/stores) to save encoding space. For consistency, c0 probably makes sense. Maybe those encodings should be marked as HINTs?

@jrtc27
Copy link
Collaborator

jrtc27 commented Jan 24, 2024

They're not hints though, they write a value to the destination register (with the exception of cd=c0, which is true regardless of what source operands you have).

@arichardson
Copy link
Collaborator

arichardson commented Jan 24, 2024

They're not hints though, they write a value to the destination register (with the exception of cd=c0, which is true regardless of what source operands you have).

That's true, I forgot they write a non-zero value. Use of CNULL doesn't really make much sense though so maybe we could just make those encodings RESERVED?

@francislaus
Copy link
Contributor Author

I could get behind the RESERVED encodings. This is done over an over in RISC-V where it does not make sense to have x0 as an operand.

@andresag01
Copy link
Collaborator

Apologies for closing this ticket. GitHub automatically closed it because it unhelpfully.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants