[llvm] r334633 - [llvm-mca] Fixed a bug in the logic that checks if a memory operation is ready to execute.

[Andrea Di Biagio via llvm-commits]

Author: adibiagio
Date: Wed Jun 13 11:30:14 2018
New Revision: 334633

URL: http://llvm.org/viewvc/llvm-project?rev=334633&view=rev
Log:
[llvm-mca] Fixed a bug in the logic that checks if a memory operation is ready to execute.

Fixes PR37790.

In some (very rare) cases, the LSUnit (Load/Store unit) was wrongly marking a
load (or store) as "ready to execute" effectively bypassing older memory barrier
instructions.

To reproduce this bug, the memory barrier must be the first instruction in the
input assembly sequence, and it doesn't have to perform any register writes.


Added:
    llvm/trunk/test/tools/llvm-mca/X86/BtVer2/pr37790.s
Modified:
    llvm/trunk/tools/llvm-mca/LSUnit.cpp

Added: llvm/trunk/test/tools/llvm-mca/X86/BtVer2/pr37790.s
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/tools/llvm-mca/X86/BtVer2/pr37790.s?rev=334633&view=auto
==============================================================================
--- llvm/trunk/test/tools/llvm-mca/X86/BtVer2/pr37790.s (added)
+++ llvm/trunk/test/tools/llvm-mca/X86/BtVer2/pr37790.s Wed Jun 13 11:30:14 2018
@@ -0,0 +1,41 @@
+# NOTE: Assertions have been autogenerated by utils/update_mca_test_checks.py
+# RUN: llvm-mca -mtriple=x86_64-unknown-unknown -mcpu=btver2 -lqueue=2 -iterations=2 -resource-pressure=false -timeline -timeline-max-cycles=104 < %s | FileCheck %s
+
+int3
+stmxcsr (%rsp)
+
+# CHECK:      Iterations:        2
+# CHECK-NEXT: Instructions:      4
+# CHECK-NEXT: Total Cycles:      205
+# CHECK-NEXT: Dispatch Width:    2
+# CHECK-NEXT: IPC:               0.02
+# CHECK-NEXT: Block RThroughput: 1.0
+
+# CHECK:      Instruction Info:
+# CHECK-NEXT: [1]: #uOps
+# CHECK-NEXT: [2]: Latency
+# CHECK-NEXT: [3]: RThroughput
+# CHECK-NEXT: [4]: MayLoad
+# CHECK-NEXT: [5]: MayStore
+# CHECK-NEXT: [6]: HasSideEffects
+
+# CHECK:      [1]    [2]    [3]    [4]    [5]    [6]    Instructions:
+# CHECK-NEXT:  1      100   0.50    *      *      *     int3
+# CHECK-NEXT:  1      1     1.00    *      *      *     stmxcsr	(%rsp)
+
+# CHECK:      Timeline view:
+# CHECK-NEXT:                     0123456789          0123456789          0123456789          0123456789          0123456789
+# CHECK-NEXT: Index     0123456789          0123456789          0123456789          0123456789          0123456789          0123
+
+# CHECK:      [0,0]     DeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeER.   int3
+# CHECK-NEXT: [0,1]     D====================================================================================================eER   stmxcsr	(%rsp)
+
+# CHECK:      Average Wait times (based on the timeline view):
+# CHECK-NEXT: [0]: Executions
+# CHECK-NEXT: [1]: Average time spent waiting in a scheduler's queue
+# CHECK-NEXT: [2]: Average time spent waiting in a scheduler's queue while ready
+# CHECK-NEXT: [3]: Average time elapsed from WB until retire stage
+
+# CHECK:            [0]    [1]    [2]    [3]
+# CHECK-NEXT: 0.     1     1.0    1.0    0.0       int3
+# CHECK-NEXT: 1.     1     101.0  0.0    0.0       stmxcsr	(%rsp)

Modified: llvm/trunk/tools/llvm-mca/LSUnit.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/llvm-mca/LSUnit.cpp?rev=334633&r1=334632&r2=334633&view=diff
==============================================================================
--- llvm/trunk/tools/llvm-mca/LSUnit.cpp (original)
+++ llvm/trunk/tools/llvm-mca/LSUnit.cpp Wed Jun 13 11:30:14 2018
@@ -78,18 +78,16 @@ bool LSUnit::isReady(const InstRef &IR)
   bool IsAStore = StoreQueue.count(Index) != 0;
   assert((IsALoad || IsAStore) && "Instruction is not in queue!");
 
-  unsigned LoadBarrierIndex = LoadBarriers.empty() ? 0 : *LoadBarriers.begin();
-  unsigned StoreBarrierIndex =
-      StoreBarriers.empty() ? 0 : *StoreBarriers.begin();
-
-  if (IsALoad && LoadBarrierIndex) {
+  if (IsALoad && !LoadBarriers.empty()) {
+    unsigned LoadBarrierIndex = *LoadBarriers.begin();
     if (Index > LoadBarrierIndex)
       return false;
     if (Index == LoadBarrierIndex && Index != *LoadQueue.begin())
       return false;
   }
 
-  if (IsAStore && StoreBarrierIndex) {
+  if (IsAStore && !StoreBarriers.empty()) {
+    unsigned StoreBarrierIndex = *StoreBarriers.begin();
     if (Index > StoreBarrierIndex)
       return false;
     if (Index == StoreBarrierIndex && Index != *StoreQueue.begin())
@@ -135,9 +133,15 @@ void LSUnit::onInstructionExecuted(const
     StoreQueue.erase(it);
   }
 
-  if (!StoreBarriers.empty() && Index == *StoreBarriers.begin())
+  if (!StoreBarriers.empty() && Index == *StoreBarriers.begin()) {
+    LLVM_DEBUG(dbgs() << "[LSUnit]: Instruction idx=" << Index
+                      << " has been removed from the set of store barriers.\n");
     StoreBarriers.erase(StoreBarriers.begin());
-  if (!LoadBarriers.empty() && Index == *LoadBarriers.begin())
+  }
+  if (!LoadBarriers.empty() && Index == *LoadBarriers.begin()) {
+    LLVM_DEBUG(dbgs() << "[LSUnit]: Instruction idx=" << Index
+                      << " has been removed from the set of load barriers.\n");
     LoadBarriers.erase(LoadBarriers.begin());
+  }
 }
 } // namespace mca