[PredicateInfo] Replace pointer comparisons with deterministic compares.

Currently there are a few pointer comparisons in ValueDFS_Compare, which
can cause non-deterministic ordering when materializing values. There
are 2 cases this patch fixes:

1. Order defs before uses used to compare pointers, which guarantees
   defs before uses, but causes non-deterministic ordering between 2
   uses or 2 defs, depending on the allocation order. By converting the
   pointers to booleans, we can circumvent that problem.

2. comparePHIRelated was comparing the basic block pointers of edges,
   which also results in a non-deterministic order and is also not
   really meaningful for ordering. By ordering by their destination DFS
   numbers we guarantee a deterministic order.

For the example below, we can end up with 2 different uselist orderings,
when running `opt -mem2reg -ipsccp` hundreds of times. Because the
non-determinism is caused by allocation ordering, we cannot reproduce it
with ipsccp alone.

    declare i32 @hoge() local_unnamed_addr #0

    define dso_local i32 @ham(i8* %arg, i8* %arg1) #0 {
    bb:
      %tmp = alloca i32
      %tmp2 = alloca i32, align 4
      br label %bb19

    bb4:                                              ; preds = %bb20
      br label %bb6

    bb6:                                              ; preds = %bb4
      %tmp7 = call i32 @hoge()
      store i32 %tmp7, i32* %tmp
      %tmp8 = load i32, i32* %tmp
      %tmp9 = icmp eq i32 %tmp8, 912730082
      %tmp10 = load i32, i32* %tmp
      br i1 %tmp9, label %bb11, label %bb16

    bb11:                                             ; preds = %bb6
      unreachable

    bb13:                                             ; preds = %bb20
      br label %bb14

    bb14:                                             ; preds = %bb13
      %tmp15 = load i32, i32* %tmp
      br label %bb16

    bb16:                                             ; preds = %bb14, %bb6
      %tmp17 = phi i32 [ %tmp10, %bb6 ], [ 0, %bb14 ]
      br label %bb19

    bb18:                                             ; preds = %bb20
      unreachable

    bb19:                                             ; preds = %bb16, %bb
      br label %bb20

    bb20:                                             ; preds = %bb19
      indirectbr i8* null, [label %bb4, label %bb13, label %bb18]
    }

Reviewers: davide, efriedma

Reviewed By: efriedma

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

llvm-svn: 367049
This commit is contained in:
Florian Hahn 2019-07-25 20:48:13 +00:00
parent 568bb7eeb6
commit c74808b914
2 changed files with 116 additions and 9 deletions

View File

