6502/llvm-6502
1
0
Fork 0
mirror of https://github.com/c64scene-ar/llvm-6502.git synced 2024-12-13 20:32:21 +00:00
Code Issues Projects Releases Wiki Activity

MemoryDependenceAnalysis: Don't miscompile atomics

Browse Source 
r216771 introduced a change to MemoryDependenceAnalysis that allowed it
to reason about acquire/release operations.  However, this change does
not ensure that the acquire/release operations pair.  Unfortunately,
this leads to miscompiles as we won't see an acquire load as properly
memory effecting.  This largely reverts r216771.

This fixes PR22708.

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@232889 91177308-0d34-0410-b5e6-96231b3b80d8
This commit is contained in:
David Majnemer 2015-03-21 06:19:17 +00:00
parent 048ca17f6e
commit ac5b42cad8
3 changed files with 21 additions and 96 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

15
lib/Analysis/MemoryDependenceAnalysis.cpp
View File

@ -407,7 +407,6 @@ getPointerDependencyFrom(const AliasAnalysis::Location &MemLoc, bool isLoad,
// by every program that can detect any optimisation of that kind: either
// it is racy (undefined) or there is a release followed by an acquire
// between the pair of accesses under consideration.
bool HasSeenAcquire = false;
if (isLoad && QueryInst) {
LoadInst *LI = dyn_cast<LoadInst>(QueryInst);
@ -468,12 +467,12 @@ getPointerDependencyFrom(const AliasAnalysis::Location &MemLoc, bool isLoad,
// Atomic loads have complications involved.
// A Monotonic (or higher) load is OK if the query inst is itself not atomic.
// An Acquire (or higher) load sets the HasSeenAcquire flag, so that any
// release store will know to return getClobber.
// FIXME: This is overly conservative.
if (LI->isAtomic() && LI->getOrdering() > Unordered) {
if (!QueryInst)
return MemDepResult::getClobber(LI);
if (LI->getOrdering() != Monotonic)
return MemDepResult::getClobber(LI);
if (auto *QueryLI = dyn_cast<LoadInst>(QueryInst)) {
if (!QueryLI->isSimple())
return MemDepResult::getClobber(LI);
@ -483,9 +482,6 @@ getPointerDependencyFrom(const AliasAnalysis::Location &MemLoc, bool isLoad,
} else if (QueryInst->mayReadOrWriteMemory()) {
return MemDepResult::getClobber(LI);
}
if (isAtLeastAcquire(LI->getOrdering()))
HasSeenAcquire = true;
}
AliasAnalysis::Location LoadLoc = AA->getLocation(LI);
@ -545,12 +541,12 @@ getPointerDependencyFrom(const AliasAnalysis::Location &MemLoc, bool isLoad,
if (StoreInst *SI = dyn_cast<StoreInst>(Inst)) {
// Atomic stores have complications involved.
// A Monotonic store is OK if the query inst is itself not atomic.
// A Release (or higher) store further requires that no acquire load
// has been seen.
// FIXME: This is overly conservative.
if (!SI->isUnordered()) {
if (!QueryInst)
return MemDepResult::getClobber(SI);
if (SI->getOrdering() != Monotonic)
return MemDepResult::getClobber(SI);
if (auto *QueryLI = dyn_cast<LoadInst>(QueryInst)) {
if (!QueryLI->isSimple())
return MemDepResult::getClobber(SI);
@ -560,9 +556,6 @@ getPointerDependencyFrom(const AliasAnalysis::Location &MemLoc, bool isLoad,
} else if (QueryInst->mayReadOrWriteMemory()) {
return MemDepResult::getClobber(SI);
}
if (HasSeenAcquire && isAtLeastRelease(SI->getOrdering()))
return MemDepResult::getClobber(SI);
}
// FIXME: this is overly conservative.

46
test/Transforms/DeadStoreElimination/atomic.ll
View File

@ -23,28 +23,6 @@ define void @test1() {
ret void
}
; DSE across seq_cst load (allowed)
define i32 @test2() {
; CHECK-LABEL: test2
; CHECK-NOT: store i32 0
; CHECK: store i32 1
store i32 0, i32* @x
%x = load atomic i32, i32* @y seq_cst, align 4
store i32 1, i32* @x
ret i32 %x
}
; DSE across seq_cst store (allowed)
define void @test3() {
; CHECK-LABEL: test3
; CHECK-NOT: store i32 0
; CHECK: store atomic i32 2
store i32 0, i32* @x
store atomic i32 2, i32* @y seq_cst, align 4
store i32 1, i32* @x
ret void
}
; DSE remove unordered store (allowed)
define void @test4() {
; CHECK-LABEL: test4
@ -141,30 +119,6 @@ define void @test12() {
ret void
}
; DSE is allowed across a pair of an atomic read and then write.
define i32 @test13() {
; CHECK-LABEL: test13
; CHECK-NOT: store i32 0
; CHECK: store i32 1
store i32 0, i32* @x
%x = load atomic i32, i32* @y seq_cst, align 4
store atomic i32 %x, i32* @y seq_cst, align 4
store i32 1, i32* @x
ret i32 %x
}
; Same if it is acquire-release instead of seq_cst/seq_cst
define i32 @test14() {
; CHECK-LABEL: test14
; CHECK-NOT: store i32 0
; CHECK: store i32 1
store i32 0, i32* @x
%x = load atomic i32, i32* @y acquire, align 4
store atomic i32 %x, i32* @y release, align 4
store i32 1, i32* @x
ret i32 %x
}
; But DSE is not allowed across a release-acquire pair.
define i32 @test15() {
; CHECK-LABEL: test15

56
test/Transforms/GVN/atomic.ll
View File

@ -18,18 +18,6 @@ entry:
ret i32 %z
}
; GVN across seq_cst store (allowed)
define i32 @test2() nounwind uwtable ssp {
; CHECK-LABEL: test2
; CHECK: add i32 %x, %x
entry:
%x = load i32, i32* @y
store atomic i32 %x, i32* @x seq_cst, align 4
%y = load i32, i32* @y
%z = add i32 %x, %y
ret i32 %z
}
; GVN across unordered load (allowed)
define i32 @test3() nounwind uwtable ssp {
; CHECK-LABEL: test3
@ -43,20 +31,6 @@ entry:
ret i32 %b
}
; GVN across acquire load (allowed as the original load was not atomic)
define i32 @test4() nounwind uwtable ssp {
; CHECK-LABEL: test4
; CHECK: load atomic i32, i32* @x
; CHECK-NOT: load i32, i32* @y
entry:
%x = load i32, i32* @y
%y = load atomic i32, i32* @x seq_cst, align 4
%x2 = load i32, i32* @y
%x3 = add i32 %x, %x2
%y2 = add i32 %y, %x3
ret i32 %y2
}
; GVN load to unordered load (allowed)
define i32 @test5() nounwind uwtable ssp {
; CHECK-LABEL: test5
@ -92,19 +66,6 @@ entry:
ret i32 %z
}
; GVN across acquire-release pair (allowed)
define i32 @test8() nounwind uwtable ssp {
; CHECK-LABEL: test8
; CHECK: add i32 %x, %x
entry:
%x = load i32, i32* @y
%w = load atomic i32, i32* @x acquire, align 4
store atomic i32 %x, i32* @x release, align 4
%y = load i32, i32* @y
%z = add i32 %x, %y
ret i32 %z
}
; GVN across monotonic store (allowed)
define i32 @test9() nounwind uwtable ssp {
; CHECK-LABEL: test9
@ -129,3 +90,20 @@ entry:
ret i32 %z
}
define i32 @PR22708(i1 %flag) {
; CHECK-LABEL: PR22708
entry:
br i1 %flag, label %if.then, label %if.end
if.then:
store i32 43, i32* @y, align 4
; CHECK: store i32 43, i32* @y, align 4
br label %if.end
if.end:
load atomic i32, i32* @x acquire, align 4
%load = load i32, i32* @y, align 4
; CHECK: load atomic i32, i32* @x acquire, align 4
; CHECK: load i32, i32* @y, align 4
ret i32 %load
}