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

The new decoder #3498

Draft
wants to merge 11 commits into
base: dev
Choose a base branch
from
Draft

The new decoder #3498

wants to merge 11 commits into from

Conversation

sequencer
Copy link
Member

@sequencer sequencer commented Sep 26, 2023

Original IDecode.scala is really hard to maintain which use the view from each instruction rather than each control field.
This PR remove original decoder with new [DecodeTable] in Chisel.
It will be a non-small refactor, but it can help us adding new instructions and have a better understand to the uarch.
After this PR is landed, all original IDecode will be moved into an "legacy" folder, downstream user can still use them if they wish, but after Boom and Xiangshan migrated, they will be removed. cc @jerryz123 , @poemonsense
sequncer/rvdecoderdb will be upstreamed to chipsalliance as a part of RC after all test clean.

BTW, I found the Zb/Zk UOP was really a bad design...

Type of change: other enhancement

Impact: API addition (no impact on existing code)

Development Phase: proposal | implementation

Release Notes
Switch to DecodeTable API.

@poemonsense
Copy link
Contributor

We are still on the way to upgrading to Chisel 5.

Thank you for cc me. We will bump rocket first to the version before new decoder table and then bump to the latest with changes on the decoder table.

Btw, we still have some local commits in Openxiangshan/rocket-chip. I'll see whether they should/could be upstreamed to chipsalliance. I agree it would be better to follow upstream rocket.

@sequencer sequencer force-pushed the new_decoder branch 2 times, most recently from e5d136b to 71a0824 Compare October 1, 2023 23:06
@sequencer
Copy link
Member Author

Spot my bug in chipsalliance/chisel#2924
https://github.com/chipsalliance/chisel/blob/440f01addeadd265fca2518c0a4df00b698e4603/src/main/scala/chisel3/util/experimental/decode/DecoderBundle.scala#L61
asTypeOf should never be LHS, otherwise it will create a wire and broken in when connection....
I'll vendor a chisel source for debugging, but not going to be merged.

@sequencer sequencer force-pushed the new_decoder branch 8 times, most recently from ee9d52d to 1ff5e30 Compare October 14, 2023 03:28
@sequencer
Copy link
Member Author

sequencer commented Oct 14, 2023

Assign the Zb/Zk to @cyyself, I'll skip them in this PR by mask them off.

After Chisel merging chipsalliance/chisel#3563 I'll bump Chisel version to 3.6.1 and 5.0.x and get this PR merged, CI for this PR is based on my hotfix branch in Chisel.

@sequencer sequencer force-pushed the new_decoder branch 5 times, most recently from 4599f46 to 168524d Compare October 17, 2023 11:01
@cyyself cyyself mentioned this pull request Nov 18, 2023
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.

None yet

2 participants