fix: C&S operand mismatch (#53)

Found the issue in the internal dependency system for one of the operands of CAS (3rd operand). It was setting the internal dep arg to 0 which made one operand mark another one as ready (the first one that was waiting on read register finishing in an extra cycle). Just forcing it to be the correct arg after the fact fixed the issue, investigating more on why arg is being set wrong Co-authored-by: Bryan Perdrizat <[email protected]>
parsa-epfl · Oct 1, 2024 · 0977ab3 · 0977ab3
1 parent 3b87a26
commit 0977ab3
Show file tree

Hide file tree

Showing 8 changed files with 38 additions and 13 deletions.
diff --git a/components/Decoder/Effects.cpp b/components/Decoder/Effects.cpp
@@ -65,7 +65,7 @@ void
 DependanceTarget::invokeSatisfy(int32_t anArg)
 {
     void (nDecoder::DependanceTarget::*satisfy_pt)(int32_t) = &nDecoder::DependanceTarget::satisfy;
-    DBG_(VVerb, (<< std::hex << "Satisfy: " << satisfy_pt << "\n"));
+    DBG_(VVerb, (<< std::hex << "Invoke Satisfy: " << " for arg " << anArg << satisfy_pt << "\n"));
     satisfy(anArg);
     DBG_(VVerb, (<< "After satisfy"));
 }

diff --git a/components/Decoder/SemanticActions.hpp b/components/Decoder/SemanticActions.hpp
@@ -179,6 +179,7 @@ class BaseSemanticAction
 
     int64_t instructionNo() const { return theInstruction->sequenceNo(); }
 
+    // Pooria Poorsarvi Tehrani Creates the internal dependancy : i.e. instruction to its operands
     InternalDependance dependance(int32_t anArg = 0)
     {
         DBG_Assert(anArg < theNumOperands);
@@ -196,7 +197,11 @@ class BaseSemanticAction
     bool cancelled() const { return theInstruction->isSquashed() || theInstruction->isRetired(); }
     bool ready() const { return theReady[0] && theReady[1] && theReady[2] && theReady[3] && theReady[4]; }
     bool signalled() const { return theSignalled; }
-    void setReady(int32_t anArg, bool aReady) { theReady[anArg] = aReady; }
+    void setReady(int32_t anArg, bool aReady) {
+      // Pooria Poorsarvi Tehrani keep this debug or debugging gets hard
+      DBG_(Iface, (<< "Changing ready status to: " << aReady << " from " << theReady[anArg] << " for arg: " << anArg << " for instruction : " << *theInstruction <<  " and instruction address of " << theInstruction << " with code " << theInstruction->instCode() << " and action address of: " << this << " " << *this));
+      theReady[anArg] = aReady;
+    }
 
     void addRef();
     void releaseRef();
@@ -281,6 +286,8 @@ readConstantAction(SemanticInstruction* anInstruction, uint64_t aVal, eOperandCo
 simple_action
 calcAddressAction(SemanticInstruction* anInstruction, std::vector<std::list<InternalDependance>>& opDeps);
 simple_action
+calcAddressAction(SemanticInstruction* anInstruction, std::vector<std::list<InternalDependance>>& opDeps, uint8_t operandInstructionBase);
+simple_action
 translationAction(SemanticInstruction* anInstruction);
 
 predicated_action

diff --git a/components/Decoder/SemanticActions/ExecuteAction.cpp b/components/Decoder/SemanticActions/ExecuteAction.cpp
@@ -136,7 +136,7 @@ struct ExecuteAction : public ExecuteBase
 
         if (ready()) {
 
-            DBG_(Iface, (<< "Executing " << *this));
+            DBG_(Iface, (<< "Executing " << *this << " with instruction: " << *theInstruction << "and address instruction of " << theInstruction << " and code for instruction of "  << theInstruction->instCode() << " the address of the action is: " << this));
 
             if (theInstruction->hasPredecessorExecuted()) {
                 std::vector<Operand> operands;
@@ -415,14 +415,24 @@ fpExecuteAction(SemanticInstruction* anInstruction,
     return predicated_action(act, act->predicate());
 }
 
+
+simple_action
+calcAddressAction(SemanticInstruction* anInstruction, std::vector<std::list<InternalDependance>>& opDeps){
+    return calcAddressAction(anInstruction, opDeps, 0);
+}
+
 simple_action
-calcAddressAction(SemanticInstruction* anInstruction, std::vector<std::list<InternalDependance>>& opDeps)
+calcAddressAction(SemanticInstruction* anInstruction, std::vector<std::list<InternalDependance>>& opDeps, uint8_t operandInstructionBase)
 {
+
+    DBG_Assert(operandInstructionBase <= (kOperand5 -kOperand1), (<< "Too much operands decoded"));
+
     std::vector<eOperandCode> operands;
     for (uint32_t i = 0; i < opDeps.size(); ++i) {
-        operands.push_back(eOperandCode(kOperand1 + i));
+        operands.push_back(eOperandCode(kOperand1 + operandInstructionBase + i));
     }
     std::unique_ptr<Operation> add = operation(kADD_);
+    // Pooria Poorsarvi Tehrani , example of actions being created
     ExecuteAction* act             = new ExecuteAction(anInstruction, operands, kAddress, add, boost::none);
     anInstruction->addNewComponent(act);
 

diff --git a/components/Decoder/SemanticActions/ReadRegisterAction.cpp b/components/Decoder/SemanticActions/ReadRegisterAction.cpp
@@ -151,7 +151,6 @@ struct ReadRegisterAction : public BaseSemanticAction
             }
             if (!signalled()) {
                 SEMANTICS_DBG("Signalling");
-
                 mapped_reg name        = theInstruction->operand<mapped_reg>(theRegisterCode);
                 eResourceStatus status = core()->requestRegister(name);
 
@@ -160,9 +159,12 @@ struct ReadRegisterAction : public BaseSemanticAction
                     val    = boost::get<uint64_t>(aValue);
                 } else {
                     setReady(0, false);
+                    squashDependants();
+                    DBG_(Iface, (<< "Skipping adding reg by using set ready: " <<  status << " ready was set to: " << ready() << "address of instruction is: " << theInstruction << " and code for instruction of "  << theInstruction->instCode() << " the address of the action is: " << this));
                     return;
                 }
             } else {
+                DBG_(Iface, (<< "Just skipping without using set ready: "  << " address of instruction is: " << theInstruction << " and code for instruction of "  << theInstruction->instCode() << " the address of the action is: " << this));
                 return;
             }
         }

diff --git a/components/Decoder/encodings/LoadStore.cpp b/components/Decoder/encodings/LoadStore.cpp
@@ -89,7 +89,7 @@ CAS(archcode const& aFetchedOpcode, uint32_t aCPU, int64_t aSequenceNo)
 
     // obtain the loaded values
     std::vector<std::list<InternalDependance>> rs_deps(1);
-    addAddressCompute(inst, rs_deps);
+    addAddressCompute(inst, rs_deps, 2);
     addReadXRegister(inst, 3, rn, rs_deps[0], true);
 
     predicated_dependant_action cas;
@@ -142,7 +142,6 @@ CAS(archcode const& aFetchedOpcode, uint32_t aCPU, int64_t aSequenceNo)
         addReadXRegister(inst, 1, rs, cmp_dep[0], sf);
         addReadXRegister(inst, 2, rs, cmp_dep[1], sf);
     }
-
     return inst;
 }
 

diff --git a/components/Decoder/encodings/SharedFunctions.cpp b/components/Decoder/encodings/SharedFunctions.cpp
@@ -484,14 +484,19 @@ addExecute(SemanticInstruction* inst,
 }
 
 simple_action
-addAddressCompute(SemanticInstruction* inst, std::vector<std::list<InternalDependance>>& rs_deps)
+addAddressCompute(SemanticInstruction* inst, std::vector<std::list<InternalDependance>>& rs_deps){
+    return addAddressCompute(inst, rs_deps, 0);
+}
+
+simple_action
+addAddressCompute(SemanticInstruction* inst, std::vector<std::list<InternalDependance>>& rs_deps, uint8_t operandInstructionBase)
 {
     DECODER_TRACE;
 
     simple_action tr                         = translationAction(inst);
     multiply_dependant_action update_address = updateVirtualAddressAction(inst);
     inst->addDispatchEffect(satisfy(inst, update_address.dependances[1]));
-    simple_action exec = calcAddressAction(inst, rs_deps);
+    simple_action exec = calcAddressAction(inst, rs_deps, operandInstructionBase);
 
     connectDependance(update_address.dependances[0], exec);
     connectDependance(tr.action->dependance(0), exec);
@@ -674,4 +679,4 @@ generateException(archcode const& aFetchedOpcode, uint32_t aCPU, int64_t aSequen
     return inst;
 }
 
-} // namespace nDecoder
+} // namespace nDecoder
diff --git a/components/Decoder/encodings/SharedFunctions.hpp b/components/Decoder/encodings/SharedFunctions.hpp
@@ -162,6 +162,8 @@ addPairDestination(SemanticInstruction* inst,
                    bool addSquash = true);
 simple_action
 addAddressCompute(SemanticInstruction* inst, std::vector<std::list<InternalDependance>>& rs_deps);
