In-order Early Halting Fix

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. */

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. */

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. */

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

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)? */

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)? */

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. */

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();

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};
 }
 

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

src/lib/arch/aarch64/Instruction.cc

@@ -130,7 +130,7 @@ std::tuple<bool, uint64_t> 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

src/lib/arch/aarch64/Instruction_decode.cc

@@ -368,15 +368,15 @@ 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;
         break;
       }
       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}

src/lib/arch/riscv/Instruction.cc

@@ -129,7 +129,7 @@ std::tuple<bool, uint64_t> 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;

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;
   }
 }

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>& 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);
   }

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

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);
     }
 

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

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<uint64_t>(14), 10);
+}
+
+INSTANTIATE_TEST_SUITE_P(RISCV, inorderPipeline,
+                         ::testing::Values(std::make_tuple(INORDER,
+                                                           YAML::Load("{}"))),
+                         paramToString);
+
+}  // namespace

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. */

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));

test/unit/MockInstruction.hh

@@ -26,7 +26,7 @@ class MockInstruction : public Instruction {
   MOCK_CONST_METHOD0(checkEarlyBranchMisprediction,
                      std::tuple<bool, uint64_t>());
   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());