This is an archive of the discontinued LLVM Phabricator instance.

Differential D104863

[LSV] Teach LSV to handle atomic ops.
AbandonedPublic

Authored by hliao on Jun 24 2021, 8:56 AM.

Details

Reviewers
arsenm
volkan
bogner
Summary
  • It's legal to reorder non-atomic and Unordered loads/stores around Monotonic or weaker atomics.

Diff Detail

Event Timeline

hliao created this revision.Jun 24 2021, 8:56 AM
Herald added subscribers: jfb, hiraditya. · View Herald TranscriptJun 24 2021, 8:56 AM
hliao requested review of this revision.Jun 24 2021, 8:56 AM
Herald added a project: Restricted Project. · View Herald TranscriptJun 24 2021, 8:56 AM
Herald added subscribers: llvm-commits, wdng. · View Herald Transcript
Harbormaster completed remote builds in B110845: Diff 354272.Jun 24 2021, 9:40 AM
hliao updated this revision to Diff 354428.Jun 24 2021, 9:41 PM
  • Fix the case where monotonic or weaker atomic ops may alias to load or store.
Harbormaster completed remote builds in B110947: Diff 354428.Jun 24 2021, 10:16 PM
arsenm added inline comments.Aug 6 2021, 6:08 AM
llvm/lib/Transforms/Vectorize/LoadStoreVectorizer.cpp
700–714

It seems like this should be a global utility function somewhere

800–810

I think there's a utility for this osmewhere already (that also will handle the intrinsic calls)

llvm/test/Transforms/LoadStoreVectorizer/atomic.ll
8–9

Should check more, should show the relation to the atomics

hliao abandoned this revision.Sep 15 2021, 2:59 PM

obsoleted by D109020

Revision Contents

PathSize
llvm/
lib/
Transforms/
Vectorize/
45 lines
test/
Transforms/
LoadStoreVectorizer/
237 lines

Diff 354428

llvm/lib/Transforms/Vectorize/LoadStoreVectorizer.cpp

Show First 20 Lines • Show All 687 Lines • ▼ Show 20 Lines for (Instruction *I : Chain) {
assert(isa<LoadInst>(I) && assert(isa<LoadInst>(I) &&
"All elements of Chain must be loads, or all must be stores."); "All elements of Chain must be loads, or all must be stores.");
else else
assert(isa<StoreInst>(I) && assert(isa<StoreInst>(I) &&
"All elements of Chain must be loads, or all must be stores."); "All elements of Chain must be loads, or all must be stores.");
} }
}); });
// Check atomic barrier where regular loads/stores cannot be freely moved
// across.
// TODO: Certain load or store movement is still allowed for acquire or
// release order.
auto IsAtomicBarrier = [](Instruction *I) {
if (!I->isAtomic())
return false;
if (auto *LD = dyn_cast<LoadInst>(I))
return isStrongerThanMonotonic(LD->getOrdering());
if (auto *ST = dyn_cast<StoreInst>(I))
return isStrongerThanMonotonic(ST->getOrdering());
if (auto *RMW = dyn_cast<AtomicRMWInst>(I))
return isStrongerThanMonotonic(RMW->getOrdering());
if (auto *CmpXchg = dyn_cast<AtomicCmpXchgInst>(I))
return isStrongerThanMonotonic(CmpXchg->getMergedOrdering());
// Atomic instructions like fence has order at least as strong as acquire.
// Treat them and other unknown atomic ops conservatively.
return true;
};
arsenmUnsubmitted
Not Done
ReplyInline Actions

It seems like this should be a global utility function somewhere

