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

Branch prediction processor #289

Draft
wants to merge 16 commits into
base: master
Choose a base branch
from

Conversation

Anglo-software
Copy link

Implemented a 5-stage branch prediction processor, along with a tab to control its properties like predictor type and whatnot, along with statistics and visualizations of internal state. Once the processor with branch prediction is selected through the processor selection menu, the branch prediction tab is enabled. The processor will hold its internal state through different program changes and reloads, allowing multiple iterations of running a single program or seeing how different programs interact through the predictor. The user can reset the state at any time if they wish. It supports reversing execution, RV32IMC, and RV64IMC.

Small note: the test programs (such as tst_riscv) supplied with Ripes do not work with the processor, but I did run each test assembly file manually and they all were successful. The errors (usually segmentation faults) originate from different places at different times, sometimes consistent between runs and sometimes producing different errors. Possible race condition?

However, in the Ripes software itself, it is very stable and error free, at least as far as I've been able to test.

If there's any changes that need to be made, I'd be more than glad to do them, and any guidance on the aforementioned issue would be greatly appreciated.

@mortbopet
Copy link
Owner

mortbopet commented Jul 10, 2023

Awesome for taking a stab at this, can't wait to try it out! Its a big PR so let's make sure that we take our time and get it right - given the size I'll be reviewing it in stages.

My first major comment before going into the nitty gritty details is that I think it would be elegant if the branch predictor work is detached from the notion that it's a RISCV processor that it's executing under. This would mean that checks for risc-v specific branch-like instructions aren't done inside the branch predictor tab/whereever this is done. Instead, processors should announce whether they support a branch predictor interface, and said interface should communicate ISA-agnostic information. I imagine it should be fairly simple to come up with an ISA-agnostic interface for this (i.e. "Is the current instruction a branch-like instruction? what is the branch target? branch taken, not taken, ...).
I think this will both improve the code, as well as make it easier for people in the future to hook into your (now more generic) branch predictor (e.g. there are people currently who's working on a MIPS processor for Ripes, wherein it would be cool if we could just hook it directly into this branch predictor!).

In line with the above, i'd also like to see the branch predictors themselves conform to some form of interface which each predictor type inherits. This will clean up your code a lot (i.e. a separate class for TwoState, AlwaysTaken, ...) + make it easier for people to add their own predictors down the road.

Third, do you think it is possible that we can do some form of inheritance between e.g. rv5s and rv5s_br? I've had to deal with so many errors regarding tiny errors when connecting up all of the components at the top-level, so it would make good sense to me if we factor out the commonalities into a shared base model.

@Anglo-software
Copy link
Author

I'll work on fixing the failed checks then try and work on what you noted, however it might prove difficult to "detach" the branch predictor from the processor implementation considering that the branch predictor itself is part of the processor hardware. But, I will definitely attempt regardless because I would find it valuable to make it more generic and extendable. I'll take the time to make sure it's done right.

@Anglo-software Anglo-software marked this pull request as draft July 10, 2023 16:51
@mortbopet
Copy link
Owner

however it might prove difficult to "detach" the branch predictor from the processor implementation considering that the branch predictor itself is part of the processor hardware

Oh most definitely, and there's probably no way around having to have an e.g. "RISC-V branch predictor"-gasketmodule. But that should just be a gasket which performs any intermediate conversions required to use the generic branch predictor model.

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