+simple_action
+addAddressCompute(SemanticInstruction* inst, std::vector<std::list<InternalDependance>>& rs_deps, uint8_t operandInstructionBase);
 void
 setRD(SemanticInstruction* inst, uint32_t rd);
 void
@@ -218,4 +220,4 @@ bool
 disas_ldst_compute_iss_sf(int size, bool is_signed, int opc);
 } // namespace nDecoder
 
-#endif // FLEXUS_armDECODER_armSHAREDFUNCTIONS_HPP_INCLUDED
+#endif // FLEXUS_armDECODER_armSHAREDFUNCTIONS_HPP_INCLUDED
diff --git a/components/uArch/CoreModel/cycle.cpp b/components/uArch/CoreModel/cycle.cpp
@@ -1499,7 +1499,7 @@ CoreImpl::commit(boost::intrusive_ptr<Instruction> anInstruction)
         CORE_DBG("Instruction is neither annuled nor is a micro-op");
 
         validation_passed &= anInstruction->preValidate();
-        DBG_(Iface, (<< "Pre Validating... " << validation_passed));
+        DBG_(Iface, (<< "Pre Validating... " << validation_passed << " for " << anInstruction << " " << *anInstruction));
 
         //    DBG_( VVerb, Condition(!validation_passed) ( << *anInstruction <<
         //    "Prevalidation failure." ) ); bool take_interrupt =