Skip to content

Conversation

jerryz123
Copy link
Contributor

This should enable downstream users to implement custom decoders without very heavy-handed changes to the core implementation (#3534)

Related issue:

Type of change: bug report | feature request | other enhancement

Impact: no functional change | API addition (no impact on existing code) | API modification

Development Phase: proposal | implementation

Release Notes

Copy link
Member

@sequencer sequencer left a comment

Choose a reason for hiding this comment

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

I'm hesitate to expose this to other users.
At least in T1's flow, we will continue use the fork version of rvdeocderdb and the chisel DecoderTable API.
Since it reduces the control signal in datapath significantly for small designs.(~30 -> ~15) and is more type safe.
But Since T1 use the forked version of RocketCore, I don't against provide this feature in the upstream RocketChip;p

@jerryz123
Copy link
Contributor Author

jerryz123 commented Jan 29, 2024

I'm hesitate to expose this to other users. At least in T1's flow, we will continue use the fork version of rvdeocderdb and the chisel DecoderTable API. Since it reduces the control signal in datapath significantly for small designs.(~30 -> ~15) and is more type safe. But Since T1 use the forked version of RocketCore, I don't against provide this feature in the upstream RocketChip;p

I see, its cool that the rvdecoderdb has that impact. What is the overall area reduction?

The reason I prefer the simpler decode tables as the default is to

  1. Reduce number of library dependencies. Since rocket-chip is the "base" for most users, it should only depend on chisel
  2. Present a simpler interface to users adding new instructions
  3. Avoid unintentionally changing behavior

While your fork is fine, I think it would be nice to have a shared rocket-vector interface in the upstream for both our projects, even if the vector backends target different design points. The ifv branch is almost stable.

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.

2 participants