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

Multiple bug fixes to run parsec benchmark #189

Merged
merged 5 commits into from
Oct 17, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions src/arch/generic/mmu.hh
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,14 @@ class BaseMMU : public SimObject
translateFunctional(const RequestPtr &req, ThreadContext *tc,
Mode mode);

virtual void setOldPriv(ThreadContext *tc) {
panic("BaseMMU::setOldPriv() not implemented");
}

virtual void useNewPriv(ThreadContext *tc) {
panic("BaseMMU::useNewPriv() not implemented");
}

class MMUTranslationGen : public TranslationGen
{
private:
Expand Down
4 changes: 2 additions & 2 deletions src/arch/riscv/isa/decoder.isa
Original file line number Diff line number Diff line change
Expand Up @@ -953,7 +953,7 @@ decode QUADRANT default Unknown::unknown() {
0x2: decode AMOFUNCT {
0x2: LoadReserved::lr_w({{
Rd_sd = Mem_sw;
}}, mem_flags=LLSC);
}}, inst_flags=IsLoadReserved, mem_flags=LLSC);
0x3: StoreCond::sc_w({{
Mem_uw = Rs2_uw;
}}, {{
Expand Down Expand Up @@ -1026,7 +1026,7 @@ decode QUADRANT default Unknown::unknown() {
0x3: decode AMOFUNCT {
0x2: LoadReserved::lr_d({{
Rd_sd = Mem_sd;
}}, mem_flags=LLSC);
}}, mem_flags=LLSC, inst_flags=IsLoadReserved);
0x3: StoreCond::sc_d({{
Mem = Rs2;
}}, {{
Expand Down
12 changes: 12 additions & 0 deletions src/arch/riscv/mmu.hh
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,18 @@ class MMU : public BaseMMU
{
return static_cast<TLB*>(dtb)->pmp;
}

void
setOldPriv(ThreadContext *tc) override {
static_cast<TLB*>(dtb)->setOldPriv(tc);
static_cast<TLB*>(itb)->setOldPriv(tc);
}

void
useNewPriv(ThreadContext *tc) override {
static_cast<TLB*>(dtb)->useNewPriv(tc);
static_cast<TLB*>(itb)->useNewPriv(tc);
}
};

} // namespace RiscvISA
Expand Down
7 changes: 7 additions & 0 deletions src/arch/riscv/tlb.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1961,6 +1961,13 @@ TLB::doTranslate(const RequestPtr &req, ThreadContext *tc,
PrivilegeMode
TLB::getMemPriv(ThreadContext *tc, BaseMMU::Mode mode)
{
if (use_old_priv && mode != BaseMMU::Execute) {
if (mode == BaseMMU::Execute) {
return old_priv_ex;
} else {
return old_priv_ldst;
}
}
STATUS status = (STATUS)tc->readMiscReg(MISCREG_STATUS);
PrivilegeMode pmode = (PrivilegeMode)tc->readMiscReg(MISCREG_PRV);
if (mode != BaseMMU::Execute && status.mprv == 1)
Expand Down
11 changes: 11 additions & 0 deletions src/arch/riscv/tlb.hh
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,9 @@ class TLB : public BaseTLB
uint64_t lastPc;
uint64_t traceFlag;

bool use_old_priv;
PrivilegeMode old_priv_ldst;
PrivilegeMode old_priv_ex;

Walker *walker;

Expand Down Expand Up @@ -251,6 +254,14 @@ class TLB : public BaseTLB
TlbEntry *lookupL2TLB(Addr vpn, uint16_t asid, BaseMMU::Mode mode, bool hidden, int f_level, bool sign_used,
uint8_t translateMode);

void setOldPriv(ThreadContext *tc) {
use_old_priv = true;
old_priv_ex = getMemPriv(tc, BaseMMU::Execute);
old_priv_ldst = getMemPriv(tc, BaseMMU::Read);
}
void useNewPriv(ThreadContext *tc) {
use_old_priv = false;
}


std::vector<TlbEntry> tlbL2L1; // our TLB
Expand Down
1 change: 1 addition & 0 deletions src/cpu/StaticInstFlags.py
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ class StaticInstFlags(Enum):
'IsLoad', # Reads from memory (load or prefetch).
'IsStore', # Writes to memory.
'IsAtomic', # Does atomic RMW to memory.
'IsLoadReserved', # Load reserved
'IsStoreConditional', # Store conditional instruction.
'IsInstPrefetch', # Instruction-cache prefetch.
'IsDataPrefetch', # Data-cache prefetch.
Expand Down
49 changes: 48 additions & 1 deletion src/cpu/base.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1064,7 +1064,38 @@ BaseCPU::diffWithNEMU(ThreadID tid, InstSeqNum seq)
"[0m, GEM5 value: \033[31m%#lx\033[0m\n", "stval",
ref_val, gem5_val);
}

// mtval
gem5_val = readMiscRegNoEffect(
RiscvISA::MiscRegIndex::MISCREG_MTVAL, tid);
ref_val = diffAllStates->referenceRegFile.mtval;
DPRINTF(Diff, "stvmtvalal:\tGEM5: %#lx,\tREF: %#lx\n", gem5_val, ref_val);
if (gem5_val != ref_val) {
diffMsg += csprintf("Diff at \033[31m%s\033[0m Ref value: \033[31m%#lx\033"
"[0m, GEM5 value: \033[31m%#lx\033[0m\n", "mtval",
ref_val, gem5_val);
diffInfo.errorCsrsValue[CsrRegIndex::mtval] = 1;
diffAllStates->gem5RegFile.mtval = gem5_val;
if (!diff_at)
diff_at = ValueDiff;
}
//DIFFTEST_STVDIFFTEST_STVAL

// mode
gem5_val = readMiscRegNoEffect(
RiscvISA::MiscRegIndex::MISCREG_PRV, tid);
ref_val = diffAllStates->referenceRegFile.mode;
DPRINTF(Diff, "priv:\tGEM5: %#lx,\tREF: %#lx\n", gem5_val, ref_val);
if (gem5_val != ref_val) {
diffMsg += csprintf("Diff at \033[31m%s\033[0m Ref value: \033[31m%#lx\033"
"[0m, GEM5 value: \033[31m%#lx\033[0m\n", "priv",
ref_val, gem5_val);
// diffInfo.errorCsrsValue[CsrRegIndex::stval] = 1;
diffAllStates->gem5RegFile.mode = gem5_val;
if (!diff_at)
diff_at = ValueDiff;
}

// mcause
gem5_val = readMiscRegNoEffect(
RiscvISA::MiscRegIndex::MISCREG_MCAUSE, tid);
Expand All @@ -1076,6 +1107,21 @@ BaseCPU::diffWithNEMU(ThreadID tid, InstSeqNum seq)
csprintf("Diff at \033[31m%s\033[0m Ref value: \033[31m%#lx\033[0m, GEM5 value: \033[31m%#lx\033[0m\n",
"mcause", ref_val, gem5_val);
}

// scause
gem5_val = readMiscRegNoEffect(
RiscvISA::MiscRegIndex::MISCREG_SCAUSE, tid);
ref_val = diffAllStates->referenceRegFile.scause;
DPRINTF(Diff, "scause:\tGEM5: %#lx,\tREF: %#lx\n", gem5_val, ref_val);
if (gem5_val != ref_val) {
diffMsg +=
csprintf("Diff at \033[31m%s\033[0m Ref value: \033[31m%#lx\033[0m, GEM5 value: \033[31m%#lx\033[0m\n",
"scause", ref_val, gem5_val);
diffInfo.errorCsrsValue[CsrRegIndex::scause] = 1;
diffAllStates->gem5RegFile.scause = gem5_val;
if (!diff_at)
diff_at = ValueDiff;
}
// satp
gem5_val =
readMiscRegNoEffect(RiscvISA::MiscRegIndex::MISCREG_SATP, tid);
Expand Down Expand Up @@ -1389,12 +1435,13 @@ BaseCPU::difftestStep(ThreadID tid, InstSeqNum seq)

bool is_fence = diffInfo.inst->isReadBarrier() || diffInfo.inst->isWriteBarrier();
bool fence_should_diff = is_fence && !diffInfo.inst->isMicroop();
bool lr_should_diff = diffInfo.inst->isLoadReserved();
bool amo_should_diff = diffInfo.inst->isAtomic() && diffInfo.inst->numDestRegs() > 0;
bool is_sc = diffInfo.inst->isStoreConditional() && diffInfo.inst->isDelayedCommit();
bool other_should_diff = !diffInfo.inst->isAtomic() && !is_fence && !is_sc &&
(!diffInfo.inst->isMicroop() || diffInfo.inst->isLastMicroop());

if (fence_should_diff || amo_should_diff || is_sc || other_should_diff) {
if (fence_should_diff || amo_should_diff || is_sc || other_should_diff || lr_should_diff) {
should_diff = true;
if (!diffAllStates->hasCommit && diffInfo.pc->instAddr() == 0x80000000u) {
diffAllStates->hasCommit = true;
Expand Down
7 changes: 7 additions & 0 deletions src/cpu/o3/commit.cc
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,7 @@ Commit::Commit(CPU *_cpu, branch_prediction::BPredUnit *_bp, const BaseO3CPUPara
faultNum.insert(RiscvISA::ExceptionCode::LOAD_PAGE);
faultNum.insert(RiscvISA::ExceptionCode::STORE_PAGE);
faultNum.insert(RiscvISA::ExceptionCode::INST_PAGE);
faultNum.insert(RiscvISA::ExceptionCode::AMO_PAGE);
faultNum.insert(RiscvISA::ExceptionCode::INST_G_PAGE);
faultNum.insert(RiscvISA::ExceptionCode::LOAD_G_PAGE);
faultNum.insert(RiscvISA::ExceptionCode::STORE_G_PAGE);
Expand Down Expand Up @@ -641,6 +642,8 @@ Commit::squashAll(ThreadID tid)
toIEW->commitInfo[tid].squashedTargetId = committedTargetId;
toIEW->commitInfo[tid].squashedLoopIter = committedLoopIter;

cpu->mmu->useNewPriv(cpu->getContext(tid));

squashInflightAndUpdateVersion(tid);
}

Expand Down Expand Up @@ -840,6 +843,8 @@ Commit::handleInterrupt()
DPRINTF(CommitTrace, "Handle interrupt No.%lx\n", cpu->getInterruptsNO() | (1ULL << 63));
cpu->processInterrupts(cpu->getInterrupts());

cpu->mmu->setOldPriv(cpu->getContext(0));

thread[0]->noSquashFromTC = false;

commitStatus[0] = TrapPending;
Expand Down Expand Up @@ -1527,6 +1532,8 @@ Commit::commitHead(const DynInstPtr &head_inst, unsigned inst_num)
head_inst->notAnInst() ? nullStaticInstPtr :
head_inst->staticInst);

cpu->mmu->setOldPriv(cpu->getContext(tid));

// Exit state update mode to avoid accidental updating.
thread[tid]->noSquashFromTC = false;

Expand Down
2 changes: 1 addition & 1 deletion src/cpu/o3/fetch.cc
Original file line number Diff line number Diff line change
Expand Up @@ -835,7 +835,7 @@ Fetch::finishTranslation(const Fault &fault, const RequestPtr &mem_req)
// If we have, just wait around for commit to squash something and put
// us on the right track
if (!cpu->system->isMemAddr(mem_req->getPaddr())) {
warn("Address %#x is outside of physical memory, stopping fetch, %lu\n",
DPRINTF(Fetch, "Address %#x is outside of physical memory, stopping fetch, %lu\n",
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use "warn" can help us find the fetch stuck easier

Copy link
Contributor

@shinezyy shinezyy Oct 16, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use "warn" can help us find the fetch stuck easier

But if on the speculative path, might it be too verbose?

@notlqr Did you observe a too verbose print here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some benchmarks produce too much warns here

mem_req->getPaddr(), curTick());
fetchStatus[tid] = NoGoodAddr;
setAllFetchStalls(StallReason::OtherFetchStall);
Expand Down
16 changes: 12 additions & 4 deletions src/cpu/o3/issue_queue.cc
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,7 @@ IssueQue::resetDepGraph(int numPhysRegs)
subDepGraph.resize(numPhysRegs);
}

void
bool
IssueQue::checkScoreboard(const DynInstPtr& inst)
{
for (int i = 0; i < inst->numSrcRegs(); i++) {
Expand All @@ -182,11 +182,17 @@ IssueQue::checkScoreboard(const DynInstPtr& inst)
// check bypass data ready or not
if (!scheduler->bypassScoreboard[src->flatIndex()]) [[unlikely]] {
auto dst_inst = scheduler->getInstByDstReg(src->flatIndex());
panic("[sn %lu] %s can't get data from bypassNetwork, dst inst: %s\n", inst->seqNum, inst->srcRegIdx(i),
dst_inst->genDisassembly());
if (!dst_inst || !dst_inst->isLoad()) {
panic("dst is not load");
}
scheduler->loadCancel(dst_inst);
DPRINTF(Schedule, "[sn %lu] %s can't get data from bypassNetwork, dst inst: %s\n",
inst->seqNum, inst->srcRegIdx(i), dst_inst->genDisassembly());
return false;
}
}
inst->checkOldVdElim();
return true;
}

void
Expand Down Expand Up @@ -217,7 +223,9 @@ IssueQue::issueToFu()
readyQclassify[inst->opClass()]->push(inst); // retry
continue;
}
checkScoreboard(inst);
if (!checkScoreboard(inst)) {
continue;
}
addToFu(inst);
if (!opPipelined[inst->opClass()]) [[unlikely]] {
// set fu busy
Expand Down
2 changes: 1 addition & 1 deletion src/cpu/o3/issue_queue.hh
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ class IssueQue : public SimObject

void replay(const DynInstPtr& inst);
void addToFu(const DynInstPtr& inst);
void checkScoreboard(const DynInstPtr& inst);
bool checkScoreboard(const DynInstPtr& inst);
void issueToFu();
void wakeUpDependents(const DynInstPtr& inst, bool speculative);
void selectInst();
Expand Down
8 changes: 8 additions & 0 deletions src/cpu/o3/lsq.cc
Original file line number Diff line number Diff line change
Expand Up @@ -909,6 +909,14 @@ LSQ::pushRequest(const DynInstPtr& inst, bool isLoad, uint8_t *data,
request->initiateTranslation();
}

if (!isLoad && !inst->isVector() && size > 1 && addr % size != 0) {
warn( "Store misaligned: size: %u, Addr: %#lx, code: %d\n", size,
addr, RiscvISA::ExceptionCode::STORE_ADDR_MISALIGNED);
return std::make_shared<RiscvISA::AddressFault>(request->mainReq()->getVaddr(),
request->mainReq()->getgPaddr(),
RiscvISA::ExceptionCode::STORE_ADDR_MISALIGNED);
}

/* This is the place were instructions get the effAddr. */
if (request->isTranslationComplete()) {
if (request->isMemAccessRequired()) {
Expand Down
1 change: 1 addition & 0 deletions src/cpu/static_inst.hh
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,7 @@ class StaticInst : public RefCounted, public StaticInstFlags
bool isLoad() const { return flags[IsLoad]; }
bool isStore() const { return flags[IsStore]; }
bool isAtomic() const { return flags[IsAtomic]; }
bool isLoadReserved() const { return flags[IsLoadReserved]; }
bool isStoreConditional() const { return flags[IsStoreConditional]; }
bool isInstPrefetch() const { return flags[IsInstPrefetch]; }
bool isDataPrefetch() const { return flags[IsDataPrefetch]; }
Expand Down
Loading