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

Make ISA definitions compile-time defined. #315

Merged
merged 102 commits into from
Nov 9, 2023

Conversation

raccog
Copy link
Contributor

@raccog raccog commented Oct 10, 2023

Changed ISA code structure to use template classes for instruction information. This information is now defined at compile-time, instead of dynamically at runtime. In this change, all information about an instruction (opcode parts, fields, bit ranges, etc.) is initialized using template parameters.

Issues Solved

This almost fully solves #306 by using template parameters to define instruction set information at compile-time instead of runtime. The only remaining thing to do here is group up instruction sets at compile-time instead of dynamically. As of this change, instruction sets are created at runtime in the enableInstructions() function depending on which extensions are enabled. To solve this, each ISA extension could have a struct that uses template parameters to define the extension's enabled instructions at compile-time. These structs can then be combined at run-time depending on which extensions are enabled. I have not tested this yet, but I think it would work and be an improvement over defining the enabled instructions in the enableInstructions() function.

This almost solves #303. ISA-specific information such as instruction encodings, register information, and other details have all been moved to the ISA library. The only remaining ISA-specific information are the RV32 and RV64 assembler classes and RISC-V relocation functions. The assembler classes are mostly boilerplate code that enables certain instructions based on which extensions are enabled. These assemblers should be replaced by a single assembler class that takes an ISA ID as input. The extensions should be detected (and instructions enabled) only in the ISA library. The relocation functions are short and should be moved to the ISA library as well.

Similarly, this almost solves #307. The main problem here is decoupling the ISA/assembler libraries from VSRTL and QT. VSRTL has only a few coupled functions, but there are many dependencies on QT classes. These problems need to be solved before the ISA/assembler libraries can be extracted.

#311 has been solved. It is true that register widths do not need to be scattered throughout many template parameters and that removing them cleans up the code base without affecting performance. This change moves register widths into 2 locations; a function called bits() in all ISAInfo sub-classes (of which there is one per ISA), and the N template parameter in the BitRange class. Although N keeps the register width as a template parameter, it is not as scattered throughout the code base because each ISA uses their own default parameter for N equal to the ISA's register width. For example, in the RISC-V C extension file:

template <unsigned start, unsigned stop>
using BitRange = Ripes::BitRange<start, stop, 16>;

Future Changes

There are also a few other improvements that can be made after this change.

One is in the RegInfoBase class that (as of this change) defines information about all of the registers for an ISA. Although it didn't make it in time for this commit, I've been working on changing this class so that it defines a single register file of an ISA. There will then be a RegInfoSet class that holds all of the currently enabled register files for an ISA. This will allow indexing into registers from either a specific register file or all enabled register files as a whole.

Another improvement is adding more compile-time instruction verifications. When instructions are combined into a struct at compile-time, as described above, it can be verified at compile-time that there are no duplicate instructions or instruction mnemonics. Other similar verifications could be added to further ensure that ISA implementations are correct.

Finally, I think that the implementation for pseudo-instructions could be improved by defining the instructions that they expand into at compile-time. Currently, the PseudoInstruction method expander() defines what the pseudo-instruction expands to, at runtime. Although I have not tested it yet, I think that this could be improved by changing the expander() function into a struct containing the expanded instructions (defined at compile-time).

@raccog
Copy link
Contributor Author

raccog commented Oct 19, 2023

@mortbopet Quick update:

I finally got things to work properly with compile-time instruction info! So far, I have just the ADDI instruction implemented, but everything compiles and code assembles/disassembles properly. I'm still working on re-implementing the rest of the instructions (and cleaning up bundles of code), but now that the structural changes are working correctly, the rest shouldn't be nearly as difficult.

This also means I'm starting to think about how I will implement the sliderules. I think that most of it is possible at compile-time now. :)

One question I have is about the use of QString for register and instruction names. Do you think that std::string_view would be better, since it can be constructed at compile-time? There were quite a few cases where the use of QString prevented functions from being constexpr.

I have more questions, but I'll wait until I've cleaned up the code so that I can link to examples.

@mortbopet
Copy link
Owner

One question I have is about the use of QString for register and instruction names. Do you think that std::string_view would be better, since it can be constructed at compile-time? There were quite a few cases where the use of QString prevented functions from being constexpr.

Yes - this codebase generally uses Qt wherever possible, but I've myself started to go away from being in favor of that. I.e. using what is essentially Qt's replacement of the STL. Anyhow, i expect this assembler/ISA library, once extracted as a separate repo, to be completely rid of any Qt dependencies.

Comment on lines 398 to 414
template <typename FirstPart, typename... OtherParts>
struct Verify {};
template <typename FirstPart, typename SecondPart, typename... OtherParts>
struct Verify<FirstPart, SecondPart, OtherParts...> {
/// Returns true if FirstPart and SecondPart are not overlapping
constexpr static bool IsNotOverlapping() {
return (FirstPart::Offset() >
(SecondPart::Offset() + SecondPart::BitRange::Width()) ||
SecondPart::Offset() >
(FirstPart::Offset() + FirstPart::BitRange::Width()));
}
enum {
nonOverlapping = (IsNotOverlapping() &&
Verify<FirstPart, OtherParts...>::nonOverlapping &&
Verify<SecondPart, OtherParts...>::nonOverlapping)
};
};
Copy link
Owner

Choose a reason for hiding this comment

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

Very clever!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, I'm really proud of those verification structures! I have a few more ideas on other parts of the class structure that can be similarly verified (ex: verify that every bit is accounted for), but do you think I should focus on re-implementing the ISA first? The benefit of adding more verifications first is that when I re-implement the instructions I will be sure that they are verified to be valid.

Copy link
Owner

Choose a reason for hiding this comment

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

but do you think I should focus on re-implementing the ISA first?

Your call - if you can get it done in a reasonable amount of time and you estimate that time to be less than the time you'll need for debugging this - sure. But you can also always create issues to capture these things and get back to it later.

@mortbopet
Copy link
Owner

mortbopet commented Oct 23, 2023

Another note: For the eventual extraction of this library, we should probably also think about how to verify these compile-time checks. Here's some reading

@raccog
Copy link
Contributor Author

raccog commented Oct 23, 2023

Another note: For the eventual extraction of this library, we should probably also think about how to verify these compile-time checks. Here's some reading

* https://www.reddit.com/r/cpp/comments/72ce4y/testing_compiletime_checks/.

* https://www.codeproject.com/Tips/1247697/Unit-Tests-for-Code-that-is-Supposed-to-Not-Compil

Very interesting! I'll look into this. :)

@raccog raccog marked this pull request as ready for review November 1, 2023 15:14
src/assembler/matcher.h Show resolved Hide resolved
src/assembler/rv64i_assembler.h Outdated Show resolved Hide resolved
src/isa/isainfo.h Outdated Show resolved Hide resolved
src/isa/isainfo.h Outdated Show resolved Hide resolved
src/isa/isainfo.h Outdated Show resolved Hide resolved
src/isa/instruction.h Outdated Show resolved Hide resolved
src/isa/instruction.h Outdated Show resolved Hide resolved
src/isa/instruction.h Outdated Show resolved Hide resolved
src/isa/instruction.h Outdated Show resolved Hide resolved
src/isa/instruction.h Outdated Show resolved Hide resolved
@mortbopet mortbopet merged commit 2a042c9 into mortbopet:master Nov 9, 2023
6 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.

2 participants