Skip to content

refactor: make extensions accept processor as an argument#1949

Merged
aswaterman merged 1 commit intoriscv-software-src:masterfrom
arromanoff:master
Apr 8, 2025
Merged

refactor: make extensions accept processor as an argument#1949
aswaterman merged 1 commit intoriscv-software-src:masterfrom
arromanoff:master

Conversation

@arromanoff
Copy link
Collaborator

As discussed in #1863 we consider a good idea to refactor custom extensions and make their handling of processor more explicit. In this commit I make all extensions's methods accept processor as an argument.

This change also makes it possible to configure instructions and disasms depending on isa info (e.g. now it is possible to add certain instructions only if we are running RV32). Previously it was not possible due to extension_t::p being set only after requesting the instructions from extension.

@arromanoff
Copy link
Collaborator Author

@jerryz123 Can you take a look please

xdummycsr_t() {}

std::vector<insn_desc_t> get_instructions() override {
std::vector<insn_desc_t> get_instructions(processor_t &) override {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should/can these be const?

virtual ~extension_t();
virtual std::vector<insn_desc_t> get_instructions(processor_t &proc) = 0;
virtual std::vector<disasm_insn_t *>
get_disasms(processor_t *proc = nullptr) = 0;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you split the formatting into a different commit? Or omit it altogether from this PR (This didn't seem to do the right thing here I think)

@arromanoff arromanoff force-pushed the master branch 4 times, most recently from f63dcdd to 5c8d087 Compare April 8, 2025 15:44
@aswaterman aswaterman merged commit e24ae2a into riscv-software-src:master Apr 8, 2025
3 of 4 checks passed
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.

3 participants