Alec Roelke has uploaded this change for review. (
https://gem5-review.googlesource.com/4041
Change subject: riscv: Fix bugs with RISC-V decoder and detailed CPUs
......................................................................
riscv: Fix bugs with RISC-V decoder and detailed CPUs
This patch fixes some bugs that were missed with the changes to the
decoder that enabled compatibility with compressed instructions. In
order to accommodate speculation with variable instruction widths, a few
assertions in decoder had to be changed to returning faults as the
specification describes should normally happen. The rest of these
assertions will be changed in a later patch.
Change-Id: I3f333008430d4a905cb59547a3513f5149b43b95
---
M src/arch/riscv/decoder.cc
M src/arch/riscv/decoder.hh
M src/arch/riscv/faults.cc
M src/arch/riscv/faults.hh
M src/arch/riscv/isa/decoder.isa
M src/arch/riscv/isa/formats/fp.isa
M src/arch/riscv/utility.hh
7 files changed, 82 insertions(+), 28 deletions(-)
diff --git a/src/arch/riscv/decoder.cc b/src/arch/riscv/decoder.cc
index 36504f4..5794f88 100644
--- a/src/arch/riscv/decoder.cc
+++ b/src/arch/riscv/decoder.cc
@@ -37,29 +37,46 @@
namespace RiscvISA
{
+static const MachInst LowerBitMask = (1 << sizeof(MachInst) * 4) - 1;
+static const MachInst UpperBitMask = LowerBitMask << sizeof(MachInst) * 4;
+
+void Decoder::reset()
+{
+// DPRINTF(Decode, "reset");
+ aligned = true;
+ mid = false;
+ more = true;
+ emi = NoopMachInst;
+ instDone = false;
+}
+
void
Decoder::moreBytes(const PCState &pc, Addr fetchPC, MachInst inst)
{
- DPRINTF(Decode, "Getting bytes 0x%08x from address %#x\n",
- inst, pc.pc());
+ DPRINTF(Decode, "Requesting bytes 0x%08x from address %#x\n", inst,
+ fetchPC);
bool aligned = pc.pc() % sizeof(MachInst) == 0;
- if (mid) {
- assert(!aligned);
- emi |= (inst & 0xFFFF) << 16;
+ if (aligned) {
+ emi = inst;
+ if (compressed(emi))
+ emi &= LowerBitMask;
+ more = !compressed(emi);
instDone = true;
} else {
- MachInst instChunk = aligned ? inst & 0xFFFF :
- (inst & 0xFFFF0000) >> 16;
- if (aligned) {
- emi = (inst & 0x3) < 0x3 ? instChunk : inst;
+ if (mid) {
+ assert((emi & UpperBitMask) == 0);
+ emi |= (inst & LowerBitMask) << sizeof(MachInst)*4;
+ mid = false;
+ more = false;
instDone = true;
} else {
- emi = instChunk;
- instDone = (instChunk & 0x3) < 0x3;
+ emi = (inst & UpperBitMask) >> sizeof(MachInst)*4;
+ mid = !compressed(emi);
+ more = true;
+ instDone = compressed(emi);
}
}
- mid = !instDone;
}
StaticInstPtr
@@ -83,12 +100,10 @@
return nullptr;
instDone = false;
- if ((emi & 0x3) < 0x3) {
- nextPC.compressed(true);
- nextPC.npc(nextPC.pc() + sizeof(MachInst)/2);
+ if (compressed(emi)) {
+ nextPC.npc(nextPC.instAddr() + sizeof(MachInst) / 2);
} else {
- nextPC.compressed(false);
- nextPC.npc(nextPC.pc() + sizeof(MachInst));
+ nextPC.npc(nextPC.instAddr() + sizeof(MachInst));
}
return decode(emi, nextPC.instAddr());
diff --git a/src/arch/riscv/decoder.hh b/src/arch/riscv/decoder.hh
index ef644fa..c1d68bf 100644
--- a/src/arch/riscv/decoder.hh
+++ b/src/arch/riscv/decoder.hh
@@ -49,7 +49,9 @@
{
private:
DecodeCache::InstMap instMap;
+ bool aligned;
bool mid;
+ bool more;
protected:
//The extended machine instruction being generated
@@ -57,18 +59,18 @@
bool instDone;
public:
- Decoder(ISA* isa=nullptr)
- : mid(false), emi(NoopMachInst), instDone(false)
- {}
+ Decoder(ISA* isa=nullptr) { reset(); }
void process() {}
- void reset() { instDone = false; }
+ void reset();
+
+ inline bool compressed(ExtMachInst inst) { return (inst & 0x3) < 0x3; }
//Use this to give data to the decoder. This should be used
//when there is control flow.
void moreBytes(const PCState &pc, Addr fetchPC, MachInst inst);
- bool needMoreBytes() { return true; }
+ bool needMoreBytes() { return more; }
bool instReady() { return instDone; }
void takeOverFrom(Decoder *old) {}
diff --git a/src/arch/riscv/faults.cc b/src/arch/riscv/faults.cc
index 58baa4e..4e44d43 100644
--- a/src/arch/riscv/faults.cc
+++ b/src/arch/riscv/faults.cc
@@ -64,6 +64,13 @@
}
void
+IllegalInstFault::invoke_se(ThreadContext *tc, const StaticInstPtr &inst)
+{
+ panic("Illegal instruction 0x%08x at pc 0x%016llx: %s", inst->machInst,
+ tc->pcState().pc(), reason.c_str());
+}
+
+void
UnimplementedFault::invoke_se(ThreadContext *tc,
const StaticInstPtr &inst)
{
diff --git a/src/arch/riscv/faults.hh b/src/arch/riscv/faults.hh
index d0d7988..1e33b64 100644
--- a/src/arch/riscv/faults.hh
+++ b/src/arch/riscv/faults.hh
@@ -116,6 +116,19 @@
invoke_se(ThreadContext *tc, const StaticInstPtr &inst);
};
+class IllegalInstFault : public RiscvFault
+{
+ private:
+ const std::string reason;
+ public:
+ IllegalInstFault(std::string r)
+ : RiscvFault("Illegal instruction", INST_ILLEGAL, SOFTWARE),
+ reason(r)
+ {}
+
+ void invoke_se(ThreadContext *tc, const StaticInstPtr &inst);
+};
+
class UnimplementedFault : public RiscvFault
{
private:
diff --git a/src/arch/riscv/isa/decoder.isa b/src/arch/riscv/isa/decoder.isa
index 4f4ef76..0e5567a 100644
--- a/src/arch/riscv/isa/decoder.isa
+++ b/src/arch/riscv/isa/decoder.isa
@@ -42,7 +42,8 @@
CIMM8<7:6> << 4 |
CIMM8<5:2> << 6;
}}, {{
- assert(imm != 0);
+ if (machInst == 0)
+ fault = make_shared<IllegalInstFault>("zero instruction");
Rp2 = sp + imm;
}});
format CompressedLoad {
@@ -103,7 +104,12 @@
if (CIMM1 > 0)
imm |= ~((uint64_t)0x1F);
}}, {{
- assert((RC1 == 0) == (imm == 0));
+ if ((RC1 == 0) != (imm == 0)) {
+ if (RC1 == 0) {
+ fault = make_shared<IllegalInstFault>("source reg
x0");
+ } else // imm == 0
+ fault = make_shared<IllegalInstFault>("immediate =
0");
+ }
Rc1_sd = Rc1_sd + imm;
}});
0x1: c_addiw({{
diff --git a/src/arch/riscv/isa/formats/fp.isa
b/src/arch/riscv/isa/formats/fp.isa
index 1f60b9b..3de0bb2 100644
--- a/src/arch/riscv/isa/formats/fp.isa
+++ b/src/arch/riscv/isa/formats/fp.isa
@@ -56,8 +56,8 @@
std::fesetround(FE_UPWARD);
break;
case 0x4:
- panic("Round to nearest, "
- "ties to max magnitude not implemented.");
+ // Round to nearest, ties to max magnitude not implemented
+ fault = make_shared<IllegalFrmFault>(ROUND_MODE);
break;
case 0x7: {
uint8_t frm = xc->readMiscReg(MISCREG_FRM);
@@ -75,8 +75,8 @@
std::fesetround(FE_UPWARD);
break;
case 0x4:
- panic("Round to nearest,"
- " ties to max magnitude not implemented.");
+ // Round to nearest, ties to max magnitude not
implemented
+ fault = make_shared<IllegalFrmFault>(ROUND_MODE);
break;
default:
fault = std::make_shared<IllegalFrmFault>(frm);
diff --git a/src/arch/riscv/utility.hh b/src/arch/riscv/utility.hh
index 38109a2..2be1637 100644
--- a/src/arch/riscv/utility.hh
+++ b/src/arch/riscv/utility.hh
@@ -48,6 +48,7 @@
#include <cmath>
#include <cstdint>
+#include <sstream>
#include <string>
#include "arch/riscv/registers.hh"
@@ -133,8 +134,18 @@
registerName(RegId reg)
{
if (reg.isIntReg()) {
+ if (reg.index() >= NumIntArchRegs) {
+ std::stringstream str;
+ str << "?? (x" << reg.index() << ')';
+ return str.str();
+ }
return IntRegNames[reg.index()];
} else {
+ if (reg.index() >= NumFloatRegs) {
+ std::stringstream str;
+ str << "?? (f" << reg.index() << ')';
+ return str.str();
+ }
return FloatRegNames[reg.index()];
}
}
--
To view, visit https://gem5-review.googlesource.com/4041
To unsubscribe, visit https://gem5-review.googlesource.com/settings
Gerrit-Project: public/gem5
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I3f333008430d4a905cb59547a3513f5149b43b95
Gerrit-Change-Number: 4041
Gerrit-PatchSet: 1
Gerrit-Owner: Alec Roelke <ar...@virginia.edu>
_______________________________________________
gem5-dev mailing list
gem5-dev@gem5.org
http://m5sim.org/mailman/listinfo/gem5-dev
