Make compressed instructions work on all tests #205
Conversation
This comment was marked as outdated.
This comment was marked as outdated.
|
This is inducing:
|
fix minor things discovered through a deep walk-through
|
All tests are now passing! Many thanks to all - especially @leekillough for their help getting this across the line |
rkabrick
left a comment
rkabrick
left a comment
There was a problem hiding this comment.
Nothing earth shattering in my comments. Take it or leave it.
Get this MERGED
include/RevInstHelpers.h
Outdated
| req, | ||
| flags); | ||
| R->SetX(Inst.rd, static_cast<T>(R->RV64[Inst.rd])); | ||
| std::cout << "RMT: Load Issued for address: " << std::hex << req.Addr << " Data: " << static_cast<T>(R->RV64[Inst.rd]) << std::dec << " Dest Reg: " << req.DestReg << std::endl; |
There was a problem hiding this comment.
Should these cout calls be output->verbose ?
There was a problem hiding this comment.
spurious dead code... removing
| RevMem *M, RevInst Inst) { | ||
| // c.fsdsp rs2, $imm = fsd rs2, x2, $imm | ||
| Inst.rs1 = 2; | ||
| //Inst.rs1 = 2; //Removed - set in decode |
There was a problem hiding this comment.
Should this be removed? Or will keeping it commented help users understand the changes
| splitLine = line.strip().split(":") | ||
| PC = splitLine[0] | ||
| if "<_start>:" in line: | ||
| if "<main>:" in line: |
There was a problem hiding this comment.
We talked in person about making this modular based on the startAddr passed in the config
There was a problem hiding this comment.
ah - yes, that is a good idea, I'll fix that in a separate commit
| return; | ||
| } | ||
| // If address is not 32bit aligned... then make it aligned | ||
| Addr &= 0xFFFFFFFFFFFFFFFC; |
There was a problem hiding this comment.
I think you can do this Addr &= ~0x3;
To provide better compatibility with the RV toolchain all RV64 tests now build with RV64GC. To get all tests to pass two changes were required:
The prefetcher was not always properly accounting for 16-bit instructions. This was a single condition added to the IsAvail() function. I also removed some unnecessary recursion in the Fill() function.
Compressed instructions had their source and destination registers modified at execution time. This causes issues with Dependency check and LS queue as the registers used to Set/Clear are off by 8, or sometimes not set at all. All these modifications that were previously done at execute time are now incorporated into the Decode() function. This means that the RevInst has the correct source and destination registers after decode