diff --git a/src/include/simeng/AlwaysNotTakenPredictor.hh b/src/include/simeng/AlwaysNotTakenPredictor.hh index 3c1bef7614..7ec8027d4b 100644 --- a/src/include/simeng/AlwaysNotTakenPredictor.hh +++ b/src/include/simeng/AlwaysNotTakenPredictor.hh @@ -11,7 +11,7 @@ class AlwaysNotTakenPredictor : public BranchPredictor { /** Generate a branch prediction for the specified instruction address; will * always predict not taken. */ BranchPrediction predict(uint64_t address, BranchType type, - uint64_t knownTarget) override; + int64_t knownOffset) override; /** Provide branch results to update the prediction model for the specified * instruction address. As this model is static, this does nothing. */ diff --git a/src/include/simeng/BranchPredictor.hh b/src/include/simeng/BranchPredictor.hh index 88be07dd3f..8e2ddd0797 100644 --- a/src/include/simeng/BranchPredictor.hh +++ b/src/include/simeng/BranchPredictor.hh @@ -47,9 +47,9 @@ class BranchPredictor { virtual ~BranchPredictor(){}; /** Generate a branch prediction for the specified instruction address with a - * branch type and possible known target. */ + * branch type and possible known branch offset. */ virtual BranchPrediction predict(uint64_t address, BranchType type, - uint64_t knownTarget) = 0; + int64_t knownOffset) = 0; /** Provide branch results to update the prediction model for the specified * instruction address. */ diff --git a/src/include/simeng/GenericPredictor.hh b/src/include/simeng/GenericPredictor.hh index 21df57a4a5..cba924125c 100644 --- a/src/include/simeng/GenericPredictor.hh +++ b/src/include/simeng/GenericPredictor.hh @@ -27,10 +27,10 @@ class GenericPredictor : public BranchPredictor { ~GenericPredictor(); /** Generate a branch prediction for the supplied instruction address, a - * branch type, and a known target if not 0. Returns a branch direction and - * branch target address. */ + * branch type, and a known branch offset; defaults to 0 meaning offset is not + * known. Returns a branch direction and branch target address. */ BranchPrediction predict(uint64_t address, BranchType type, - uint64_t knownTarget) override; + int64_t knownOffset = 0) override; /** Updates appropriate predictor model objects based on the address and * outcome of the branch instruction. */ diff --git a/src/include/simeng/Instruction.hh b/src/include/simeng/Instruction.hh index 8b1cf2f9db..5d684e10eb 100644 --- a/src/include/simeng/Instruction.hh +++ b/src/include/simeng/Instruction.hh @@ -99,8 +99,8 @@ class Instruction { /** Retrieve branch type. */ virtual BranchType getBranchType() const = 0; - /** Retrieve a branch target from the instruction's metadata if known. */ - virtual uint64_t getKnownTarget() const = 0; + /** Retrieve a branch offset from the instruction's metadata if known. */ + virtual int64_t getKnownOffset() const = 0; /** Is this a store address operation (a subcategory of store operations which * deal with the generation of store addresses to store data at)? */ @@ -208,8 +208,9 @@ class Instruction { /** What type of branch this instruction is. */ BranchType branchType_ = BranchType::Unknown; - /** If the branch target is known at the time of decode, store it. */ - uint64_t knownTarget_ = 0; + /** The branch offset that may be known at the time of instruction decoding. + * The default value of 0 represents an unknown branch offset.*/ + int64_t knownOffset_ = 0; // Flushing /** This instruction's sequence ID; a higher ID represents a chronologically diff --git a/src/include/simeng/arch/aarch64/Instruction.hh b/src/include/simeng/arch/aarch64/Instruction.hh index a4c28950a9..d4ae80aa16 100644 --- a/src/include/simeng/arch/aarch64/Instruction.hh +++ b/src/include/simeng/arch/aarch64/Instruction.hh @@ -297,8 +297,8 @@ class Instruction : public simeng::Instruction { /** Retrieve branch type. */ BranchType getBranchType() const override; - /** Retrieve a branch target from the instruction's metadata if known. */ - uint64_t getKnownTarget() const override; + /** Retrieve a branch offset from the instruction's metadata if known. */ + int64_t getKnownOffset() const override; /** Is this a store address operation (a subcategory of store operations which * deal with the generation of store addresses to store data at)? */ diff --git a/src/include/simeng/arch/riscv/Instruction.hh b/src/include/simeng/arch/riscv/Instruction.hh index 61b83037ca..8ee390f94c 100644 --- a/src/include/simeng/arch/riscv/Instruction.hh +++ b/src/include/simeng/arch/riscv/Instruction.hh @@ -125,8 +125,8 @@ class Instruction : public simeng::Instruction { /** Retrieve branch type. */ BranchType getBranchType() const override; - /** Retrieve a branch target from the instruction's metadata if known. */ - uint64_t getKnownTarget() const override; + /** Retrieve a branch offset from the instruction's metadata if known. */ + int64_t getKnownOffset() const override; /** Is this a store address operation (a subcategory of store operations which * deal with the generation of store addresses to store data at)? */ diff --git a/src/include/simeng/pipeline/ExecuteUnit.hh b/src/include/simeng/pipeline/ExecuteUnit.hh index e95391a8f2..8a480fac10 100644 --- a/src/include/simeng/pipeline/ExecuteUnit.hh +++ b/src/include/simeng/pipeline/ExecuteUnit.hh @@ -65,6 +65,10 @@ class ExecuteUnit { /** Retrieve the number of active execution cycles. */ uint64_t getCycles() const; + /** Query whether the execution unit is empty and not currently processing any + * instructions. */ + bool isEmpty() const; + private: /** Execute the supplied uop, write it into the output buffer, and forward * results back to dispatch/issue. */ diff --git a/src/include/simeng/pipeline/PipelineBuffer.hh b/src/include/simeng/pipeline/PipelineBuffer.hh index b827d6f037..3d1e0df1a8 100644 --- a/src/include/simeng/pipeline/PipelineBuffer.hh +++ b/src/include/simeng/pipeline/PipelineBuffer.hh @@ -29,6 +29,16 @@ class PipelineBuffer { headIsStart = !headIsStart; } + /** Return the slots waiting to be processed by the next pipeline unit */ + const T* getPendingSlots() const { + // If stalled head and tail slots won't have been swapped + if (isStalled_) { + return getTailSlots(); + } else { + return getHeadSlots(); + } + } + /** Get a tail slots pointer. */ T* getTailSlots() { T* ptr = buffer.data(); diff --git a/src/lib/AlwaysNotTakenPredictor.cc b/src/lib/AlwaysNotTakenPredictor.cc index 784c9f5cc9..9ad8d1e2e4 100644 --- a/src/lib/AlwaysNotTakenPredictor.cc +++ b/src/lib/AlwaysNotTakenPredictor.cc @@ -4,7 +4,7 @@ namespace simeng { BranchPrediction AlwaysNotTakenPredictor::predict(uint64_t address, BranchType type, - uint64_t knownTarget) { + int64_t knownOffset) { return {false, 0}; } diff --git a/src/lib/GenericPredictor.cc b/src/lib/GenericPredictor.cc index 2539d7ae59..d9188a9e4a 100644 --- a/src/lib/GenericPredictor.cc +++ b/src/lib/GenericPredictor.cc @@ -26,7 +26,7 @@ GenericPredictor::~GenericPredictor() { } BranchPrediction GenericPredictor::predict(uint64_t address, BranchType type, - uint64_t knownTarget) { + int64_t knownOffset) { // Get index via an XOR hash between the global history and the lower btbBits_ // bits of the instruction address uint64_t hashedIndex = (address & ((1 << btbBits_) - 1)) ^ globalHistory_; @@ -36,7 +36,7 @@ BranchPrediction GenericPredictor::predict(uint64_t address, BranchType type, bool direction = btb_[hashedIndex].first < (1 << (satCntBits_ - 1)) ? false : true; uint64_t target = - (knownTarget != 0) ? address + knownTarget : btb_[hashedIndex].second; + (knownOffset != 0) ? address + knownOffset : btb_[hashedIndex].second; BranchPrediction prediction = {direction, target}; // Ammend prediction based on branch type diff --git a/src/lib/arch/aarch64/Instruction.cc b/src/lib/arch/aarch64/Instruction.cc index b6884443c0..676f705b40 100644 --- a/src/lib/arch/aarch64/Instruction.cc +++ b/src/lib/arch/aarch64/Instruction.cc @@ -130,7 +130,7 @@ std::tuple Instruction::checkEarlyBranchMisprediction() const { BranchType Instruction::getBranchType() const { return branchType_; } -uint64_t Instruction::getKnownTarget() const { return knownTarget_; } +int64_t Instruction::getKnownOffset() const { return knownOffset_; } uint16_t Instruction::getGroup() const { // Use identifiers to decide instruction group diff --git a/src/lib/arch/aarch64/Instruction_decode.cc b/src/lib/arch/aarch64/Instruction_decode.cc index 4237475f52..3c0a4dfee3 100644 --- a/src/lib/arch/aarch64/Instruction_decode.cc +++ b/src/lib/arch/aarch64/Instruction_decode.cc @@ -368,7 +368,7 @@ void Instruction::decode() { switch (metadata.opcode) { case Opcode::AArch64_B: // b label branchType_ = BranchType::Unconditional; - knownTarget_ = metadata.operands[0].imm; + knownOffset_ = metadata.operands[0].imm; break; case Opcode::AArch64_BR: { // br xn branchType_ = BranchType::Unconditional; @@ -376,7 +376,7 @@ void Instruction::decode() { } case Opcode::AArch64_BL: // bl #imm branchType_ = BranchType::SubroutineCall; - knownTarget_ = metadata.operands[0].imm; + knownOffset_ = metadata.operands[0].imm; break; case Opcode::AArch64_BLR: { // blr xn branchType_ = BranchType::SubroutineCall; @@ -387,7 +387,7 @@ void Instruction::decode() { branchType_ = BranchType::LoopClosing; else branchType_ = BranchType::Conditional; - knownTarget_ = metadata.operands[0].imm; + knownOffset_ = metadata.operands[0].imm; break; } case Opcode::AArch64_CBNZW: // cbnz wn, #imm @@ -401,7 +401,7 @@ void Instruction::decode() { branchType_ = BranchType::LoopClosing; else branchType_ = BranchType::Conditional; - knownTarget_ = metadata.operands[1].imm; + knownOffset_ = metadata.operands[1].imm; break; } case Opcode::AArch64_TBNZW: // tbnz wn, #imm, label @@ -415,7 +415,7 @@ void Instruction::decode() { branchType_ = BranchType::LoopClosing; else branchType_ = BranchType::Conditional; - knownTarget_ = metadata.operands[2].imm; + knownOffset_ = metadata.operands[2].imm; break; } case Opcode::AArch64_RET: { // ret {xr} diff --git a/src/lib/arch/riscv/Instruction.cc b/src/lib/arch/riscv/Instruction.cc index 530890e9a6..6f75ecb356 100644 --- a/src/lib/arch/riscv/Instruction.cc +++ b/src/lib/arch/riscv/Instruction.cc @@ -129,7 +129,7 @@ std::tuple Instruction::checkEarlyBranchMisprediction() const { BranchType Instruction::getBranchType() const { return branchType_; } -uint64_t Instruction::getKnownTarget() const { return knownTarget_; } +int64_t Instruction::getKnownOffset() const { return knownOffset_; } uint16_t Instruction::getGroup() const { uint16_t base = InstructionGroups::INT; diff --git a/src/lib/arch/riscv/Instruction_decode.cc b/src/lib/arch/riscv/Instruction_decode.cc index 6db263796b..a058a2bfe9 100644 --- a/src/lib/arch/riscv/Instruction_decode.cc +++ b/src/lib/arch/riscv/Instruction_decode.cc @@ -239,6 +239,20 @@ void Instruction::decode() { isCompare_ = true; } + if ((Opcode::RISCV_MUL <= metadata.opcode && + metadata.opcode <= Opcode::RISCV_MULW)) { + // Multiply instructions + isMultiply_ = true; + } + + if (((Opcode::RISCV_REM <= metadata.opcode && + metadata.opcode <= Opcode::RISCV_REMW) || + (Opcode::RISCV_DIV <= metadata.opcode && + metadata.opcode <= Opcode::RISCV_DIVW))) { + // Divide instructions + isDivide_ = true; + } + // Set branch type switch (metadata.opcode) { case Opcode::RISCV_BEQ: @@ -248,12 +262,12 @@ void Instruction::decode() { case Opcode::RISCV_BGE: case Opcode::RISCV_BGEU: branchType_ = BranchType::Conditional; - knownTarget_ = instructionAddress_ + metadata.operands[2].imm; + knownOffset_ = metadata.operands[2].imm; break; case Opcode::RISCV_JAL: case Opcode::RISCV_JALR: branchType_ = BranchType::Unconditional; - knownTarget_ = instructionAddress_ + metadata.operands[1].imm; + knownOffset_ = metadata.operands[1].imm; break; } } diff --git a/src/lib/models/inorder/Core.cc b/src/lib/models/inorder/Core.cc index a9604bc085..e57fda264f 100644 --- a/src/lib/models/inorder/Core.cc +++ b/src/lib/models/inorder/Core.cc @@ -71,9 +71,8 @@ void Core::tick() { // as these will now loop around and become the tail. fetchToDecodeBuffer_.tick(); decodeToExecuteBuffer_.tick(); - for (auto& buffer : completionSlots_) { - buffer.tick(); - } + // Only ever 1 completion slot + completionSlots_[0].tick(); if (exceptionGenerated_) { handleException(); @@ -116,14 +115,24 @@ bool Core::hasHalted() const { } // Core is considered to have halted when the fetch unit has halted, there - // are no uops at the head of any buffer, and no exception is currently being - // handled. - bool decodePending = fetchToDecodeBuffer_.getHeadSlots()[0].size() > 0; - bool executePending = decodeToExecuteBuffer_.getHeadSlots()[0] != nullptr; - bool writebackPending = completionSlots_[0].getHeadSlots()[0] != nullptr; + // are no uops pending in any buffer, the execute unit is not currently + // processing an instructions and no exception is currently being handled. + + // Units place uops in buffer tail, but if buffer is stalled, uops do not + // move to head slots and so remain in tail slots. Buffer head appears + // empty. Check emptiness accordingly + auto decSlots = fetchToDecodeBuffer_.getPendingSlots()[0]; + bool decodePending = decSlots.size() > 0; + + auto exeSlots = decodeToExecuteBuffer_.getPendingSlots()[0]; + bool executePending = exeSlots != nullptr; + + auto writeSlots = completionSlots_[0].getPendingSlots()[0]; + bool writebackPending = writeSlots != nullptr; return (fetchUnit_.hasHalted() && !decodePending && !writebackPending && - !executePending && exceptionHandler_ == nullptr); + !executePending && executeUnit_.isEmpty() && + exceptionHandler_ == nullptr); } const ArchitecturalRegisterFileSet& Core::getArchitecturalRegisterFileSet() @@ -220,8 +229,8 @@ void Core::loadData(const std::shared_ptr& instruction) { dataMemory_.requestRead(target); } - // NOTE: This model only supports zero-cycle data memory models, and will not - // work unless data requests are handled synchronously. + // NOTE: This model only supports zero-cycle data memory models, and will + // not work unless data requests are handled synchronously. for (const auto& response : dataMemory_.getCompletedReads()) { instruction->supplyData(response.target.address, response.data); } diff --git a/src/lib/pipeline/ExecuteUnit.cc b/src/lib/pipeline/ExecuteUnit.cc index 83a71c5b1a..7d789b34bd 100644 --- a/src/lib/pipeline/ExecuteUnit.cc +++ b/src/lib/pipeline/ExecuteUnit.cc @@ -224,5 +224,14 @@ uint64_t ExecuteUnit::getBranchMispredictedCount() const { uint64_t ExecuteUnit::getCycles() const { return cycles_; } +bool ExecuteUnit::isEmpty() const { + // Execution unit is considered empty if no instructions are present in the + // pipeline_ and operationsStalled_ queues + if (pipeline_.size() != 0 || operationsStalled_.size() != 0) { + return false; + } + return true; +} + } // namespace pipeline } // namespace simeng diff --git a/src/lib/pipeline/FetchUnit.cc b/src/lib/pipeline/FetchUnit.cc index ade3d307c0..28d2eaba51 100644 --- a/src/lib/pipeline/FetchUnit.cc +++ b/src/lib/pipeline/FetchUnit.cc @@ -129,7 +129,7 @@ void FetchUnit::tick() { BranchPrediction prediction = {false, 0}; if (macroOp[0]->isBranch()) { prediction = branchPredictor_.predict(pc_, macroOp[0]->getBranchType(), - macroOp[0]->getKnownTarget()); + macroOp[0]->getKnownOffset()); macroOp[0]->setBranchPrediction(prediction); } diff --git a/test/regression/riscv/CMakeLists.txt b/test/regression/riscv/CMakeLists.txt index 1821d7ab5f..24050bfa48 100644 --- a/test/regression/riscv/CMakeLists.txt +++ b/test/regression/riscv/CMakeLists.txt @@ -2,6 +2,7 @@ add_executable(regression-riscv RISCVRegressionTest.cc RISCVRegressionTest.hh LoadStoreQueue.cc + InorderPipeline.cc SmokeTest.cc Syscall.cc instructions/arithmetic.cc diff --git a/test/regression/riscv/InorderPipeline.cc b/test/regression/riscv/InorderPipeline.cc new file mode 100644 index 0000000000..f3c8c8fb00 --- /dev/null +++ b/test/regression/riscv/InorderPipeline.cc @@ -0,0 +1,38 @@ +#include "RISCVRegressionTest.hh" + +namespace { + +using inorderPipeline = RISCVRegressionTest; + +TEST_P(inorderPipeline, prematureMulticycleHalting) { + RUN_RISCV(R"( + li a1, 2 + li a2, 1 + li a4, 5 + + beq a1, a2, end # mispredict with target out of programByteLength creates + # pipeline bubble + div a3, a1, a2 # multicycle instruction ties up execution unit causing + # decode to halt and next instruction to be stuck in the + # tail of pipelined buffer + beq a1, a2, end # mispredict with target out of programByteLength halts + # fetch unit this only occurs because the instruction + # address is set after decode therefor a garbage value is + # used sometimes causing the halt at ~FetchUnit.cc::177 + + # This sequence of instructions with this inorder pipeline causes all + # buffers to appear empty and the fetch unit to halt causing the inorder + # core to halt early which is incorrect behaviour. This is fixed in PR 294 + + li a4, 10 # Occurs if core does not halt + end: + )"); + EXPECT_EQ(getGeneralRegister(14), 10); +} + +INSTANTIATE_TEST_SUITE_P(RISCV, inorderPipeline, + ::testing::Values(std::make_tuple(INORDER, + YAML::Load("{}"))), + paramToString); + +} // namespace diff --git a/test/regression/riscv/RISCVRegressionTest.hh b/test/regression/riscv/RISCVRegressionTest.hh index 29ce769783..2dca8b073e 100644 --- a/test/regression/riscv/RISCVRegressionTest.hh +++ b/test/regression/riscv/RISCVRegressionTest.hh @@ -13,14 +13,15 @@ "FrontEnd: 4, LSQ-Completion: 2}, Queue-Sizes: {ROB: 180, Load: 64, " \ "Store: 36}, Branch-Predictor: {BTB-Tag-Bits: 11, Saturating-Count-Bits: " \ "2, Global-History-Length: 10, RAS-entries: 5, Fallback-Static-Predictor: " \ - "0}, L1-Data-Memory: {Interface-Type: Fixed}, L1-Instruction-Memory: " \ + "2}, L1-Data-Memory: {Interface-Type: Fixed}, L1-Instruction-Memory: " \ "{Interface-Type: Flat}, LSQ-L1-Interface: {Access-Latency: 4, Exclusive: " \ "False, Load-Bandwidth: 32, Store-Bandwidth: 16, " \ "Permitted-Requests-Per-Cycle: 2, Permitted-Loads-Per-Cycle: 2, " \ "Permitted-Stores-Per-Cycle: 1}, Ports: {'0': {Portname: Port 0, " \ "Instruction-Group-Support: [0, 10, 11, 12 ]}}, Reservation-Stations: " \ "{'0': {Size: 60, Dispatch-Rate: 4, Ports: [0]}}, Execution-Units: " \ - "{'0': {Pipelined: true}}}") + "{'0': {Pipelined: true}}, Latencies: {'0': {Instruction-Group: {0: '7'}, " \ + "Execution-Latency: 39, Execution-Throughput: 39}}}") /** A helper function to convert the supplied parameters of * INSTANTIATE_TEST_SUITE_P into test name. */ diff --git a/test/unit/MockBranchPredictor.hh b/test/unit/MockBranchPredictor.hh index ac0d42966b..05868a6fed 100644 --- a/test/unit/MockBranchPredictor.hh +++ b/test/unit/MockBranchPredictor.hh @@ -9,7 +9,7 @@ namespace simeng { class MockBranchPredictor : public BranchPredictor { public: MOCK_METHOD3(predict, BranchPrediction(uint64_t address, BranchType type, - uint64_t knownTarget)); + int64_t knownTarget)); MOCK_METHOD4(update, void(uint64_t address, bool taken, uint64_t targetAddress, BranchType type)); MOCK_METHOD1(flush, void(uint64_t address)); diff --git a/test/unit/MockInstruction.hh b/test/unit/MockInstruction.hh index 5574b06c78..f4e17b64ed 100644 --- a/test/unit/MockInstruction.hh +++ b/test/unit/MockInstruction.hh @@ -26,7 +26,7 @@ class MockInstruction : public Instruction { MOCK_CONST_METHOD0(checkEarlyBranchMisprediction, std::tuple()); MOCK_CONST_METHOD0(getBranchType, BranchType()); - MOCK_CONST_METHOD0(getKnownTarget, uint64_t()); + MOCK_CONST_METHOD0(getKnownOffset, int64_t()); MOCK_CONST_METHOD0(isStoreAddress, bool()); MOCK_CONST_METHOD0(isStoreData, bool());