Skip to content

Commit 98099d8

Browse files
nemecadnemecad
committed
Machine: eliminate data duplication for privilege state
Store only `current_privilege_u` in CoreState and use accessor methods to convert to CSR::PrivilegeLevel. Resolves previous issues with duplicated privilege representation.
1 parent 4093fd3 commit 98099d8

File tree

2 files changed

+43
-40
lines changed

2 files changed

+43
-40
lines changed

‎src/machine/core.cpp‎

Lines changed: 35 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -3,10 +3,9 @@
33
#include "common/logging.h"
44
#include "execute/alu.h"
55
#include "utils.h"
6-
#include <iostream>
7-
#include <ostream>
86

97
#include <cinttypes>
8+
#include <ostream>
109

1110
LOG_CATEGORY("machine.core");
1211

@@ -63,8 +62,7 @@ void Core::reset() {
6362
state.cycle_count = 0;
6463
state.stall_count = 0;
6564
do_reset();
66-
state.current_privilege = CSR::PrivilegeLevel::MACHINE;
67-
state.current_privilege_u = static_cast<unsigned>(state.current_privilege);
65+
state.set_current_privilege(CSR::PrivilegeLevel::MACHINE);
6866
}
6967

7068
unsigned Core::get_cycle_count() const {
@@ -163,9 +161,8 @@ bool Core::handle_exception(
163161
if (control_state->read_internal(CSR::Id::MTVEC) != 0
164162
&& !get_step_over_exception(excause)) {
165163
control_state->exception_initiate(
166-
state.current_privilege, CSR::PrivilegeLevel::MACHINE);
167-
state.current_privilege = CSR::PrivilegeLevel::MACHINE;
168-
state.current_privilege_u = static_cast<unsigned>(state.current_privilege);
164+
state.current_privilege(), CSR::PrivilegeLevel::MACHINE);
165+
state.set_current_privilege(CSR::PrivilegeLevel::MACHINE);
169166
regs->write_pc(control_state->exception_pc_address());
170167
}
171168
}
@@ -199,16 +196,16 @@ static int32_t amo32_operations(enum AccessControl memctl, int32_t a, int32_t b)
199196
}
200197