arsenm: It seems like this should be a global utility function somewhere
for (Instruction &I : make_range(getBoundaryInstrs(Chain))) { for (Instruction &I : make_range(getBoundaryInstrs(Chain))) {
if (I.isAtomic()) {
if (IsAtomicBarrier(&I)) {
LLVM_DEBUG(dbgs() << "LSV: Found atomic barrier: " << I << '\n');
break;
}
// Regular loads/stores could be moved around them, but extra aliasing
// check is required.
MemoryInstrs.push_back(&I);
continue;
}
if (isa<LoadInst>(I) || isa<StoreInst>(I)) { if (isa<LoadInst>(I) || isa<StoreInst>(I)) {
if (!is_contained(Chain, &I)) if (!is_contained(Chain, &I))
MemoryInstrs.push_back(&I); MemoryInstrs.push_back(&I);
else else
ChainInstrs.push_back(&I); ChainInstrs.push_back(&I);
} else if (isa<IntrinsicInst>(&I) && } else if (isa<IntrinsicInst>(&I) &&
cast<IntrinsicInst>(&I)->getIntrinsicID() == cast<IntrinsicInst>(&I)->getIntrinsicID() ==
Intrinsic::sideeffect) { Intrinsic::sideeffect) {
▲ Show 20 Lines • Show All 57 Lines • ▼ Show 20 Lines for (Instruction *MemInstr : MemoryInstrs) {
// Same case, but in reverse. // Same case, but in reverse.
if (MemLoad && isa<StoreInst>(ChainInstr) && if (MemLoad && isa<StoreInst>(ChainInstr) &&
(IsInvariantLoad(MemLoad) || MemLoad->comesBefore(ChainInstr))) (IsInvariantLoad(MemLoad) || MemLoad->comesBefore(ChainInstr)))
continue; continue;
if (!AA.isNoAlias(MemoryLocation::get(MemInstr), if (!AA.isNoAlias(MemoryLocation::get(MemInstr),
MemoryLocation::get(ChainInstr))) { MemoryLocation::get(ChainInstr))) {
LLVM_DEBUG({ LLVM_DEBUG({
auto getPointerOperand = [](const Instruction *I) -> const Value * {
if (auto *Load = dyn_cast<LoadInst>(I))
return Load->getPointerOperand();
if (auto *Store = dyn_cast<StoreInst>(I))
return Store->getPointerOperand();
if (auto *RMW = dyn_cast<AtomicRMWInst>(I))
return RMW->getPointerOperand();
if (auto *CmpXchg = dyn_cast<AtomicCmpXchgInst>(I))
return CmpXchg->getPointerOperand();
return nullptr;
};
arsenmUnsubmitted
Not Done
ReplyInline Actions

I think there's a utility for this osmewhere already (that also will handle the intrinsic calls)

arsenm: I think there's a utility for this osmewhere already (that also will handle the intrinsic calls)
dbgs() << "LSV: Found alias:\n" dbgs() << "LSV: Found alias:\n"
" Aliasing instruction and pointer:\n" " Aliasing instruction and pointer:\n"
<< " " << *MemInstr << '\n' << " " << *MemInstr << '\n'
<< " " << *getLoadStorePointerOperand(MemInstr) << '\n' << " " << *getPointerOperand(MemInstr) << '\n'
<< " Aliased instruction and pointer:\n" << " Aliased instruction and pointer:\n"
<< " " << *ChainInstr << '\n' << " " << *ChainInstr << '\n'
<< " " << *getLoadStorePointerOperand(ChainInstr) << '\n'; << " " << *getPointerOperand(ChainInstr) << '\n';
}); });
// Save this aliasing memory instruction as a barrier, but allow other // Save this aliasing memory instruction as a barrier, but allow other
// instructions that precede the barrier to be vectorized with this one. // instructions that precede the barrier to be vectorized with this one.
BarrierMemoryInstr = MemInstr; BarrierMemoryInstr = MemInstr;
break; break;
} }
} }
// Continue the search only for store chains, since vectorizing stores that // Continue the search only for store chains, since vectorizing stores that
▲ Show 20 Lines • Show All 571 Lines • Show Last 20 Lines

llvm/test/Transforms/LoadStoreVectorizer/atomic.ll

  • This file was added.
; RUN: opt -S < %s -load-store-vectorizer | FileCheck %s
; RUN: opt -S < %s -passes='function(load-store-vectorizer)' | FileCheck %s
; load-store vectorization across atomic ops if its order not stronger than
; monotonic.
; CHECK-LABEL: t0
; CHECK: load <4 x float>
; CHECK: store <4 x float>
arsenmUnsubmitted
Not Done
ReplyInline Actions

Should check more, should show the relation to the atomics

arsenm: Should check more, should show the relation to the atomics
define void @t0(float* noalias %p, i32* noalias %q) {
%q0 = getelementptr i32, i32* %q, i64 0
%q1 = getelementptr i32, i32* %q, i64 1
%p0 = getelementptr float, float* %p, i64 0
%p1 = getelementptr float, float* %p, i64 1
%p2 = getelementptr float, float* %p, i64 2
%p3 = getelementptr float, float* %p, i64 3
%l0 = load float, float* %p0, align 16
%l1 = load float, float* %p1
%l2 = load float, float* %p2
%a0 = load atomic i32, i32* %q0 monotonic, align 4
%l3 = load float, float* %p3
store float %l0, float* %p0, align 16
%a1 = load atomic i32, i32* %q1 monotonic, align 4
store float %l1, float* %p1
store float %l2, float* %p2
store float %l3, float* %p3
ret void
}
; CHECK-LABEL: f0
; CHECK-NOT: load <4 x float>
; CHECK-NOT: store <4 x float>
; CHECK: ret void
define void @f0(float* noalias %p, i32* noalias %q) {
%q0 = getelementptr i32, i32* %q, i64 0
%q1 = getelementptr i32, i32* %q, i64 1
%p0 = getelementptr float, float* %p, i64 0
%p1 = getelementptr float, float* %p, i64 1
%p2 = getelementptr float, float* %p, i64 2
%p3 = getelementptr float, float* %p, i64 3
%l0 = load float, float* %p0, align 16
%l1 = load float, float* %p1
%l2 = load float, float* %p2
%a0 = load atomic i32, i32* %q0 acquire, align 4
%l3 = load float, float* %p3
store float %l0, float* %p0, align 16
%a1 = load atomic i32, i32* %q1 acquire, align 4
store float %l1, float* %p1
store float %l2, float* %p2
store float %l3, float* %p3
ret void
}
; CHECK-LABEL: t1
; CHECK: load <4 x float>
; CHECK: store <4 x float>
define void @t1(float* noalias %p, i32* noalias %q) {
%q0 = getelementptr i32, i32* %q, i64 0
%q1 = getelementptr i32, i32* %q, i64 1
%p0 = getelementptr float, float* %p, i64 0
%p1 = getelementptr float, float* %p, i64 1
%p2 = getelementptr float, float* %p, i64 2
%p3 = getelementptr float, float* %p, i64 3
%l0 = load float, float* %p0, align 16
%l1 = load float, float* %p1
%l2 = load float, float* %p2
store atomic i32 0, i32* %q0 monotonic, align 4
%l3 = load float, float* %p3
store float %l0, float* %p0, align 16
store atomic i32 1, i32* %q1 monotonic, align 4
store float %l1, float* %p1
store float %l2, float* %p2
store float %l3, float* %p3
ret void
}
; CHECK-LABEL: f1
; CHECK-NOT: load <4 x float>
; CHECK-NOT: store <4 x float>
; CHECK: ret void
define void @f1(float* noalias %p, i32* noalias %q) {
%q0 = getelementptr i32, i32* %q, i64 0
%q1 = getelementptr i32, i32* %q, i64 1
%p0 = getelementptr float, float* %p, i64 0
%p1 = getelementptr float, float* %p, i64 1
%p2 = getelementptr float, float* %p, i64 2
%p3 = getelementptr float, float* %p, i64 3
%l0 = load float, float* %p0, align 16
%l1 = load float, float* %p1
%l2 = load float, float* %p2
store atomic i32 0, i32* %q0 release, align 4
%l3 = load float, float* %p3
store float %l0, float* %p0, align 16
store atomic i32 1, i32* %q1 release, align 4
store float %l1, float* %p1
store float %l2, float* %p2
store float %l3, float* %p3
ret void
}
; CHECK-LABEL: t2
; CHECK: load <4 x float>
; CHECK: store <4 x float>
define void @t2(float* noalias %p, i32* noalias %q) {
%q0 = getelementptr i32, i32* %q, i64 0
%q1 = getelementptr i32, i32* %q, i64 1
%p0 = getelementptr float, float* %p, i64 0
%p1 = getelementptr float, float* %p, i64 1
%p2 = getelementptr float, float* %p, i64 2
%p3 = getelementptr float, float* %p, i64 3
%l0 = load float, float* %p0, align 16
%l1 = load float, float* %p1
%l2 = load float, float* %p2
%a0 = atomicrmw add i32* %q0, i32 1 monotonic
%l3 = load float, float* %p3
store float %l0, float* %p0, align 16
%a1 = atomicrmw add i32* %q1, i32 2 monotonic
store float %l1, float* %p1
store float %l2, float* %p2
store float %l3, float* %p3
ret void
}
; CHECK-LABEL: f2
; CHECK-NOT: load <4 x float>
; CHECK-NOT: store <4 x float>
; CHECK: ret void
define void @f2(float* noalias %p, i32* noalias %q) {
%q0 = getelementptr i32, i32* %q, i64 0
%q1 = getelementptr i32, i32* %q, i64 1
%p0 = getelementptr float, float* %p, i64 0
%p1 = getelementptr float, float* %p, i64 1
%p2 = getelementptr float, float* %p, i64 2
%p3 = getelementptr float, float* %p, i64 3
%l0 = load float, float* %p0, align 16
%l1 = load float, float* %p1
%l2 = load float, float* %p2
%a0 = atomicrmw add i32* %q0, i32 1 acq_rel
%l3 = load float, float* %p3
store float %l0, float* %p0, align 16
%a1 = atomicrmw add i32* %q1, i32 2 acq_rel
store float %l1, float* %p1
store float %l2, float* %p2
store float %l3, float* %p3
ret void
}
; CHECK-LABEL: g2
; CHECK-NOT: load <4 x i32>
; CHECK-NOT: store <4 x i32>
; CHECK: ret void
define void @g2(i32* noalias %p) {
%p0 = getelementptr i32, i32* %p, i64 0
%p1 = getelementptr i32, i32* %p, i64 1
%p2 = getelementptr i32, i32* %p, i64 2
%p3 = getelementptr i32, i32* %p, i64 3
%l0 = load i32, i32* %p0, align 16
%l1 = load i32, i32* %p1
%l2 = load i32, i32* %p2
%a0 = atomicrmw add i32* %p3, i32 1 monotonic
%l3 = load i32, i32* %p3
store i32 %l0, i32* %p0, align 16
%a1 = atomicrmw add i32* %p0, i32 2 monotonic
store i32 %l1, i32* %p1
store i32 %l2, i32* %p2
store i32 %l3, i32* %p3
ret void
}
; CHECK-LABEL: t3
; CHECK: load <4 x float>
; CHECK: store <4 x float>
define void @t3(float* noalias %p, i32* noalias %q) {
%q0 = getelementptr i32, i32* %q, i64 0
%q1 = getelementptr i32, i32* %q, i64 1
%p0 = getelementptr float, float* %p, i64 0
%p1 = getelementptr float, float* %p, i64 1
%p2 = getelementptr float, float* %p, i64 2
%p3 = getelementptr float, float* %p, i64 3
%l0 = load float, float* %p0, align 16
%l1 = load float, float* %p1
%l2 = load float, float* %p2
%a0 = cmpxchg i32* %q0, i32 0, i32 1 monotonic monotonic
%l3 = load float, float* %p3
store float %l0, float* %p0, align 16
%a1 = cmpxchg i32* %q1, i32 2, i32 3 monotonic monotonic
store float %l1, float* %p1
store float %l2, float* %p2
store float %l3, float* %p3
ret void
}
; CHECK-LABEL: f3
; CHECK-NOT: load <4 x float>
; CHECK-NOT: store <4 x float>
; CHECK: ret void
define void @f3(float* noalias %p, i32* noalias %q) {
%q0 = getelementptr i32, i32* %q, i64 0
%q1 = getelementptr i32, i32* %q, i64 1
%p0 = getelementptr float, float* %p, i64 0
%p1 = getelementptr float, float* %p, i64 1
%p2 = getelementptr float, float* %p, i64 2
%p3 = getelementptr float, float* %p, i64 3
%l0 = load float, float* %p0, align 16
%l1 = load float, float* %p1
%l2 = load float, float* %p2
%a0 = cmpxchg i32* %q0, i32 0, i32 1 acquire monotonic
%l3 = load float, float* %p3
store float %l0, float* %p0, align 16
%a1 = cmpxchg i32* %q1, i32 2, i32 3 acquire monotonic
store float %l1, float* %p1
store float %l2, float* %p2
store float %l3, float* %p3
ret void
}
; CHECK-LABEL: g3
; CHECK-NOT: load <4 x i32>
; CHECK-NOT: store <4 x i32>
; CHECK: ret void
define void @g3(i32* noalias %p) {
%p0 = getelementptr i32, i32* %p, i64 0
%p1 = getelementptr i32, i32* %p, i64 1
%p2 = getelementptr i32, i32* %p, i64 2
%p3 = getelementptr i32, i32* %p, i64 3
%l0 = load i32, i32* %p0, align 16
%l1 = load i32, i32* %p1
%l2 = load i32, i32* %p2
%a0 = cmpxchg i32* %p3, i32 0, i32 1 acquire monotonic
%l3 = load i32, i32* %p3
store i32 %l0, i32* %p0, align 16
%a1 = cmpxchg i32* %p0, i32 2, i32 3 acquire monotonic
store i32 %l1, i32* %p1
store i32 %l2, i32* %p2
store i32 %l3, i32* %p3
ret void
}