Skip to content
This repository was archived by the owner on Mar 28, 2020. It is now read-only.

Commit 8ac7be5

Browse files
majnemermajnemer
committed
[MergedLoadStoreMotion] Don't transform across may-throw calls
It is unsafe to hoist a load before a function call which may throw, the throw might prevent a pointer dereference. Likewise, it is unsafe to sink a store after a call which may throw. The caller might be able to observe the difference. This fixes PR27858. git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@270828 91177308-0d34-0410-b5e6-96231b3b80d8
1 parent b09e1d3 commit 8ac7be5

File tree

3 files changed

+90
-25
lines changed

3 files changed

+90
-25
lines changed

‎lib/Transforms/Scalar/MergedLoadStoreMotion.cpp

Lines changed: 32 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,6 @@
7979
#include "llvm/Analysis/Loads.h"
8080
#include "llvm/Analysis/MemoryBuiltins.h"
8181
#include "llvm/Analysis/MemoryDependenceAnalysis.h"
82-
#include "llvm/Analysis/TargetLibraryInfo.h"
8382
#include "llvm/IR/Metadata.h"
8483
#include "llvm/IR/PatternMatch.h"
8584
#include "llvm/Support/Debug.h"
@@ -114,7 +113,6 @@ class MergedLoadStoreMotion : public FunctionPass {
114113
// This transformation requires dominator postdominator info
115114
void getAnalysisUsage(AnalysisUsage &AU) const override {
116115
AU.setPreservesCFG();
117-
AU.addRequired<TargetLibraryInfoWrapperPass>();
118116
AU.addRequired<AAResultsWrapperPass>();
119117
AU.addPreserved<GlobalsAAWrapperPass>();
120118
AU.addPreserved<MemoryDependenceWrapperPass>();
@@ -130,9 +128,9 @@ class MergedLoadStoreMotion : public FunctionPass {
130128
BasicBlock *getDiamondTail(BasicBlock *BB);
131129
bool isDiamondHead(BasicBlock *BB);
132130
// Routines for hoisting loads
133-
bool isLoadHoistBarrierInRange(const Instruction& Start,
134-
const Instruction& End,
135-
LoadInst* LI);
131+
bool isLoadHoistBarrierInRange(const Instruction &Start,
132+
const Instruction &End, LoadInst *LI,
133+
bool SafeToLoadUnconditionally);
136134
LoadInst *canHoistFromBlock(BasicBlock *BB, LoadInst *LI);
137135
void hoistInstruction(BasicBlock *BB, Instruction *HoistCand,
138136
Instruction *ElseInst);
@@ -166,7 +164,6 @@ FunctionPass *llvm::createMergedLoadStoreMotionPass() {
166164
INITIALIZE_PASS_BEGIN(MergedLoadStoreMotion, "mldst-motion",
167165
"MergedLoadStoreMotion", false, false)
168166
INITIALIZE_PASS_DEPENDENCY(MemoryDependenceWrapperPass)
169-
INITIALIZE_PASS_DEPENDENCY(TargetLibraryInfoWrapperPass)
170167
INITIALIZE_PASS_DEPENDENCY(AAResultsWrapperPass)
171168
INITIALIZE_PASS_DEPENDENCY(GlobalsAAWrapperPass)
172169
INITIALIZE_PASS_END(MergedLoadStoreMotion, "mldst-motion",
@@ -229,9 +226,14 @@ bool MergedLoadStoreMotion::isDiamondHead(BasicBlock *BB) {
229226
/// being loaded or protect against the load from happening
230227
/// it is considered a hoist barrier.
231228
///
232-
bool MergedLoadStoreMotion::isLoadHoistBarrierInRange(const Instruction& Start,
233-
const Instruction& End,
234-
LoadInst* LI) {
229+
bool MergedLoadStoreMotion::isLoadHoistBarrierInRange(
230+
const Instruction &Start, const Instruction &End, LoadInst *LI,
231+
bool SafeToLoadUnconditionally) {
232+
if (!SafeToLoadUnconditionally)
233+
for (const Instruction &Inst :
234+
make_range(Start.getIterator(), End.getIterator()))
235+
if (Inst.mayThrow())
236+
return true;
235237
MemoryLocation Loc = MemoryLocation::get(LI);
236238
return AA->canInstructionRangeModRef(Start, End, Loc, MRI_Mod);
237239
}
@@ -245,23 +247,28 @@ bool MergedLoadStoreMotion::isLoadHoistBarrierInRange(const Instruction& Start,
245247
///
246248
LoadInst *MergedLoadStoreMotion::canHoistFromBlock(BasicBlock *BB1,
247249
LoadInst *Load0) {
248-
250+
BasicBlock *BB0 = Load0->getParent();
251+
BasicBlock *Head = BB0->getSinglePredecessor();
252+
bool SafeToLoadUnconditionally = isSafeToLoadUnconditionally(
253+
Load0->getPointerOperand(), Load0->getAlignment(),
254+
Load0->getModule()->getDataLayout(),
255+
/*ScanFrom=*/Head->getTerminator());
249256
for (BasicBlock::iterator BBI = BB1->begin(), BBE = BB1->end(); BBI != BBE;
250257
++BBI) {
251258
Instruction *Inst = &*BBI;
252259

253260
// Only merge and hoist loads when their result in used only in BB
254-
if (!isa<LoadInst>(Inst) || Inst->isUsedOutsideOfBlock(BB1))
261+
auto *Load1 = dyn_cast<LoadInst>(Inst);
262+
if (!Load1 || Inst->isUsedOutsideOfBlock(BB1))
255263
continue;
256264

257-
auto *Load1 = cast<LoadInst>(Inst);
258-
BasicBlock *BB0 = Load0->getParent();
259-
260265
MemoryLocation Loc0 = MemoryLocation::get(Load0);
261266
MemoryLocation Loc1 = MemoryLocation::get(Load1);
262267
if (AA->isMustAlias(Loc0, Loc1) && Load0->isSameOperationAs(Load1) &&
263-
!isLoadHoistBarrierInRange(BB1->front(), *Load1, Load1) &&
264-
!isLoadHoistBarrierInRange(BB0->front(), *Load0, Load0)) {
268+
!isLoadHoistBarrierInRange(BB1->front(), *Load1, Load1,
269+
SafeToLoadUnconditionally) &&
270+
!isLoadHoistBarrierInRange(BB0->front(), *Load0, Load0,
271+
SafeToLoadUnconditionally)) {
265272
return Load1;
266273
}
267274
}
@@ -387,6 +394,10 @@ bool MergedLoadStoreMotion::mergeLoads(BasicBlock *BB) {
387394
bool MergedLoadStoreMotion::isStoreSinkBarrierInRange(const Instruction &Start,
388395
const Instruction &End,
389396
MemoryLocation Loc) {
397+
for (const Instruction &Inst :
398+
make_range(Start.getIterator(), End.getIterator()))
399+
if (Inst.mayThrow())
400+
return true;
390401
return AA->canInstructionRangeModRef(Start, End, Loc, MRI_ModRef);
391402
}
392403

@@ -403,18 +414,15 @@ StoreInst *MergedLoadStoreMotion::canSinkFromBlock(BasicBlock *BB1,
403414
RBI != RBE; ++RBI) {
404415
Instruction *Inst = &*RBI;
405416

406-
if (!isa<StoreInst>(Inst))
407-
continue;
408-
409-
StoreInst *Store1 = cast<StoreInst>(Inst);
417+
auto *Store1 = dyn_cast<StoreInst>(Inst);
418+
if (!Store1)
419+
continue;
410420

411421
MemoryLocation Loc0 = MemoryLocation::get(Store0);
412422
MemoryLocation Loc1 = MemoryLocation::get(Store1);
413423
if (AA->isMustAlias(Loc0, Loc1) && Store0->isSameOperationAs(Store1) &&
414-
!isStoreSinkBarrierInRange(*(std::next(BasicBlock::iterator(Store1))),
415-
BB1->back(), Loc1) &&
416-
!isStoreSinkBarrierInRange(*(std::next(BasicBlock::iterator(Store0))),
417-
BB0->back(), Loc0)) {
424+
!isStoreSinkBarrierInRange(*Store1->getNextNode(), BB1->back(), Loc1) &&
425+
!isStoreSinkBarrierInRange(*Store0->getNextNode(), BB0->back(), Loc0)) {
418426
return Store1;
419427
}
420428
}

‎test/Transforms/InstMerge/exceptions.ll

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
; RUN: opt -basicaa -memdep -mldst-motion -S < %s | FileCheck %s
2+
target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
3+
target triple = "x86_64-unknown-linux-gnu"
4+
5+
@r = common global i32 0, align 4
6+
@s = common global i32 0, align 4
7+
8+
; CHECK-LABEL: define void @test1(
9+
define void @test1(i1 %cmp, i32* noalias %p) {
10+
entry:
11+
br i1 %cmp, label %if.then, label %if.else
12+
13+
if.then: ; preds = %entry
14+
call void @may_throw()
15+
%arrayidx = getelementptr inbounds i32, i32* %p, i64 1
16+
%0 = load i32, i32* %arrayidx, align 4
17+
store i32 %0, i32* @r, align 4
18+
br label %if.end
19+
; CHECK: call void @may_throw()
20+
; CHECK-NEXT: %[[gep:.*]] = getelementptr inbounds i32, i32* %p, i64 1
21+
; CHECK-NEXT: %[[load:.*]] = load i32, i32* %[[gep]], align 4
22+
; CHECK-NEXT: store i32 %[[load]], i32* @r, align 4
23+
24+
if.else: ; preds = %entry
25+
%arrayidx1 = getelementptr inbounds i32, i32* %p, i64 1
26+
%1 = load i32, i32* %arrayidx1, align 4
27+
store i32 %1, i32* @s, align 4
28+
br label %if.end
29+
30+
if.end: ; preds = %if.else, %if.then
31+
ret void
32+
}
33+
34+
; CHECK-LABEL: define void @test2(
35+
define void @test2(i1 %cmp, i32* noalias %p) {
36+
entry:
37+
br i1 %cmp, label %if.then, label %if.else
38+
39+
if.then: ; preds = %entry
40+
%arrayidx = getelementptr inbounds i32, i32* %p, i64 1
41+
store i32 1, i32* %arrayidx, align 4
42+
call void @may_throw()
43+
; CHECK: %[[gep:.*]] = getelementptr inbounds i32, i32* %p, i64 1
44+
; CHECK-NEXT: store i32 1, i32* %[[gep]], align 4
45+
; CHECK-NEXT: call void @may_throw()
46+
br label %if.end
47+
48+
if.else: ; preds = %entry
49+
%arrayidx1 = getelementptr inbounds i32, i32* %p, i64 1
50+
store i32 2, i32* %arrayidx1, align 4
51+
br label %if.end
52+
53+
if.end: ; preds = %if.else, %if.then
54+
ret void
55+
}
56+
57+
declare void @may_throw()

‎test/Transforms/InstMerge/st_sink_no_barrier_call.ll

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ if.else: ; preds = %entry
3333
%p3 = getelementptr inbounds %struct.node, %struct.node* %node.017, i32 0, i32 6
3434
; CHECK-NOT: store i32
3535
store i32 %add, i32* %p3, align 4
36-
call i32 @foo(i32 5) ;not a barrier
36+
call i32 @foo(i32 5) nounwind ;not a barrier
3737
br label %if.end
3838

3939
; CHECK: if.end

0 commit comments

Comments
 (0)