@ -125,8 +125,10 @@ static bool valueComesBefore(OrderedInstructions &OI, const Value *A,
// necessary to compare uses/defs in the same block. Doing so allows us to walk
// the minimum number of instructions necessary to compute our def/use ordering.
struct ValueDFS_Compare {
DominatorTree &DT;
OrderedInstructions &OI;
ValueDFS_Compare(OrderedInstructions &OI) : OI(OI) {}
ValueDFS_Compare(DominatorTree &DT, OrderedInstructions &OI)
: DT(DT), OI(OI) {}
bool operator()(const ValueDFS &A, const ValueDFS &B) const {
if (&A == &B)
@ -136,7 +138,9 @@ struct ValueDFS_Compare {
// comesbefore to see what the real ordering is, because they are in the
// same basic block.
bool SameBlock = std::tie(A.DFSIn, A.DFSOut) == std::tie(B.DFSIn, B.DFSOut);
assert((A.DFSIn != B.DFSIn || A.DFSOut == B.DFSOut) &&
"Equal DFS-in numbers imply equal out numbers");
bool SameBlock = A.DFSIn == B.DFSIn;
// We want to put the def that will get used for a given set of phi uses,
// before those phi uses.
@ -145,9 +149,11 @@ struct ValueDFS_Compare {
if (SameBlock && A.LocalNum == LN_Last && B.LocalNum == LN_Last)
return comparePHIRelated(A, B);
bool isADef = A.Def;
bool isBDef = B.Def;
if (!SameBlock || A.LocalNum != LN_Middle || B.LocalNum != LN_Middle)
return std::tie(A.DFSIn, A.DFSOut, A.LocalNum, A.Def, A.U) <
std::tie(B.DFSIn, B.DFSOut, B.LocalNum, B.Def, B.U);
return std::tie(A.DFSIn, A.LocalNum, isADef) <
std::tie(B.DFSIn, B.LocalNum, isBDef);
return localComesBefore(A, B);
}
@ -164,10 +170,35 @@ struct ValueDFS_Compare {
// For two phi related values, return the ordering.
bool comparePHIRelated(const ValueDFS &A, const ValueDFS &B) const {
auto &ABlockEdge = getBlockEdge(A);
auto &BBlockEdge = getBlockEdge(B);
// Now sort by block edge and then defs before uses.
return std::tie(ABlockEdge, A.Def, A.U) < std::tie(BBlockEdge, B.Def, B.U);
BasicBlock *ASrc, *ADest, *BSrc, *BDest;
std::tie(ASrc, ADest) = getBlockEdge(A);
std::tie(BSrc, BDest) = getBlockEdge(B);
#ifndef NDEBUG
// This function should only be used for values in the same BB, check that.
DomTreeNode *DomASrc = DT.getNode(ASrc);
DomTreeNode *DomBSrc = DT.getNode(BSrc);
assert(DomASrc->getDFSNumIn() == (unsigned)A.DFSIn &&
"DFS numbers for A should match the ones of the source block");
assert(DomBSrc->getDFSNumIn() == (unsigned)B.DFSIn &&
"DFS numbers for B should match the ones of the source block");
assert(A.DFSIn == B.DFSIn && "Values must be in the same block");
#endif
(void)ASrc;
(void)BSrc;
// Use DFS numbers to compare destination blocks, to guarantee a
// deterministic order.
DomTreeNode *DomADest = DT.getNode(ADest);
DomTreeNode *DomBDest = DT.getNode(BDest);
unsigned AIn = DomADest->getDFSNumIn();
unsigned BIn = DomBDest->getDFSNumIn();
bool isADef = A.Def;
bool isBDef = B.Def;
assert((!A.Def || !A.U) && (!B.Def || !B.U) &&
"Def and U cannot be set at the same time");
// Now sort by edge destination and then defs before uses.
return std::tie(AIn, isADef) < std::tie(BIn, isBDef);
}
// Get the definition of an instruction that occurs in the middle of a block.
@ -567,7 +598,7 @@ Value *PredicateInfo::materializeStack(unsigned int &Counter,
// TODO: Use this algorithm to perform fast single-variable renaming in
// promotememtoreg and memoryssa.
void PredicateInfo::renameUses(SmallVectorImpl<Value *> &OpsToRename) {
ValueDFS_Compare Compare(OI);
ValueDFS_Compare Compare(DT, OI);
// Compute liveness, and rename in O(uses) per Op.
for (auto *Op : OpsToRename) {
LLVM_DEBUG(dbgs() << "Visiting " << *Op << "\n");

View File

@ -0,0 +1,76 @@
; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
; RUN: opt -ipsccp -S %s | FileCheck %s
declare i32 @hoge()
define dso_local i32 @ham(i8* %arg, i8* %arg1) {
; CHECK-LABEL: @ham(
; CHECK-NEXT: bb:
; CHECK-NEXT: [[TMP:%.*]] = alloca i32
; CHECK-NEXT: [[TMP2:%.*]] = alloca i32, align 4
; CHECK-NEXT: br label [[BB19:%.*]]
; CHECK: bb4:
; CHECK-NEXT: br label [[BB6:%.*]]
; CHECK: bb6:
; CHECK-NEXT: [[TMP7:%.*]] = call i32 @hoge()
; CHECK-NEXT: store i32 [[TMP7]], i32* [[TMP]]
; CHECK-NEXT: [[TMP8:%.*]] = load i32, i32* [[TMP]]
; CHECK-NEXT: [[TMP9:%.*]] = icmp eq i32 [[TMP8]], 912730082
; CHECK-NEXT: [[TMP10:%.*]] = load i32, i32* [[TMP]]
; CHECK-NEXT: br i1 [[TMP9]], label [[BB11:%.*]], label [[BB16:%.*]]
; CHECK: bb11:
; CHECK-NEXT: unreachable
; CHECK: bb13:
; CHECK-NEXT: br label [[BB14:%.*]]
; CHECK: bb14:
; CHECK-NEXT: [[TMP15:%.*]] = load i32, i32* [[TMP]]
; CHECK-NEXT: br label [[BB16]]
; CHECK: bb16:
; CHECK-NEXT: [[TMP17:%.*]] = phi i32 [ [[TMP10]], [[BB6]] ], [ 0, [[BB14]] ]
; CHECK-NEXT: br label [[BB19]]
; CHECK: bb18:
; CHECK-NEXT: unreachable
; CHECK: bb19:
; CHECK-NEXT: br label [[BB20:%.*]]
; CHECK: bb20:
; CHECK-NEXT: indirectbr i8* null, [label [[BB4:%.*]], label [[BB13:%.*]], label %bb18]
;
bb:
%tmp = alloca i32
%tmp2 = alloca i32, align 4
br label %bb19
bb4: ; preds = %bb20
br label %bb6
bb6: ; preds = %bb4
%tmp7 = call i32 @hoge()
store i32 %tmp7, i32* %tmp
%tmp8 = load i32, i32* %tmp
%tmp9 = icmp eq i32 %tmp8, 912730082
%tmp10 = load i32, i32* %tmp
br i1 %tmp9, label %bb11, label %bb16
bb11: ; preds = %bb6
unreachable
bb13: ; preds = %bb20
br label %bb14
bb14: ; preds = %bb13
%tmp15 = load i32, i32* %tmp
br label %bb16
bb16: ; preds = %bb14, %bb6
%tmp17 = phi i32 [ %tmp10, %bb6 ], [ 0, %bb14 ]
br label %bb19
bb18: ; preds = %bb20
unreachable
bb19: ; preds = %bb16, %bb
br label %bb20
bb20: ; preds = %bb19
indirectbr i8* null, [label %bb4, label %bb13, label %bb18]
}