diff options
author | Andrea Di Biagio <Andrea_DiBiagio@sn.scee.net> | 2018-06-13 18:30:14 +0000 |
---|---|---|
committer | Andrea Di Biagio <Andrea_DiBiagio@sn.scee.net> | 2018-06-13 18:30:14 +0000 |
commit | 0ffb2271a1e60de368fc6b0b1df4595220981a8d (patch) | |
tree | 75728e9095837e73b3f11de93d6feac3ee575ef0 | |
parent | d71614a438b4c9a6c595c695e86e8d24e6070fac (diff) | |
download | llvm-0ffb2271a1e60de368fc6b0b1df4595220981a8d.zip llvm-0ffb2271a1e60de368fc6b0b1df4595220981a8d.tar.gz llvm-0ffb2271a1e60de368fc6b0b1df4595220981a8d.tar.bz2 |
[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.
llvm-svn: 334633
-rw-r--r-- | llvm/test/tools/llvm-mca/X86/BtVer2/pr37790.s | 41 | ||||
-rw-r--r-- | llvm/tools/llvm-mca/LSUnit.cpp | 20 |
2 files changed, 53 insertions, 8 deletions
diff --git a/llvm/test/tools/llvm-mca/X86/BtVer2/pr37790.s b/llvm/test/tools/llvm-mca/X86/BtVer2/pr37790.s new file mode 100644 index 0000000..990e15f --- /dev/null +++ b/llvm/test/tools/llvm-mca/X86/BtVer2/pr37790.s @@ -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) diff --git a/llvm/tools/llvm-mca/LSUnit.cpp b/llvm/tools/llvm-mca/LSUnit.cpp index dfd3e53..c2f3b9d 100644 --- a/llvm/tools/llvm-mca/LSUnit.cpp +++ b/llvm/tools/llvm-mca/LSUnit.cpp @@ -78,18 +78,16 @@ bool LSUnit::isReady(const InstRef &IR) const { 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 InstRef &IR) { 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 |