Gabe Black has uploaded this change for review. (
https://gem5-review.googlesource.com/c/public/gem5/+/45520 )
Change subject: arch-riscv: When an inst generates a fault, return it
immediately.
......................................................................
arch-riscv: When an inst generates a fault, return it immediately.
When a fault is generated, it needs to be returned, and nothing else
should be done. There's no point in keeping it around and having to
check over and over if there was a fault and if other parts of the
execute functions should be skipped.
This simplifies the logic a bit which should speed up execution, and
also makes life easier for the compiler since behavior is obvious and
doesn't have to be deduced from possible data values and ifs.
Change-Id: I2004c7d22ac6222e1ef2acb51d49b4eb2e60b144
---
M src/arch/riscv/fp_inst.hh
M src/arch/riscv/isa/decoder.isa
M src/arch/riscv/isa/formats/amo.isa
M src/arch/riscv/isa/formats/basic.isa
M src/arch/riscv/isa/formats/compressed.isa
M src/arch/riscv/isa/formats/fp.isa
M src/arch/riscv/isa/formats/mem.isa
M src/arch/riscv/isa/formats/standard.isa
8 files changed, 195 insertions(+), 324 deletions(-)
diff --git a/src/arch/riscv/fp_inst.hh b/src/arch/riscv/fp_inst.hh
index 3a1e2d6..604c016 100644
--- a/src/arch/riscv/fp_inst.hh
+++ b/src/arch/riscv/fp_inst.hh
@@ -34,10 +34,10 @@
#define
RM_REQUIRED \
uint_fast8_t rm =
ROUND_MODE; \
uint_fast8_t frm =
xc->readMiscReg(MISCREG_FRM); \
- if (rm ==
7) \
+ if (rm ==
7) \
rm =
frm; \
- if (rm >
4) \
- fault = std::make_shared<IllegalInstFault>("RM fault",
machInst);\
+ if (rm >
4) \
+ return std::make_shared<IllegalInstFault>("RM fault",
machInst);\
softfloat_roundingMode =
rm; \
#endif // __ARCH_RISCV_FP_INST_HH__
diff --git a/src/arch/riscv/isa/decoder.isa b/src/arch/riscv/isa/decoder.isa
index 4f4bdb7..823698c 100644
--- a/src/arch/riscv/isa/decoder.isa
+++ b/src/arch/riscv/isa/decoder.isa
@@ -43,7 +43,7 @@
CIMM8<5:2> << 6;
}}, {{
if (machInst == 0)
- fault = std::make_shared<IllegalInstFault>("zero
instruction",
+ return std::make_shared<IllegalInstFault>("zero
instruction",
machInst);
Rp2 = sp + imm;
}}, uint64_t);
@@ -53,7 +53,7 @@
}}, {{
STATUS status = xc->readMiscReg(MISCREG_STATUS);
if (status.fs == FPUStatus::OFF)
- fault = std::make_shared<IllegalInstFault>("FPU is
off",
+ return std::make_shared<IllegalInstFault>("FPU is off",
machInst);
Fp2_bits = Mem;
@@ -83,7 +83,7 @@
}}, {{
STATUS status = xc->readMiscReg(MISCREG_STATUS);
if (status.fs == FPUStatus::OFF)
- fault = std::make_shared<IllegalInstFault>("FPU is
off",
+ return std::make_shared<IllegalInstFault>("FPU is off",
machInst);
Mem = Fp2_bits;
@@ -117,11 +117,12 @@
}}, {{
if ((RC1 == 0) != (imm == 0)) {
if (RC1 == 0) {
- fault = std::make_shared<IllegalInstFault>(
+ return std::make_shared<IllegalInstFault>(
"source reg x0", machInst);
- } else // imm == 0
- fault = std::make_shared<IllegalInstFault>(
+ } else { // imm == 0
+ return std::make_shared<IllegalInstFault>(
"immediate = 0", machInst);
+ }
}
Rc1_sd = Rc1_sd + imm;
}});
@@ -131,7 +132,7 @@
imm |= ~((uint64_t)0x1F);
}}, {{
if (RC1 == 0) {
- fault = std::make_shared<IllegalInstFault>(
+ return std::make_shared<IllegalInstFault>(
"source reg x0", machInst);
}
Rc1_sd = (int32_t)Rc1_sd + imm;
@@ -142,7 +143,7 @@
imm |= ~((uint64_t)0x1F);
}}, {{
if (RC1 == 0) {
- fault = std::make_shared<IllegalInstFault>(
+ return std::make_shared<IllegalInstFault>(
"source reg x0", machInst);
}
Rc1_sd = imm;
@@ -157,7 +158,7 @@
imm |= ~((int64_t)0x1FF);
}}, {{
if (imm == 0) {
- fault = std::make_shared<IllegalInstFault>(
+ return std::make_shared<IllegalInstFault>(
"immediate = 0", machInst);
}
sp_sd = sp_sd + imm;
@@ -168,11 +169,11 @@
imm |= ~((uint64_t)0x1FFFF);
}}, {{
if (RC1 == 0 || RC1 == 2) {
- fault = std::make_shared<IllegalInstFault>(
+ return std::make_shared<IllegalInstFault>(
"source reg x0", machInst);
}
if (imm == 0) {
- fault = std::make_shared<IllegalInstFault>(
+ return std::make_shared<IllegalInstFault>(
"immediate = 0", machInst);
}
Rc1_sd = imm;
@@ -185,7 +186,7 @@
imm = CIMM5 | (CIMM1 << 5);
}}, {{
if (imm == 0) {
- fault = std::make_shared<IllegalInstFault>(
+ return std::make_shared<IllegalInstFault>(
"immediate = 0", machInst);
}
Rp1 = Rp1 >> imm;
@@ -194,7 +195,7 @@
imm = CIMM5 | (CIMM1 << 5);
}}, {{
if (imm == 0) {
- fault = std::make_shared<IllegalInstFault>(
+ return std::make_shared<IllegalInstFault>(
"immediate = 0", machInst);
}
Rp1_sd = Rp1_sd >> imm;
@@ -257,11 +258,11 @@
imm = CIMM5 | (CIMM1 << 5);
}}, {{
if (imm == 0) {
- fault = std::make_shared<IllegalInstFault>(
+ return std::make_shared<IllegalInstFault>(
"immediate = 0", machInst);
}
if (RC1 == 0) {
- fault = std::make_shared<IllegalInstFault>(
+ return std::make_shared<IllegalInstFault>(
"source reg x0", machInst);
}
Rc1 = Rc1 << imm;
@@ -282,7 +283,7 @@
CIMM5<1:0> << 6;
}}, {{
if (RC1 == 0) {
- fault = std::make_shared<IllegalInstFault>(
+ return std::make_shared<IllegalInstFault>(
"source reg x0", machInst);
}
Rc1_sd = Mem_sw;
@@ -295,7 +296,7 @@
CIMM5<2:0> << 6;
}}, {{
if (RC1 == 0) {
- fault = std::make_shared<IllegalInstFault>(
+ return std::make_shared<IllegalInstFault>(
"source reg x0", machInst);
}
Rc1_sd = Mem_sd;
@@ -307,14 +308,14 @@
0x0: decode RC2 {
0x0: Jump::c_jr({{
if (RC1 == 0) {
- fault = std::make_shared<IllegalInstFault>(
+ return std::make_shared<IllegalInstFault>(
"source reg x0", machInst);
}
NPC = Rc1;
}}, IsIndirectControl, IsUncondControl, IsCall);
default: CROp::c_mv({{
if (RC1 == 0) {
- fault = std::make_shared<IllegalInstFault>(
+ return std::make_shared<IllegalInstFault>(
"source reg x0", machInst);
}
Rc1 = Rc2;
@@ -323,15 +324,15 @@
0x1: decode RC1 {
0x0: SystemOp::c_ebreak({{
if (RC2 != 0) {
- fault = std::make_shared<IllegalInstFault>(
+ return std::make_shared<IllegalInstFault>(
"source reg x1", machInst);
}
- fault =
std::make_shared<BreakpointFault>(xc->pcState());
+ return
std::make_shared<BreakpointFault>(xc->pcState());
}}, IsSerializeAfter, IsNonSpeculative, No_OpClass);
default: decode RC2 {
0x0: Jump::c_jalr({{
if (RC1 == 0) {
- fault = std::make_shared<IllegalInstFault>(
+ return std::make_shared<IllegalInstFault>(
"source reg x0", machInst);
}
ra = NPC;
@@ -402,7 +403,7 @@
0x2: flw({{
STATUS status = xc->readMiscReg(MISCREG_STATUS);
if (status.fs == FPUStatus::OFF)
- fault = std::make_shared<IllegalInstFault>(
+ return std::make_shared<IllegalInstFault>(
"FPU is off", machInst);
freg_t fd;
fd = freg(f32(Mem_uw));
@@ -411,7 +412,7 @@
0x3: fld({{
STATUS status = xc->readMiscReg(MISCREG_STATUS);
if (status.fs == FPUStatus::OFF)
- fault = std::make_shared<IllegalInstFault>(
+ return std::make_shared<IllegalInstFault>(
"FPU is off", machInst);
freg_t fd;
fd = freg(f64(Mem));
@@ -508,7 +509,7 @@
0x2: fsw({{
STATUS status = xc->readMiscReg(MISCREG_STATUS);
if (status.fs == FPUStatus::OFF)
- fault = std::make_shared<IllegalInstFault>(
+ return std::make_shared<IllegalInstFault>(
"FPU is off", machInst);
Mem_uw = (uint32_t)Fs2_bits;
@@ -516,7 +517,7 @@
0x3: fsd({{
STATUS status = xc->readMiscReg(MISCREG_STATUS);
if (status.fs == FPUStatus::OFF)
- fault = std::make_shared<IllegalInstFault>(
+ return std::make_shared<IllegalInstFault>(
"FPU is off", machInst);
Mem_ud = Fs2_bits;
@@ -1112,7 +1113,7 @@
}
0x20: fcvt_s_d({{
if (CONV_SGN != 1) {
- fault = std::make_shared<IllegalInstFault>(
+ return std::make_shared<IllegalInstFault>(
"CONV_SGN != 1", machInst);
}
RM_REQUIRED;
@@ -1122,7 +1123,7 @@
}}, FloatCvtOp);
0x21: fcvt_d_s({{
if (CONV_SGN != 0) {
- fault = std::make_shared<IllegalInstFault>(
+ return std::make_shared<IllegalInstFault>(
"CONV_SGN != 0", machInst);
}
RM_REQUIRED;
@@ -1132,7 +1133,7 @@
}}, FloatCvtOp);
0x2c: fsqrt_s({{
if (RS2 != 0) {
- fault = std::make_shared<IllegalInstFault>(
+ return std::make_shared<IllegalInstFault>(
"source reg x1", machInst);
}
freg_t fd;
@@ -1142,7 +1143,7 @@
}}, FloatSqrtOp);
0x2d: fsqrt_d({{
if (RS2 != 0) {
- fault = std::make_shared<IllegalInstFault>(
+ return std::make_shared<IllegalInstFault>(
"source reg x1", machInst);
}
freg_t fd;
@@ -1352,12 +1353,12 @@
0x0: decode FUNCT7 {
0x0: decode RS2 {
0x0: ecall({{
- fault = std::make_shared<SyscallFault>(
+ return std::make_shared<SyscallFault>(
(PrivilegeMode)xc->readMiscReg(MISCREG_PRV));
}}, IsSerializeAfter, IsNonSpeculative, IsSyscall,
No_OpClass);
0x1: ebreak({{
- fault = std::make_shared<BreakpointFault>(
+ return std::make_shared<BreakpointFault>(
xc->pcState());
}}, IsSerializeAfter, IsNonSpeculative,
No_OpClass);
0x2: uret({{
@@ -1375,7 +1376,7 @@
MISCREG_PRV);
if (pm == PRV_U ||
(pm == PRV_S && status.tsr == 1)) {
- fault = std::make_shared<IllegalInstFault>(
+ return std::make_shared<IllegalInstFault>(
"sret in user mode or TSR
enabled",
machInst);
NPC = NPC;
@@ -1394,7 +1395,7 @@
MISCREG_PRV);
if (pm == PRV_U ||
(pm == PRV_S && status.tw == 1)) {
- fault = std::make_shared<IllegalInstFault>(
+ return std::make_shared<IllegalInstFault>(
"wfi in user mode or TW
enabled",
machInst);
}
@@ -1405,7 +1406,7 @@
STATUS status = xc->readMiscReg(MISCREG_STATUS);
auto pm =
(PrivilegeMode)xc->readMiscReg(MISCREG_PRV);
if (pm == PRV_U || (pm == PRV_S && status.tvm ==
1)) {
- fault = std::make_shared<IllegalInstFault>(
+ return std::make_shared<IllegalInstFault>(
"sfence in user mode or TVM
enabled",
machInst);
}
@@ -1413,7 +1414,7 @@
}}, IsNonSpeculative, IsSerializeAfter, No_OpClass);
0x18: mret({{
if (xc->readMiscReg(MISCREG_PRV) != PRV_M) {
- fault = std::make_shared<IllegalInstFault>(
+ return std::make_shared<IllegalInstFault>(
"mret at lower privilege",
machInst);
NPC = NPC;
} else {
diff --git a/src/arch/riscv/isa/formats/amo.isa
b/src/arch/riscv/isa/formats/amo.isa
index cde0fd8..7d4d11d 100644
--- a/src/arch/riscv/isa/formats/amo.isa
+++ b/src/arch/riscv/isa/formats/amo.isa
@@ -236,22 +236,22 @@
ExecContext *xc, Trace::InstRecord *traceData) const
{
Addr EA;
- Fault fault = NoFault;
%(op_decl)s;
%(op_rd)s;
%(ea_code)s;
- if (fault == NoFault) {
- fault = readMemAtomicLE(xc, traceData, EA, Mem,
memAccessFlags);
- %(memacc_code)s;
+ {
+ Fault fault =
+ readMemAtomicLE(xc, traceData, EA, Mem, memAccessFlags);
+ if (fault != NoFault)
+ return fault;
}
- if (fault == NoFault) {
- %(op_wb)s;
- }
+ %(memacc_code)s;
+ %(op_wb)s;
- return fault;
+ return NoFault;
}
}};
@@ -260,34 +260,27 @@
Trace::InstRecord *traceData) const
{
Addr EA;
- Fault fault = NoFault;
uint64_t result;
%(op_decl)s;
%(op_rd)s;
%(ea_code)s;
- if (fault == NoFault) {
- %(memacc_code)s;
- }
+ %(memacc_code)s;
- if (fault == NoFault) {
- fault = writeMemAtomicLE(xc, traceData, Mem, EA,
memAccessFlags,
- &result);
- // RISC-V has the opposite convention gem5 has for success
flags,
- // so we invert the result here.
- result = !result;
+ {
+ Fault fault =
+ writeMemAtomicLE(xc, traceData, Mem, EA, memAccessFlags,
+ &result);
}
+ // RISC-V has the opposite convention gem5 has for success flags,
+ // so we invert the result here.
+ result = !result;
- if (fault == NoFault) {
- %(postacc_code)s;
- }
+ %(postacc_code)s;
+ %(op_wb)s;
- if (fault == NoFault) {
- %(op_wb)s;
- }
-
- return fault;
+ return NoFault;
}
}};
@@ -296,7 +289,6 @@
Trace::InstRecord *traceData) const
{
Addr EA;
- Fault fault = NoFault;
%(op_decl)s;
%(op_rd)s;
@@ -305,21 +297,16 @@
assert(amo_op);
- if (fault == NoFault) {
- fault = amoMemAtomicLE(xc, traceData, Mem, EA, memAccessFlags,
- amo_op);
- %(memacc_code)s;
+ {
+ Fault fault =
+ amoMemAtomicLE(xc, traceData, Mem, EA, memAccessFlags,
amo_op);
}
- if (fault == NoFault) {
- %(postacc_code)s;
- }
+ %(memacc_code)s;
+ %(postacc_code)s;
+ %(op_wb)s;
- if (fault == NoFault) {
- %(op_wb)s;
- }
-
- return fault;
+ return NoFault;
}
}};
@@ -331,17 +318,12 @@
Trace::InstRecord *traceData) const
{
Addr EA;
- Fault fault = NoFault;
%(op_src_decl)s;
%(op_rd)s;
%(ea_code)s;
- if (fault == NoFault) {
- fault = initiateMemRead(xc, traceData, EA, Mem,
memAccessFlags);
- }
-
- return fault;
+ return initiateMemRead(xc, traceData, EA, Mem, memAccessFlags);
}
}};
@@ -351,26 +333,22 @@
Trace::InstRecord *traceData) const
{
Addr EA;
- Fault fault = NoFault;
%(op_decl)s;
%(op_rd)s;
%(ea_code)s;
+ %(memacc_code)s;
- if (fault == NoFault) {
- %(memacc_code)s;
- }
-
- if (fault == NoFault) {
- fault = writeMemTimingLE(xc, traceData, Mem, EA,
+ {
+ Fault fault = writeMemTimingLE(xc, traceData, Mem, EA,
memAccessFlags, nullptr);
+ if (fault != NoFault)
+ return fault;
}
- if (fault == NoFault) {
- %(op_wb)s;
- }
+ %(op_wb)s;
- return fault;
+ return NoFault;
}
}};
@@ -380,7 +358,6 @@
Trace::InstRecord *traceData) const
{
Addr EA;
- Fault fault = NoFault;
%(op_src_decl)s;
%(op_rd)s;
@@ -389,12 +366,7 @@
assert(amo_op);
- if (fault == NoFault) {
- fault = initiateMemAMO(xc, traceData, EA, Mem, memAccessFlags,
- amo_op);
- }
-
- return fault;
+ return initiateMemAMO(xc, traceData, EA, Mem, memAccessFlags,
amo_op);
}
}};
@@ -405,22 +377,15 @@
%(class_name)s::%(class_name)sMicro::completeAcc(PacketPtr pkt,
ExecContext *xc, Trace::InstRecord *traceData) const
{
- Fault fault = NoFault;
-
%(op_decl)s;
%(op_rd)s;
getMemLE(pkt, Mem, traceData);
- if (fault == NoFault) {
- %(memacc_code)s;
- }
+ %(memacc_code)s;
+ %(op_wb)s;
- if (fault == NoFault) {
- %(op_wb)s;
- }
-
- return fault;
+ return NoFault;
}
}};
@@ -428,23 +393,16 @@
Fault %(class_name)s::%(class_name)sMicro::completeAcc(Packet *pkt,
ExecContext *xc, Trace::InstRecord *traceData) const
{
- Fault fault = NoFault;
-
%(op_dest_decl)s;
// RISC-V has the opposite convention gem5 has for success flags,
// so we invert the result here.
uint64_t result = !pkt->req->getExtraData();
- if (fault == NoFault) {
- %(postacc_code)s;
- }
+ %(postacc_code)s;
+ %(op_wb)s;
- if (fault == NoFault) {
- %(op_wb)s;
- }
-
- return fault;
+ return NoFault;
}
}};
@@ -452,22 +410,15 @@
Fault %(class_name)s::%(class_name)sRMW::completeAcc(Packet *pkt,
ExecContext *xc, Trace::InstRecord *traceData) const
{
- Fault fault = NoFault;
-
%(op_decl)s;
%(op_rd)s;
getMemLE(pkt, Mem, traceData);
- if (fault == NoFault) {
- %(memacc_code)s;
- }
+ %(memacc_code)s;
+ %(op_wb)s;
- if (fault == NoFault) {
- %(op_wb)s;
- }
-
- return fault;
+ return NoFault;
}
}};
diff --git a/src/arch/riscv/isa/formats/basic.isa
b/src/arch/riscv/isa/formats/basic.isa
index 4b9f6c9..416b458 100644
--- a/src/arch/riscv/isa/formats/basic.isa
+++ b/src/arch/riscv/isa/formats/basic.isa
@@ -62,17 +62,11 @@
%(class_name)s::execute(ExecContext *xc,
Trace::InstRecord *traceData) const
{
- Fault fault = NoFault;
-
%(op_decl)s;
%(op_rd)s;
- if (fault == NoFault) {
- %(code)s;
- if (fault == NoFault) {
- %(op_wb)s;
- }
- }
- return fault;
+ %(code)s;
+ %(op_wb)s;
+ return NoFault;
}
}};
diff --git a/src/arch/riscv/isa/formats/compressed.isa
b/src/arch/riscv/isa/formats/compressed.isa
index ad73383..d467ea6 100644
--- a/src/arch/riscv/isa/formats/compressed.isa
+++ b/src/arch/riscv/isa/formats/compressed.isa
@@ -138,17 +138,11 @@
%(class_name)s::execute(ExecContext *xc,
Trace::InstRecord *traceData) const
{
- Fault fault = NoFault;
-
%(op_decl)s;
%(op_rd)s;
- if (fault == NoFault) {
- %(code)s;
- if (fault == NoFault) {
- %(op_wb)s;
- }
- }
- return fault;
+ %(code)s;
+ %(op_wb)s;
+ return NoFault;
}
std::string
diff --git a/src/arch/riscv/isa/formats/fp.isa
b/src/arch/riscv/isa/formats/fp.isa
index 1181cb6..d015239 100644
--- a/src/arch/riscv/isa/formats/fp.isa
+++ b/src/arch/riscv/isa/formats/fp.isa
@@ -36,30 +36,24 @@
Fault %(class_name)s::execute(ExecContext *xc,
Trace::InstRecord *traceData) const
{
- Fault fault = NoFault;
-
STATUS status = xc->readMiscReg(MISCREG_STATUS);
if (status.fs == FPUStatus::OFF)
- fault = std::make_shared<IllegalInstFault>("FPU is off",
machInst);
+ return std::make_shared<IllegalInstFault>("FPU is off",
machInst);
%(op_decl)s;
%(op_rd)s;
- if (fault == NoFault) {
- RegVal FFLAGS = xc->readMiscReg(MISCREG_FFLAGS);
- std::feclearexcept(FE_ALL_EXCEPT);
- %(code)s;
+ RegVal FFLAGS = xc->readMiscReg(MISCREG_FFLAGS);
+ std::feclearexcept(FE_ALL_EXCEPT);
+ %(code)s;
- FFLAGS |= softfloat_exceptionFlags;
- softfloat_exceptionFlags = 0;
- xc->setMiscReg(MISCREG_FFLAGS, FFLAGS);
- }
+ FFLAGS |= softfloat_exceptionFlags;
+ softfloat_exceptionFlags = 0;
+ xc->setMiscReg(MISCREG_FFLAGS, FFLAGS);
- if (fault == NoFault) {
- %(op_wb)s;
- }
+ %(op_wb)s;
- return fault;
+ return NoFault;
}
}};
diff --git a/src/arch/riscv/isa/formats/mem.isa
b/src/arch/riscv/isa/formats/mem.isa
index 1dd9dc2..a1005b4 100644
--- a/src/arch/riscv/isa/formats/mem.isa
+++ b/src/arch/riscv/isa/formats/mem.isa
@@ -101,22 +101,23 @@
ExecContext *xc, Trace::InstRecord *traceData) const
{
Addr EA;
- Fault fault = NoFault;
%(op_decl)s;
%(op_rd)s;
%(ea_code)s;
- if (fault == NoFault) {
- fault = readMemAtomicLE(xc, traceData, EA, Mem,
memAccessFlags);
- %(memacc_code)s;
+ {
+ Fault fault =
+ readMemAtomicLE(xc, traceData, EA, Mem, memAccessFlags);
+ if (fault != NoFault)
+ return fault;
}
- if (fault == NoFault) {
- %(op_wb)s;
- }
+ %(memacc_code)s;
- return fault;
+ %(op_wb)s;
+
+ return NoFault;
}
}};
@@ -126,17 +127,12 @@
Trace::InstRecord *traceData) const
{
Addr EA;
- Fault fault = NoFault;
%(op_src_decl)s;
%(op_rd)s;
%(ea_code)s;
- if (fault == NoFault) {
- fault = initiateMemRead(xc, traceData, EA, Mem,
memAccessFlags);
- }
-
- return fault;
+ return initiateMemRead(xc, traceData, EA, Mem, memAccessFlags);
}
}};
@@ -145,22 +141,15 @@
%(class_name)s::completeAcc(PacketPtr pkt, ExecContext *xc,
Trace::InstRecord *traceData) const
{
- Fault fault = NoFault;
-
%(op_decl)s;
%(op_rd)s;
getMemLE(pkt, Mem, traceData);
- if (fault == NoFault) {
- %(memacc_code)s;
- }
+ %(memacc_code)s;
+ %(op_wb)s;
- if (fault == NoFault) {
- %(op_wb)s;
- }
-
- return fault;
+ return NoFault;
}
}};
@@ -170,30 +159,25 @@
Trace::InstRecord *traceData) const
{
Addr EA;
- Fault fault = NoFault;
%(op_decl)s;
%(op_rd)s;
%(ea_code)s;
- if (fault == NoFault) {
- %(memacc_code)s;
+ %(memacc_code)s;
+
+ {
+ Fault fault =
+ writeMemAtomicLE(xc, traceData, Mem, EA, memAccessFlags,
+ nullptr);
+ if (fault != NoFault)
+ return fault;
}
- if (fault == NoFault) {
- fault = writeMemAtomicLE(xc, traceData, Mem, EA,
memAccessFlags,
- nullptr);
- }
+ %(postacc_code)s;
+ %(op_wb)s;
- if (fault == NoFault) {
- %(postacc_code)s;
- }
-
- if (fault == NoFault) {
- %(op_wb)s;
- }
-
- return fault;
+ return NoFault;
}
}};
@@ -203,26 +187,23 @@
Trace::InstRecord *traceData) const
{
Addr EA;
- Fault fault = NoFault;
%(op_decl)s;
%(op_rd)s;
%(ea_code)s;
- if (fault == NoFault) {
- %(memacc_code)s;
- }
+ %(memacc_code)s;
- if (fault == NoFault) {
- fault = writeMemTimingLE(xc, traceData, Mem, EA,
+ {
+ Fault fault = writeMemTimingLE(xc, traceData, Mem, EA,
memAccessFlags, nullptr);
+ if (fault != NoFault)
+ return fault;
}
- if (fault == NoFault) {
- %(op_wb)s;
- }
+ %(op_wb)s;
- return fault;
+ return NoFault;
}
}};
diff --git a/src/arch/riscv/isa/formats/standard.isa
b/src/arch/riscv/isa/formats/standard.isa
index 4ed35c9..c31682d 100644
--- a/src/arch/riscv/isa/formats/standard.isa
+++ b/src/arch/riscv/isa/formats/standard.isa
@@ -66,17 +66,11 @@
%(class_name)s::execute(
ExecContext *xc, Trace::InstRecord *traceData) const
{
- Fault fault = NoFault;
-
%(op_decl)s;
%(op_rd)s;
- if (fault == NoFault) {
- %(code)s;
- if (fault == NoFault) {
- %(op_wb)s;
- }
- }
- return fault;
+ %(code)s;
+ %(op_wb)s;
+ return NoFault;
}
std::string
@@ -98,17 +92,11 @@
%(class_name)s::execute(
ExecContext *xc, Trace::InstRecord *traceData) const
{
- Fault fault = NoFault;
-
%(op_decl)s;
%(op_rd)s;
- if (fault == NoFault) {
- %(code)s;
- if (fault == NoFault) {
- %(op_wb)s;
- }
- }
- return fault;
+ %(code)s;
+ %(op_wb)s;
+ return NoFault;
}
std::string
@@ -132,17 +120,11 @@
%(class_name)s::execute(
ExecContext *xc, Trace::InstRecord *traceData) const
{
- Fault fault = NoFault;
-
%(op_decl)s;
%(op_rd)s;
- if (fault == NoFault) {
- %(code)s;
- if (fault == NoFault) {
- %(op_wb)s;
- }
- }
- return fault;
+ %(code)s;
+ %(op_wb)s;
+ return NoFault;
}
std::string
@@ -205,17 +187,11 @@
%(class_name)s::execute(ExecContext *xc,
Trace::InstRecord *traceData) const
{
- Fault fault = NoFault;
-
%(op_decl)s;
%(op_rd)s;
- if (fault == NoFault) {
- %(code)s;
- if (fault == NoFault) {
- %(op_wb)s;
- }
- }
- return fault;
+ %(code)s;
+ %(op_wb)s;
+ return NoFault;
}
RiscvISA::PCState
@@ -268,17 +244,11 @@
%(class_name)s::execute(
ExecContext *xc, Trace::InstRecord *traceData) const
{
- Fault fault = NoFault;
-
%(op_decl)s;
%(op_rd)s;
- if (fault == NoFault) {
- %(code)s;
- if (fault == NoFault) {
- %(op_wb)s;
- }
- }
- return fault;
+ %(code)s;
+ %(op_wb)s;
+ return NoFault;
}
RiscvISA::PCState
@@ -309,8 +279,6 @@
%(class_name)s::execute(ExecContext *xc,
Trace::InstRecord *traceData) const
{
- Fault fault = NoFault;
-
%(op_decl)s;
%(op_rd)s;
@@ -324,10 +292,9 @@
auto pm = (PrivilegeMode)xc->readMiscReg(MISCREG_PRV);
STATUS status = xc->readMiscReg(MISCREG_STATUS);
if (pm == PRV_U || (pm == PRV_S && status.tvm == 1)) {
- std::string error = csprintf(
- "SATP access in user mode or with TVM enabled\n");
- fault = std::make_shared<IllegalInstFault>(error,
machInst);
- olddata = 0;
+ return std::make_shared<IllegalInstFault>(
+ "SATP access in user mode or with TVM enabled\n",
+ machInst);
} else {
olddata = xc->readMiscReg(CSRData.at(csr).physIndex);
}
@@ -336,10 +303,9 @@
case CSR_MSTATUS: {
auto pm = (PrivilegeMode)xc->readMiscReg(MISCREG_PRV);
if (pm != PrivilegeMode::PRV_M) {
- std::string error = csprintf(
- "MSTATUS is only accessibly in machine mode\n");
- fault = std::make_shared<IllegalInstFault>(error,
machInst);
- olddata = 0;
+ return std::make_shared<IllegalInstFault>(
+ "MSTATUS is only accessibly in machine mode\n",
+ machInst);
} else {
olddata = xc->readMiscReg(CSRData.at(csr).physIndex);
}
@@ -349,9 +315,8 @@
if (CSRData.find(csr) != CSRData.end()) {
olddata = xc->readMiscReg(CSRData.at(csr).physIndex);
} else {
- std::string error = csprintf("Illegal CSR index %#x\n",
csr);
- fault = std::make_shared<IllegalInstFault>(error,
machInst);
- olddata = 0;
+ return std::make_shared<IllegalInstFault>(
+ csprintf("Illegal CSR index %#x\n", csr),
machInst);
}
break;
}
@@ -363,60 +328,51 @@
olddata);
data = olddata;
- if (fault == NoFault) {
- %(code)s;
- if (fault == NoFault) {
- if (mask != CSRMasks.end())
- data &= mask->second;
- if (data != olddata) {
- if (bits(csr, 11, 10) == 0x3) {
- std::string error = csprintf("CSR %s is
read-only\n",
- CSRData.at(csr).name);
- fault = std::make_shared<IllegalInstFault>(
- error, machInst);
- } else {
- auto newdata_all = data;
- if (mask != CSRMasks.end()) {
- // we must keep those original bits not in mask
- // olddata and data only contain thebits
visable
- // in current privilige level
- newdata_all = (olddata_all & (~mask->second))
- | data;
- }
- DPRINTF(RiscvMisc, "Writing %#x to CSR %s.\n",
- newdata_all,
- CSRData.at(csr).name);
- switch (csr) {
- case CSR_FCSR:
- xc->setMiscReg(MISCREG_FFLAGS, bits(data, 4,
0));
- xc->setMiscReg(MISCREG_FRM, bits(data, 7, 5));
- break;
- case CSR_MIP: case CSR_MIE:
- case CSR_SIP: case CSR_SIE:
- case CSR_UIP: case CSR_UIE:
- case CSR_MSTATUS: case CSR_SSTATUS: case
CSR_USTATUS:
- if (newdata_all != olddata_all) {
- xc->setMiscReg(CSRData.at(csr).physIndex,
- newdata_all);
- } else {
- std::string error = "Only bits in mask
are "
- "allowed to be set\n";
- fault = std::make_shared<IllegalInstFault>(
- error, machInst);
- }
- break;
- default:
- xc->setMiscReg(CSRData.at(csr).physIndex,
data);
- break;
- }
- }
- }
+ %(code)s;
+ if (mask != CSRMasks.end())
+ data &= mask->second;
+ if (data != olddata) {
+ if (bits(csr, 11, 10) == 0x3) {
+ return std::make_shared<IllegalInstFault>(
+ csprintf("CSR %s is read-only\n",
+ CSRData.at(csr).name), machInst);
}
- if (fault == NoFault) {
- %(op_wb)s;
+ auto newdata_all = data;
+ if (mask != CSRMasks.end()) {
+ // we must keep those original bits not in mask
+ // olddata and data only contain thebits visable
+ // in current privilige level
+ newdata_all = (olddata_all & (~mask->second))
+ | data;
+ }
+ DPRINTF(RiscvMisc, "Writing %#x to CSR %s.\n",
+ newdata_all,
+ CSRData.at(csr).name);
+ switch (csr) {
+ case CSR_FCSR:
+ xc->setMiscReg(MISCREG_FFLAGS, bits(data, 4, 0));
+ xc->setMiscReg(MISCREG_FRM, bits(data, 7, 5));
+ break;
+ case CSR_MIP: case CSR_MIE:
+ case CSR_SIP: case CSR_SIE:
+ case CSR_UIP: case CSR_UIE:
+ case CSR_MSTATUS: case CSR_SSTATUS: case CSR_USTATUS:
+ if (newdata_all != olddata_all) {
+ xc->setMiscReg(CSRData.at(csr).physIndex,
+ newdata_all);
+ } else {
+ return std::make_shared<IllegalInstFault>(
+ "Only bits in mask are allowed to be set\n",
+ machInst);
+ }
+ break;
+ default:
+ xc->setMiscReg(CSRData.at(csr).physIndex, data);
+ break;
}
}
- return fault;
+ %(op_wb)s;
+ return NoFault;
}
}};
--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/45520
To unsubscribe, or for help writing mail filters, visit
https://gem5-review.googlesource.com/settings
Gerrit-Project: public/gem5
Gerrit-Branch: develop
Gerrit-Change-Id: I2004c7d22ac6222e1ef2acb51d49b4eb2e60b144
Gerrit-Change-Number: 45520
Gerrit-PatchSet: 1
Gerrit-Owner: Gabe Black <gabe.bl...@gmail.com>
Gerrit-MessageType: newchange
_______________________________________________
gem5-dev mailing list -- gem5-dev@gem5.org
To unsubscribe send an email to gem5-dev-le...@gem5.org
%(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s
