[PATCH] D79351: [MCA] Fixed a bug where loads and stores were sometimes incorrectly marked as depedent (PR45793).

Mon May 4 13:26:57 PDT 2020

mattd added a comment.

I'm totally cool with this change; however, it's been a while since I've taken a look at this part of MCA.  I'll let other's chime in as well, but +1 from me.

================
Comment at: llvm/lib/MCA/HardwareUnits/LSUnit.cpp:69

 unsigned LSUnit::dispatch(const InstRef &IR) {
   const InstrDesc &Desc = IR.getInstruction()->getDesc();
----------------
This function is getting pretty large.  I'm not sure how to break this up, but it could be useful as a NFC follow-up at a later time.

================
Comment at: llvm/lib/MCA/HardwareUnits/LSUnit.cpp:131
+
+  bool ShouldCreateANewGroup = IsMemBarrier || !ImmediateLoadDominator ||
+                               ImmediateLoadDominator <= CurrentStoreGroupID ||
----------------
Is !ImmediateLoadDominator necessary here?  It seems that would be implied by the next condition:  `ImmediateLoadDominator <= CurrentStoreGroupID`  However, I suppose this does read clearer by leaving the !ImmediateLoadDominator check in place..

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

https://reviews.llvm.org/D79351