[NewGVN] Fix a consistent order for phi nodes operands.

The way we currently define congruency for two PHIExpression(s) is:

1) The operands to the phi functions are congruent
2) The PHIs are defined in the same BasicBlock.

NewGVN works under the assumption that phi operands are in predecessor
order, or at least in some consistent order. OTOH, is valid IR:

patatino:
  %meh = phi i16 [ %0, %winky ], [ %conv1, %tinky ]
  %banana = phi i16 [ %0, %tinky ], [ %conv1, %winky ]
  br label %end

and the in-memory representations of the two SSA registers have an
inconsistent order. This violation of NewGVN assumptions results into
two PHIs found congruent when they're not. While we think it's useful
to have always a consistent order enforced, let's fix this in NewGVN
sorting uses in predecessor order before creating a PHI expression.

Differential Revision:  https://reviews.llvm.org/D32990

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@302552 91177308-0d34-0410-b5e6-96231b3b80d8
This commit is contained in:
Davide Italiano 2017-05-09 16:58:28 +00:00
parent 80f6556e8a
commit 9689ee1fa3
2 changed files with 61 additions and 7 deletions

View File

@ -730,23 +730,35 @@ PHIExpression *NewGVN::createPHIExpression(Instruction *I, bool &HasBackedge,
unsigned PHIRPO = RPOOrdering.lookup(DT->getNode(PHIBlock));
// NewGVN assumes the operands of a PHI node are in a consistent order across
// PHIs. LLVM doesn't seem to always guarantee this. While we need to fix
// this in LLVM at some point we don't want GVN to find wrong congruences.
// Therefore, here we sort uses in predecessor order.
SmallVector<const Use *, 4> PHIOperands;
for (const Use &U : PN->operands())
PHIOperands.push_back(&U);
std::sort(PHIOperands.begin(), PHIOperands.end(),
[&](const Use *U1, const Use *U2) {
return PN->getIncomingBlock(*U1) < PN->getIncomingBlock(*U2);
});
// Filter out unreachable phi operands.
auto Filtered = make_filter_range(PN->operands(), [&](const Use &U) {
return ReachableEdges.count({PN->getIncomingBlock(U), PHIBlock});
auto Filtered = make_filter_range(PHIOperands, [&](const Use *U) {
return ReachableEdges.count({PN->getIncomingBlock(*U), PHIBlock});
});
std::transform(Filtered.begin(), Filtered.end(), op_inserter(E),
[&](const Use &U) -> Value * {
auto *BB = PN->getIncomingBlock(U);
[&](const Use *U) -> Value * {
auto *BB = PN->getIncomingBlock(*U);
auto *DTN = DT->getNode(BB);
if (RPOOrdering.lookup(DTN) >= PHIRPO)
HasBackedge = true;
AllConstant &= isa<UndefValue>(U) || isa<Constant>(U);
AllConstant &= isa<UndefValue>(*U) || isa<Constant>(*U);
// Don't try to transform self-defined phis.
if (U == PN)
if (*U == PN)
return PN;
return lookupOperandLeader(U);
return lookupOperandLeader(*U);
});
return E;
}

View File

@ -0,0 +1,42 @@
; PR32952: Don't erroneously consider congruent two phi nodes which
; have the same arguments but different incoming edges.
; RUN: opt -newgvn -S %s | FileCheck %s
@a = common global i16 0, align 2
@.str = private unnamed_addr constant [4 x i8] c"%d\0A\00", align 1
define i32 @tinkywinky() {
entry:
%0 = load i16, i16* @a, align 2
%conv = sext i16 %0 to i32
%neg = xor i32 %conv, -1
%conv1 = trunc i32 %neg to i16
%conv3 = zext i16 %conv1 to i32
%cmp = icmp slt i32 %conv, %conv3
br i1 %cmp, label %tinky, label %winky
tinky:
store i16 2, i16* @a, align 2
br label %patatino
winky:
br label %patatino
patatino:
; CHECK: %meh = phi i16 [ %0, %winky ], [ %conv1, %tinky ]
; CHECK: %banana = phi i16 [ %0, %tinky ], [ %conv1, %winky ]
%meh = phi i16 [ %0, %winky ], [ %conv1, %tinky ]
%banana = phi i16 [ %0, %tinky ], [ %conv1, %winky ]
br label %end
end:
; CHECK: %promoted = zext i16 %banana to i32
; CHECK: %other = zext i16 %meh to i32
%promoted = zext i16 %banana to i32
%other = zext i16 %meh to i32
%first = tail call i32 (i8*, ...) @printf(i8* getelementptr inbounds ([4 x i8], [4 x i8]* @.str, i64 0, i64 0), i32 %promoted)
%second = tail call i32 (i8*, ...) @printf(i8* getelementptr inbounds ([4 x i8], [4 x i8]* @.str, i64 0, i64 0), i32 %other)
ret i32 0
}
declare i32 @printf(i8*, ...)