[PATCH] D103955: [MCA] Use LSU for the in-order pipeline

[Andrea Di Biagio via Phabricator via llvm-commits]

andreadb added a comment.

Thanks Andrew.



================
Comment at: llvm/lib/MCA/Stages/InOrderIssueStage.cpp:53-54
 
+  if (LSU.isAvailable(IR) != LSUnit::LSU_AVAILABLE)
+    return false;
+
----------------
This check is harmless, but completely redundant for an in-order processor.
It should never fail in practice because load/store queues are not really modelled by in-order processors.
In an in-order processors, the dispatch event coincides with the issue event, so there is no need for queueing loads/stores.
So, we should always ignore the presence of queues and I think you can safely get rid of that check.


================
Comment at: llvm/lib/MCA/Stages/InOrderIssueStage.cpp:167-171
+  } else if (IR.getInstruction()->isMemOp() && !LSU.isReady(IR)) {
+    // This load (store) aliases with a preceding store (load). Delay
+    // it until the depenency is cleared.
+    *StallCycles = 1;
   } else if (LastWriteBackCycle) {
----------------
I don't think that this should be an else-if.
It is better to always test this condition at the end of the if-then-else chain if `StallCycles` is still zero.

I suspect this is the reason why you get some out-of-order execution in the test that you have added.


================
Comment at: llvm/test/tools/llvm-mca/AArch64/Cortex/A55-load-store-noalias.s:97-98
+# CHECK-NEXT: [0,1]     .DeeeE    ..   str	x1, [x10]
+# CHECK-NEXT: [0,2]     .DeeE.    ..   ldr	x2, [x10]
+# CHECK-NEXT: [0,3]     . DE .    ..   nop
+# CHECK-NEXT: [0,4]     .  DeeE   ..   ldr	x2, [x10]
----------------
This doesn't look correct.
Any idea why these instructions are executed out of order?
Edit: I think it might be due to the LSU check you have added (see my other comment).



Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D103955/new/

https://reviews.llvm.org/D103955