201198
static int64_t amo64_operations(enum AccessControl memctl, int64_t a, int64_t b) {
202-
switch(memctl) {
199+
switch (memctl) {
203200
case AC_AMOSWAP64: return b;
204-
case AC_AMOADD64: return a + b;
205-
case AC_AMOXOR64: return a ^ b;
206-
case AC_AMOAND64: return a & b;
207-
case AC_AMOOR64: return a | b;
208-
case AC_AMOMIN64: return a < b? a: b;
209-
case AC_AMOMAX64: return a < b? b: a;
210-
case AC_AMOMINU64: return (uint64_t)a < (uint64_t)b? a: b;
211-
case AC_AMOMAXU64: return (uint64_t)a < (uint64_t)b? b: a;
201+
case AC_AMOADD64: return a + b;
202+
case AC_AMOXOR64: return a ^ b;
203+
case AC_AMOAND64: return a & b;
204+
case AC_AMOOR64: return a | b;
205+
case AC_AMOMIN64: return a < b ? a : b;
206+
case AC_AMOMAX64: return a < b ? b : a;
207+
case AC_AMOMINU64: return (uint64_t)a < (uint64_t)b ? a : b;
208+
case AC_AMOMAXU64: return (uint64_t)a < (uint64_t)b ? b : a;
212209
default: break;
213210
}
214211
return 0;
@@ -259,8 +256,7 @@ enum ExceptionCause Core::memory_special(
259256
towrite_val = 1;
260257
}
261258
break;
262-
case AC_FISRT_AMO_MODIFY32 ... AC_LAST_AMO_MODIFY32:
263-
{
259+
case AC_FISRT_AMO_MODIFY32 ... AC_LAST_AMO_MODIFY32: {
264260
if (!memread || !memwrite) { break; }
265261
int32_t fetched_value;
266262
fetched_value = (int32_t)(mem_data->read_u32(mem_addr));
@@ -269,8 +265,7 @@ enum ExceptionCause Core::memory_special(
269265
towrite_val = fetched_value;
270266
break;
271267
}
272-
case AC_FISRT_AMO_MODIFY64 ... AC_LAST_AMO_MODIFY64:
273-
{
268+
case AC_FISRT_AMO_MODIFY64 ... AC_LAST_AMO_MODIFY64: {
274269
if (!memread || !memwrite) { break; }
275270
int64_t fetched_value;
276271
fetched_value = (int64_t)(mem_data->read_u64(mem_addr));
@@ -294,9 +289,7 @@ FetchState Core::fetch(PCInterstage pc, bool skip_break) {
294289

295290
if (!skip_break && hw_breaks.contains(inst_addr)) { excause = EXCAUSE_HWBREAK; }
296291

297-
if (control_state != nullptr) {
298-
control_state->increment_internal(CSR::Id::MCYCLE, 1);
299-
}
292+
if (control_state != nullptr) { control_state->increment_internal(CSR::Id::MCYCLE, 1); }
300293

301294
if (control_state != nullptr && excause == EXCAUSE_NONE) {
302295
if (control_state->core_interrupt_request()) { excause = EXCAUSE_INT; }
@@ -322,9 +315,7 @@ DecodeState Core::decode(const FetchInterstage &dt) {
322315

323316
dt.inst.flags_alu_op_mem_ctl(flags, alu_op, mem_ctl);
324317

325-
if ((flags ^ check_inst_flags_val) & check_inst_flags_mask) {
326-
excause = EXCAUSE_INSN_ILLEGAL;
327-
}
318+
if ((flags ^ check_inst_flags_val) & check_inst_flags_mask) { excause = EXCAUSE_INSN_ILLEGAL; }
328319

329320
RegisterId num_rs = (flags & (IMF_ALU_REQ_RS | IMF_ALU_RS_ID)) ? dt.inst.rs() : 0;
330321
RegisterId num_rt = (flags & IMF_ALU_REQ_RT) ? dt.inst.rt() : 0;
@@ -350,8 +341,7 @@ DecodeState Core::decode(const FetchInterstage &dt) {
350341
// TODO: EXCAUSE_ECALL_S, EXCAUSE_ECALL_U
351342
}
352343
}
353-
if (flags & IMF_FORCE_W_OP)
354-
w_operation = true;
344+
if (flags & IMF_FORCE_W_OP) w_operation = true;
355345

356346
return { DecodeInternalState {
357347
.alu_op_num = static_cast<unsigned>(alu_op.alu_op),
@@ -373,8 +363,9 @@ DecodeState Core::decode(const FetchInterstage &dt) {
373363
.excause = excause,
374364
.ff_rs = FORWARD_NONE,
375365
.ff_rt = FORWARD_NONE,
376-
.alu_component = (flags & IMF_AMO) ? AluComponent::PASS :
377-
(flags & IMF_MUL) ? AluComponent::MUL : AluComponent::ALU,
366+
.alu_component = (flags & IMF_AMO) ? AluComponent::PASS
367+
: (flags & IMF_MUL) ? AluComponent::MUL
368+
: AluComponent::ALU,
378369
.aluop = alu_op,
379370
.memctl = mem_ctl,
380371
.num_rs = num_rs,
@@ -416,7 +407,8 @@ ExecuteState Core::execute(const DecodeInterstage &dt) {
416407
}();
417408
const RegisterValue alu_val = [=] {
418409
if (excause != EXCAUSE_NONE) return RegisterValue(0);
419-
return alu_combined_operate(dt.aluop, dt.alu_component, dt.w_operation, dt.alu_mod, alu_fst, alu_sec);
410+
return alu_combined_operate(
411+
dt.aluop, dt.alu_component, dt.w_operation, dt.alu_mod, alu_fst, alu_sec);
420412
}();
421413
const Address branch_jal_target = dt.inst_addr + dt.immediate_val.as_i64();
422414

@@ -509,11 +501,13 @@ MemoryState Core::memory(const ExecuteInterstage &dt) {
509501
// Predictor update
510502
if (dt.branch_jal) {
511503
// JAL Jump instruction (J-type (alternative to U-type with different immediate bit order))
512-
predictor->update(dt.inst, dt.inst_addr, dt.branch_jal_target, BranchType::JUMP, BranchResult::TAKEN);
504+
predictor->update(
505+
dt.inst, dt.inst_addr, dt.branch_jal_target, BranchType::JUMP, BranchResult::TAKEN);
513506
} else if (dt.branch_jalr) {
514507
// JALR Jump register instruction (I-type)
515508
predictor->update(
516-
dt.inst, dt.inst_addr, Address(get_xlen_from_reg(dt.alu_val)), BranchType::JUMP, BranchResult::TAKEN);
509+
dt.inst, dt.inst_addr, Address(get_xlen_from_reg(dt.alu_val)), BranchType::JUMP,
510+
BranchResult::TAKEN);
517511
} else if (dt.branch_bxx) {
518512
// BXX Conditional branch instruction (B-type (alternative to S-type with different
519513
// immediate bit order))
@@ -530,13 +524,15 @@ MemoryState Core::memory(const ExecuteInterstage &dt) {
530524
csr_written = true;
531525
}
532526
if (dt.xret) {
533-
CSR::PrivilegeLevel restored = control_state->exception_return(state.current_privilege);
534-
state.current_privilege = restored;
535-
state.current_privilege_u = static_cast<unsigned>(state.current_privilege);
527+
CSR::PrivilegeLevel restored
528+
= control_state->exception_return(state.current_privilege());
529+
state.set_current_privilege(restored);
536530
if (this->xlen == Xlen::_32)
537-
computed_next_inst_addr = Address(control_state->read_internal(CSR::Id::MEPC).as_u32());
531+
computed_next_inst_addr
532+
= Address(control_state->read_internal(CSR::Id::MEPC).as_u32());
538533
else
539-
computed_next_inst_addr = Address(control_state->read_internal(CSR::Id::MEPC).as_u64());
534+
computed_next_inst_addr
535+
= Address(control_state->read_internal(CSR::Id::MEPC).as_u64());
540536
csr_written = true;
541537
}
542538
}
@@ -586,7 +582,7 @@ WritebackState Core::writeback(const MemoryInterstage &dt) {
586582
if (dt.regwrite) { regs->write_gp(dt.num_rd, dt.towrite_val); }
587583

588584
return WritebackState { WritebackInternalState {
589-
.inst = (dt.excause == EXCAUSE_NONE)? dt.inst: Instruction::NOP,
585+
.inst = (dt.excause == EXCAUSE_NONE) ? dt.inst : Instruction::NOP,
590586
.inst_addr = dt.inst_addr,
591587
.value = dt.towrite_val,
592588
.num_rd = dt.num_rd,

‎src/machine/core/core_state.h‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,15 @@ struct CoreState {
1818
AddressRange LoadReservedRange;
1919
uint32_t stall_count = 0;
2020
uint32_t cycle_count = 0;
21-
CSR::PrivilegeLevel current_privilege = CSR::PrivilegeLevel::MACHINE;
2221
unsigned current_privilege_u = static_cast<unsigned>(CSR::PrivilegeLevel::MACHINE);
22+
23+
[[nodiscard]] CSR::PrivilegeLevel current_privilege() const noexcept {
24+
return static_cast<CSR::PrivilegeLevel>(current_privilege_u);
25+
}
26+
27+
void set_current_privilege(CSR::PrivilegeLevel p) noexcept {
28+
current_privilege_u = static_cast<unsigned>(p);
29+
}
2330
};
2431

2532
} // namespace machine

0 commit comments

Comments
 (0)