Skip to content

Commit 86991d3

Browse files
committed
[LoopUnswitch] Fix logic to avoid unswitching with atomic loads.
The existing code did not deal with atomic loads correctly. Such loads are represented as MemoryDefs. Bail out on any MemoryAccess that is not a MemoryUse.
1 parent c8b4337 commit 86991d3

File tree

2 files changed

+16
-10
lines changed

2 files changed

+16
-10
lines changed

llvm/lib/Transforms/Scalar/LoopUnswitch.cpp

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -681,16 +681,22 @@ hasPartialIVCondition(Loop *L, MemorySSA &MSSA, AAResults *AA) {
681681
if (!isa<LoadInst>(I) && !isa<GetElementPtrInst>(I))
682682
return {};
683683

684-
// Do not duplicate volatile loads.
684+
// Do not duplicate volatile and atomic loads.
685685
if (auto *LI = dyn_cast<LoadInst>(I))
686-
if (LI->isVolatile())
686+
if (LI->isVolatile() || LI->isAtomic())
687687
return {};
688688

689689
ToDuplicate.push_back(I);
690-
if (auto *MemUse = dyn_cast_or_null<MemoryUse>(MSSA.getMemoryAccess(I))) {
691-
// Queue the defining access to check for alias checks.
692-
AccessesToCheck.push_back(MemUse->getDefiningAccess());
693-
AccessedLocs.push_back(MemoryLocation::get(I));
690+
if (MemoryAccess *MA = MSSA.getMemoryAccess(I)) {
691+
if (auto *MemUse = dyn_cast_or_null<MemoryUse>(MA)) {
692+
// Queue the defining access to check for alias checks.
693+
AccessesToCheck.push_back(MemUse->getDefiningAccess());
694+
AccessedLocs.push_back(MemoryLocation::get(I));
695+
} else {
696+
// MemoryDefs may clobber the location or may be atomic memory
697+
// operations. Bail out.
698+
return {};
699+
}
694700
}
695701
WorkList.append(I->op_begin(), I->op_end());
696702
}
@@ -972,6 +978,8 @@ bool LoopUnswitch::processCurrentLoop() {
972978
!findOptionMDForLoop(CurrentLoop, "llvm.loop.unswitch.partial.disable")) {
973979
auto ToDuplicate = hasPartialIVCondition(CurrentLoop, *MSSA, AA);
974980
if (!ToDuplicate.first.empty()) {
981+
LLVM_DEBUG(dbgs() << "loop-unswitch: Found partially invariant condition "
982+
<< *ToDuplicate.first[0] << "\n");
975983
++NumBranches;
976984
unswitchIfProfitable(ToDuplicate.first[0], ToDuplicate.second,
977985
CurrentLoop->getHeader()->getTerminator(),

llvm/test/Transforms/LoopUnswitch/partial-unswitch.ll

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -731,11 +731,10 @@ exit:
731731

732732
; Do not unswitch if the condition depends on an atomic load. Duplicating such
733733
; loads is not safe.
734-
; TODO
735734
define i32 @no_partial_unswitch_atomic_load_unordered(i32* %ptr, i32 %N) {
736735
; CHECK-LABEL: @no_partial_unswitch_atomic_load_unordered
737736
; CHECK-NEXT: entry:
738-
; CHECK-NEXT: load
737+
; CHECK-NEXT: br label %loop.header
739738
;
740739
entry:
741740
br label %loop.header
@@ -764,11 +763,10 @@ exit:
764763

765764
; Do not unswitch if the condition depends on an atomic load. Duplicating such
766765
; loads is not safe.
767-
; TODO
768766
define i32 @no_partial_unswitch_atomic_load_monotonic(i32* %ptr, i32 %N) {
769767
; CHECK-LABEL: @no_partial_unswitch_atomic_load_monotonic
770768
; CHECK-NEXT: entry:
771-
; CHECK-NEXT: load
769+
; CHECK-NEXT: br label %loop.header
772770
;
773771
entry:
774772
br label %loop.header

0 commit comments

Comments
 (0)