summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
Diffstat
-rw-r--r--llvm/lib/Transforms/Scalar/GVN.cpp84
-rw-r--r--llvm/test/Transforms/GVN/PRE/hoist-loads.ll53
-rw-r--r--llvm/test/Transforms/GVN/PRE/pre-single-pred.ll14
3 files changed, 36 insertions, 115 deletions
diff --git a/llvm/lib/Transforms/Scalar/GVN.cpp b/llvm/lib/Transforms/Scalar/GVN.cpp
index d815160a4bd..b6591531265 100644
--- a/llvm/lib/Transforms/Scalar/GVN.cpp
+++ b/llvm/lib/Transforms/Scalar/GVN.cpp
@@ -122,84 +122,69 @@ template <> struct DenseMapInfo<GVN::Expression> {
/// location of the instruction from which it was formed.
struct llvm::gvn::AvailableValue {
enum ValType {
- SimpleVal, // A simple offsetted value that is accessed.
- LoadVal, // A value produced by a load.
- MemIntrinVal, // A memory intrinsic which is loaded from.
- UndefVal, // A UndefValue representing a value from dead block (which
- // is not yet physically removed from the CFG).
- CreateLoadVal // A duplicate load can be created higher up in the CFG that
- // will eliminate this one.
+ SimpleVal, // A simple offsetted value that is accessed.
+ LoadVal, // A value produced by a load.
+ MemIntrin, // A memory intrinsic which is loaded from.
+ UndefVal // A UndefValue representing a value from dead block (which
+ // is not yet physically removed from the CFG).
};
/// V - The value that is live out of the block.
- std::pair<Value *, ValType> Val;
+ PointerIntPair<Value *, 2, ValType> Val;
/// Offset - The byte offset in Val that is interesting for the load query.
unsigned Offset;
static AvailableValue get(Value *V, unsigned Offset = 0) {
AvailableValue Res;
- Res.Val.first = V;
- Res.Val.second = SimpleVal;
+ Res.Val.setPointer(V);
+ Res.Val.setInt(SimpleVal);
Res.Offset = Offset;
return Res;
}
static AvailableValue getMI(MemIntrinsic *MI, unsigned Offset = 0) {
AvailableValue Res;
- Res.Val.first = MI;
- Res.Val.second = MemIntrinVal;
+ Res.Val.setPointer(MI);
+ Res.Val.setInt(MemIntrin);
Res.Offset = Offset;
return Res;
}
- static AvailableValue getCreateLoad(LoadInst *LI) {
- AvailableValue Res;
- Res.Val.first = LI;
- Res.Val.second = CreateLoadVal;
- return Res;
- }
-
static AvailableValue getLoad(LoadInst *LI, unsigned Offset = 0) {
AvailableValue Res;
- Res.Val.first = LI;
- Res.Val.second = LoadVal;
+ Res.Val.setPointer(LI);
+ Res.Val.setInt(LoadVal);
Res.Offset = Offset;
return Res;
}
static AvailableValue getUndef() {
AvailableValue Res;
- Res.Val.first = nullptr;
- Res.Val.second = UndefVal;
+ Res.Val.setPointer(nullptr);
+ Res.Val.setInt(UndefVal);
Res.Offset = 0;
return Res;
}
- bool isSimpleValue() const { return Val.second == SimpleVal; }
- bool isCoercedLoadValue() const { return Val.second == LoadVal; }
- bool isMemIntrinValue() const { return Val.second == MemIntrinVal; }
- bool isUndefValue() const { return Val.second == UndefVal; }
- bool isCreateLoadValue() const { return Val.second == CreateLoadVal; }
-
- LoadInst *getCreateLoadValue() const {
- assert(isCreateLoadValue() && "Wrong accessor");
- return cast<LoadInst>(Val.first);
- }
+ bool isSimpleValue() const { return Val.getInt() == SimpleVal; }
+ bool isCoercedLoadValue() const { return Val.getInt() == LoadVal; }
+ bool isMemIntrinValue() const { return Val.getInt() == MemIntrin; }
+ bool isUndefValue() const { return Val.getInt() == UndefVal; }
Value *getSimpleValue() const {
assert(isSimpleValue() && "Wrong accessor");
- return Val.first;
+ return Val.getPointer();
}
LoadInst *getCoercedLoadValue() const {
assert(isCoercedLoadValue() && "Wrong accessor");
- return cast<LoadInst>(Val.first);
+ return cast<LoadInst>(Val.getPointer());
}
MemIntrinsic *getMemIntrinValue() const {
assert(isMemIntrinValue() && "Wrong accessor");
- return cast<MemIntrinsic>(Val.first);
+ return cast<MemIntrinsic>(Val.getPointer());
}
/// Emit code at the specified insertion point to adjust the value defined
@@ -1180,11 +1165,7 @@ Value *AvailableValue::MaterializeAdjustedValue(LoadInst *LI,
Value *Res;
Type *LoadTy = LI->getType();
const DataLayout &DL = LI->getModule()->getDataLayout();
- if (isCreateLoadValue()) {
- Instruction *I = getCreateLoadValue()->clone();
- I->insertBefore(InsertPt);
- Res = I;
- } else if (isSimpleValue()) {
+ if (isSimpleValue()) {
Res = getSimpleValue();
if (Res->getType() != LoadTy) {
Res = GetStoreValueForLoad(Res, Offset, LoadTy, InsertPt, DL);
@@ -1372,7 +1353,7 @@ void GVN::AnalyzeLoadAvailability(LoadInst *LI, LoadDepVect &Deps,
continue;
}
- if (!DepInfo.isDef() && !DepInfo.isClobber() && !DepInfo.isNonFuncLocal()) {
+ if (!DepInfo.isDef() && !DepInfo.isClobber()) {
UnavailableBlocks.push_back(DepBB);
continue;
}
@@ -1383,25 +1364,12 @@ void GVN::AnalyzeLoadAvailability(LoadInst *LI, LoadDepVect &Deps,
Value *Address = Deps[i].getAddress();
AvailableValue AV;
- // TODO: We can use anything where the operands are available, and we should
- // learn to recreate operands in other blocks if they are available.
- // Because we don't have the infrastructure in our PRE, we restrict this to
- // global values, because we know the operands are always available.
- if (DepInfo.isNonFuncLocal()) {
- if (isSafeToSpeculativelyExecute(LI) &&
- isa<GlobalValue>(LI->getPointerOperand())) {
- AV = AvailableValue::getCreateLoad(LI);
- ValuesPerBlock.push_back(AvailableValueInBlock::get(
- &LI->getParent()->getParent()->getEntryBlock(), std::move(AV)));
- } else
- UnavailableBlocks.push_back(DepBB);
-
- } else if (AnalyzeLoadAvailability(LI, DepInfo, Address, AV)) {
+ if (AnalyzeLoadAvailability(LI, DepInfo, Address, AV)) {
// subtlety: because we know this was a non-local dependency, we know
// it's safe to materialize anywhere between the instruction within
// DepInfo and the end of it's block.
- ValuesPerBlock.push_back(
- AvailableValueInBlock::get(DepBB, std::move(AV)));
+ ValuesPerBlock.push_back(AvailableValueInBlock::get(DepBB,
+ std::move(AV)));
} else {
UnavailableBlocks.push_back(DepBB);
}
diff --git a/llvm/test/Transforms/GVN/PRE/hoist-loads.ll b/llvm/test/Transforms/GVN/PRE/hoist-loads.ll
deleted file mode 100644
index 493a288d6a7..00000000000
--- a/llvm/test/Transforms/GVN/PRE/hoist-loads.ll
+++ /dev/null
@@ -1,53 +0,0 @@
-; RUN: opt -gvn %s -S -o - | FileCheck %s
-
-target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
-target triple = "x86_64-unknown-linux-gnu"
-
-; Check that the loads of @aaa, @bbb and @ccc are hoisted.
-; CHECK-LABEL: define void @foo
-; CHECK-NEXT: %2 = load i32, i32* @ccc, align 4
-; CHECK-NEXT: %3 = load i32, i32* @bbb, align 4
-; CHECK-NEXT: %4 = load i32, i32* @aaa, align 4
-
-@aaa = local_unnamed_addr global i32 10, align 4
-@bbb = local_unnamed_addr global i32 20, align 4
-@ccc = local_unnamed_addr global i32 30, align 4
-
-define void @foo(i32* nocapture readonly) local_unnamed_addr {
- br label %2
-
- %.0 = phi i32* [ %0, %1 ], [ %3, %22 ]
- %3 = getelementptr inbounds i32, i32* %.0, i64 1
- %4 = load i32, i32* %.0, align 4
- %5 = load i32, i32* @ccc, align 4
- %6 = icmp sgt i32 %4, %5
- br i1 %6, label %7, label %10
-
- %8 = load i32, i32* @bbb, align 4
- %9 = add nsw i32 %8, %4
- store i32 %9, i32* @bbb, align 4
- br label %10
-
- %11 = load i32, i32* @bbb, align 4
- %12 = icmp sgt i32 %4, %11
- br i1 %12, label %13, label %16
-
- %14 = load i32, i32* @aaa, align 4
- %15 = add nsw i32 %14, %4
- store i32 %15, i32* @aaa, align 4
- br label %16
-
- %17 = load i32, i32* @aaa, align 4
- %18 = icmp sgt i32 %4, %17
- br i1 %18, label %19, label %22
-
- %20 = load i32, i32* @ccc, align 4
- %21 = add nsw i32 %20, %4
- store i32 %21, i32* @ccc, align 4
- br label %22
-
- %23 = icmp slt i32 %4, 0
- br i1 %23, label %24, label %2
-
- ret void
-}
diff --git a/llvm/test/Transforms/GVN/PRE/pre-single-pred.ll b/llvm/test/Transforms/GVN/PRE/pre-single-pred.ll
index bd02594a40e..0df45cf5c1c 100644
--- a/llvm/test/Transforms/GVN/PRE/pre-single-pred.ll
+++ b/llvm/test/Transforms/GVN/PRE/pre-single-pred.ll
@@ -1,9 +1,16 @@
; RUN: opt < %s -gvn -enable-load-pre -S | FileCheck %s
+; This testcase assumed we'll PRE the load into %for.cond, but we don't actually
+; verify that doing so is safe. If there didn't _happen_ to be a load in
+; %for.end, we would actually be lengthening the execution on some paths, and
+; we were never actually checking that case. Now we actually do perform some
+; conservative checking to make sure we don't make paths longer, but we don't
+; currently get this case, which we got lucky on previously.
+;
+; Now that that faulty assumption is corrected, test that we DON'T incorrectly
+; hoist the load. Doing the right thing for the wrong reasons is still a bug.
@p = external global i32
define i32 @f(i32 %n) nounwind {
-; CHECK: entry:
-; CHECK-NEXT: %0 = load i32, i32* @p
entry:
br label %for.cond
@@ -15,9 +22,8 @@ for.cond: ; preds = %for.inc, %entry
for.cond.for.end_crit_edge: ; preds = %for.cond
br label %for.end
-; The load of @p should be hoisted into the entry block.
; CHECK: for.body:
-; CHECK-NEXT: %dec = add i32 %tmp3, -1
+; CHECK-NEXT: %tmp3 = load i32, i32* @p
for.body: ; preds = %for.cond
%tmp3 = load i32, i32* @p ; <i32> [#uses=1]
%dec = add i32 %tmp3, -1 ; <i32> [#uses=2]
OpenPOWER on IntegriCloud