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

Replace bounds decoding ascii art with bytefield diagram #526

Merged
merged 2 commits into from
Feb 7, 2025

Conversation

andresag01
Copy link
Collaborator

Replace the ascii art with the bounds decoding diagram to make it clear how they are computed and what the resulting width of top and base.

This PR replaces this:

top:    t = { {1'b0, a[MXLEN - 1:E + MW]} + ct, T[MW - 1:0], {E{1'b0}} }
base:   b = {        a[MXLEN - 1:E + MW]  + cb, B[MW - 1:0], {E{1'b0}} }

With the bytefield figures in the attached image.
Screenshot 2025-02-06 at 13 34 16

@jrtc27
Copy link
Collaborator

jrtc27 commented Feb 6, 2025

Where's the extra 1'b0 in the diagram?

@tariqkurd-repo
Copy link
Collaborator

Where's the extra 1'b0 in the diagram?

that's what I was wondering - becaue the text in the box isn't wide enough without it

@andresag01
Copy link
Collaborator Author

andresag01 commented Feb 6, 2025

I put the 1'b0 back. Now it looks like this
Screenshot 2025-02-06 at 19 21 42

@arichardson arichardson merged commit ae5ba0b into riscv:main Feb 7, 2025
3 checks passed
tariqkurd-repo pushed a commit to tariqkurd-repo/riscv-cheri that referenced this pull request Feb 10, 2025
Replace the ascii art with the bounds decoding diagram to make it clear
how they are computed and what the resulting width of top and base.

This PR replaces this:

```
top:    t = { {1'b0, a[MXLEN - 1:E + MW]} + ct, T[MW - 1:0], {E{1'b0}} }
base:   b = {        a[MXLEN - 1:E + MW]  + cb, B[MW - 1:0], {E{1'b0}} }
```

With the bytefield figures  in the attached image.
<img width="694" alt="Screenshot 2025-02-06 at 19 21 42" src="https://github.com/user-attachments/assets/521e4239-f23d-4bd2-b9a0-e242cf35cc41" />
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