From c7f6e2661c958d996479ae9fe8c8cf2c8a9482f6 Mon Sep 17 00:00:00 2001 From: Korey Sewell Date: Sun, 31 Jan 2010 18:30:59 -0500 Subject: inorder: double delete inst bug Make sure that instructions are dereferenced/deleted twice by marking they are on the remove list --- src/cpu/inorder/cpu.cc | 48 +++++++++++++---- src/cpu/inorder/inorder_dyn_inst.cc | 1 + src/cpu/inorder/inorder_dyn_inst.hh | 10 +++- src/cpu/inorder/pipeline_stage.cc | 1 + src/cpu/inorder/resources/cache_unit.cc | 95 +++++++++++++++++++-------------- 5 files changed, 104 insertions(+), 51 deletions(-) (limited to 'src/cpu/inorder') diff --git a/src/cpu/inorder/cpu.cc b/src/cpu/inorder/cpu.cc index e28af9e7a..7342f9bc5 100644 --- a/src/cpu/inorder/cpu.cc +++ b/src/cpu/inorder/cpu.cc @@ -1190,8 +1190,18 @@ void InOrderCPU::addToRemoveList(DynInstPtr &inst) { removeInstsThisCycle = true; - - removeList.push(inst->getInstListIt()); + if (!inst->isRemoveList()) { + DPRINTF(InOrderCPU, "Pushing instruction [tid:%i] PC %#x " + "[sn:%lli] to remove list\n", + inst->threadNumber, inst->readPC(), inst->seqNum); + inst->setRemoveList(); + removeList.push(inst->getInstListIt()); + } else { + DPRINTF(InOrderCPU, "Ignoring instruction removal for [tid:%i] PC %#x " + "[sn:%lli], already remove list\n", + inst->threadNumber, inst->readPC(), inst->seqNum); + } + } void @@ -1204,11 +1214,18 @@ InOrderCPU::removeInst(DynInstPtr &inst) removeInstsThisCycle = true; // Remove the instruction. + if (!inst->isRemoveList()) { + DPRINTF(InOrderCPU, "Pushing instruction [tid:%i] PC %#x " + "[sn:%lli] to remove list\n", + inst->threadNumber, inst->readPC(), inst->seqNum); + inst->setRemoveList(); + removeList.push(inst->getInstListIt()); + } else { + DPRINTF(InOrderCPU, "Ignoring instruction removal for [tid:%i] PC %#x " + "[sn:%lli], already on remove list\n", + inst->threadNumber, inst->readPC(), inst->seqNum); + } - DPRINTF(RefCount, "Pushing instruction [tid:%i] PC %#x " - "[sn:%lli] to remove list\n", - inst->threadNumber, inst->readPC(), inst->seqNum); - removeList.push(inst->getInstListIt()); } void @@ -1252,11 +1269,22 @@ InOrderCPU::squashInstIt(const ListIt &instIt, ThreadID tid) (*instIt)->setSquashed(); - DPRINTF(RefCount, "Pushing instruction [tid:%i] PC %#x " - "[sn:%lli] to remove list\n", - (*instIt)->threadNumber, (*instIt)->readPC(), (*instIt)->seqNum); - removeList.push(instIt); + if (!(*instIt)->isRemoveList()) { + DPRINTF(InOrderCPU, "Pushing instruction [tid:%i] PC %#x " + "[sn:%lli] to remove list\n", + (*instIt)->threadNumber, (*instIt)->readPC(), + (*instIt)->seqNum); + (*instIt)->setRemoveList(); + removeList.push(instIt); + } else { + DPRINTF(InOrderCPU, "Ignoring instruction removal for [tid:%i] PC %#x " + "[sn:%lli], already on remove list\n", + (*instIt)->threadNumber, (*instIt)->readPC(), + (*instIt)->seqNum); + } + } + } diff --git a/src/cpu/inorder/inorder_dyn_inst.cc b/src/cpu/inorder/inorder_dyn_inst.cc index 75e1c570f..1b55c90e0 100644 --- a/src/cpu/inorder/inorder_dyn_inst.cc +++ b/src/cpu/inorder/inorder_dyn_inst.cc @@ -115,6 +115,7 @@ InOrderDynInst::initVars() split2ndAddr = 0; split2ndAccess = false; splitInst = false; + splitInstSked = false; splitFinishCnt = 0; effAddr = 0; diff --git a/src/cpu/inorder/inorder_dyn_inst.hh b/src/cpu/inorder/inorder_dyn_inst.hh index 8a5f9cf25..8c9cd69e0 100644 --- a/src/cpu/inorder/inorder_dyn_inst.hh +++ b/src/cpu/inorder/inorder_dyn_inst.hh @@ -164,6 +164,7 @@ class InOrderDynInst : public FastAlloc, public RefCounted /// instructions ahead of it SerializeAfter, /// Needs to serialize instructions behind it SerializeHandled, /// Serialization has been handled + RemoveList, /// Is Instruction on Remove List? NumStatus }; @@ -342,7 +343,8 @@ class InOrderDynInst : public FastAlloc, public RefCounted bool splitInst; int splitFinishCnt; uint64_t *split2ndStoreDataPtr; - + bool splitInstSked; + //////////////////////////////////////////////////////////// // // BASE INSTRUCTION INFORMATION. @@ -915,6 +917,12 @@ class InOrderDynInst : public FastAlloc, public RefCounted /** Returns whether or not the entry is on the CPU Reg Dep Map */ bool isRegDepEntry() const { return status[RegDepMapEntry]; } + /** Sets this instruction as entered on the CPU Reg Dep Map */ + void setRemoveList() { status.set(RemoveList); } + + /** Returns whether or not the entry is on the CPU Reg Dep Map */ + bool isRemoveList() const { return status[RemoveList]; } + /** Sets this instruction as completed. */ void setCompleted() { status.set(Completed); } diff --git a/src/cpu/inorder/pipeline_stage.cc b/src/cpu/inorder/pipeline_stage.cc index 571cf10bb..dcf4d81bf 100644 --- a/src/cpu/inorder/pipeline_stage.cc +++ b/src/cpu/inorder/pipeline_stage.cc @@ -380,6 +380,7 @@ PipelineStage::squashPrevStageInsts(InstSeqNum squash_seq_num, ThreadID tid) for (int i=0; i < prevStage->size; i++) { if (prevStage->insts[i]->threadNumber == tid && prevStage->insts[i]->seqNum > squash_seq_num) { + // Change Comment to Annulling previous instruction DPRINTF(InOrderStage, "[tid:%i]: Squashing instruction, " "[sn:%i] PC %08p.\n", tid, diff --git a/src/cpu/inorder/resources/cache_unit.cc b/src/cpu/inorder/resources/cache_unit.cc index 00058163f..cb1861ea9 100644 --- a/src/cpu/inorder/resources/cache_unit.cc +++ b/src/cpu/inorder/resources/cache_unit.cc @@ -140,7 +140,8 @@ CacheUnit::getSlot(DynInstPtr inst) // For a Split-Load, the instruction would have processed once already // causing the address to be unset. if (!inst->validMemAddr() && !inst->splitInst) { - panic("Mem. Addr. must be set before requesting cache access\n"); + panic("[tid:%i][sn:%i] Mem. Addr. must be set before requesting cache access\n", + inst->readTid(), inst->seqNum); } Addr req_addr = inst->getMemAddr(); @@ -439,7 +440,7 @@ CacheUnit::read(DynInstPtr inst, Addr addr, T &data, unsigned flags) cache_req->splitAccess = true; cache_req->split2ndAccess = true; - DPRINTF(InOrderCachePort, "%i: sn[%i] Split Read Access (2 of 2) for (%#x, %#x).\n", curTick, inst->seqNum, + DPRINTF(InOrderCachePort, "[sn:%i] Split Read Access (2 of 2) for (%#x, %#x).\n", inst->seqNum, inst->getMemAddr(), inst->split2ndAddr); } @@ -459,27 +460,31 @@ CacheUnit::read(DynInstPtr inst, Addr addr, T &data, unsigned flags) inst->splitMemData = new uint8_t[dataSize]; inst->splitTotalSize = dataSize; - - // Schedule Split Read/Complete for Instruction - // ============================== - int stage_num = cache_req->getStageNum(); + if (!inst->splitInstSked) { + // Schedule Split Read/Complete for Instruction + // ============================== + int stage_num = cache_req->getStageNum(); - int stage_pri = ThePipeline::getNextPriority(inst, stage_num); + int stage_pri = ThePipeline::getNextPriority(inst, stage_num); - inst->resSched.push(new ScheduleEntry(stage_num, - stage_pri, - cpu->resPool->getResIdx(DCache), - CacheUnit::InitSecondSplitRead, - 1) - ); - - inst->resSched.push(new ScheduleEntry(stage_num + 1, - 1/*stage_pri*/, - cpu->resPool->getResIdx(DCache), - CacheUnit::CompleteSecondSplitRead, - 1) - ); - + inst->resSched.push(new ScheduleEntry(stage_num, + stage_pri, + cpu->resPool->getResIdx(DCache), + CacheUnit::InitSecondSplitRead, + 1) + ); + + inst->resSched.push(new ScheduleEntry(stage_num + 1, + 1/*stage_pri*/, + cpu->resPool->getResIdx(DCache), + CacheUnit::CompleteSecondSplitRead, + 1) + ); + inst->splitInstSked = true; + } else { + DPRINTF(InOrderCachePort, "[tid:%i] [sn:%i] Retrying Split Read Access (1 of 2) for (%#x, %#x).\n", + inst->readTid(), inst->seqNum, addr, secondAddr); + } // Split Information for First Access // ============================== @@ -533,7 +538,7 @@ CacheUnit::write(DynInstPtr inst, T data, Addr addr, unsigned flags, cache_req->splitAccess = true; cache_req->split2ndAccess = true; - DPRINTF(InOrderCachePort, "%i: sn[%i] Split Write Access (2 of 2) for (%#x, %#x).\n", curTick, inst->seqNum, + DPRINTF(InOrderCachePort, "[sn:%i] Split Write Access (2 of 2) for (%#x, %#x).\n", inst->seqNum, inst->getMemAddr(), inst->split2ndAddr); } @@ -542,7 +547,8 @@ CacheUnit::write(DynInstPtr inst, T data, Addr addr, unsigned flags, Addr secondAddr = roundDown(addr + dataSize - 1, blockSize); if (secondAddr > addr && !inst->split2ndAccess) { - DPRINTF(InOrderCachePort, "%i: sn[%i] Split Write Access (1 of 2) for (%#x, %#x).\n", curTick, inst->seqNum, + + DPRINTF(InOrderCachePort, "[sn:%i] Split Write Access (1 of 2) for (%#x, %#x).\n", inst->seqNum, addr, secondAddr); // Save All "Total" Split Information @@ -550,25 +556,33 @@ CacheUnit::write(DynInstPtr inst, T data, Addr addr, unsigned flags, inst->splitInst = true; inst->splitTotalSize = dataSize; - // Schedule Split Read/Complete for Instruction - // ============================== - int stage_num = cache_req->getStageNum(); + if (!inst->splitInstSked) { + // Schedule Split Read/Complete for Instruction + // ============================== + int stage_num = cache_req->getStageNum(); + + int stage_pri = ThePipeline::getNextPriority(inst, stage_num); + + inst->resSched.push(new ScheduleEntry(stage_num, + stage_pri, + cpu->resPool->getResIdx(DCache), + CacheUnit::InitSecondSplitWrite, + 1) + ); + + inst->resSched.push(new ScheduleEntry(stage_num + 1, + 1/*stage_pri*/, + cpu->resPool->getResIdx(DCache), + CacheUnit::CompleteSecondSplitWrite, + 1) + ); + inst->splitInstSked = true; + } else { + DPRINTF(InOrderCachePort, "[tid:%i] sn:%i] Retrying Split Read Access (1 of 2) for (%#x, %#x).\n", + inst->readTid(), inst->seqNum, addr, secondAddr); + } - int stage_pri = ThePipeline::getNextPriority(inst, stage_num); - inst->resSched.push(new ScheduleEntry(stage_num, - stage_pri, - cpu->resPool->getResIdx(DCache), - CacheUnit::InitSecondSplitWrite, - 1) - ); - - inst->resSched.push(new ScheduleEntry(stage_num + 1, - 1/*stage_pri*/, - cpu->resPool->getResIdx(DCache), - CacheUnit::CompleteSecondSplitWrite, - 1) - ); // Split Information for First Access // ============================== @@ -582,6 +596,7 @@ CacheUnit::write(DynInstPtr inst, T data, Addr addr, unsigned flags, inst->split2ndStoreDataPtr = &cache_req->inst->storeData; inst->split2ndStoreDataPtr += dataSize; inst->split2ndFlags = flags; + inst->splitInstSked = true; } doTLBAccess(inst, cache_req, dataSize, flags, TheISA::TLB::Write); -- cgit v1.2.3