Skip to content

Commit

Permalink
Revert "[GuardWidening] Freeze the introduced use."
Browse files Browse the repository at this point in the history 
This reverts commit f4b2360.

The patch has no specific order in adding freeze instruction in the
entry basic block. It causes failure of CHECK like unit tests.
  • Loading branch information
Serguei Katkov committed Mar 29, 2023
1 parent da71cba commit cb2c510
Show file tree
Hide file tree
Showing 13 changed files with 125 additions and 334 deletions.
83 changes: 2 additions & 81 deletions llvm/lib/Transforms/Scalar/GuardWidening.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,6 @@ using namespace llvm;

STATISTIC(GuardsEliminated, "Number of eliminated guards");
STATISTIC(CondBranchEliminated, "Number of eliminated conditional branches");
STATISTIC(FreezeAdded, "Number of freeze instruction introduced");

static cl::opt<bool>
WidenBranchGuards("guard-widening-widen-branch-guards", cl::Hidden,
Expand Down Expand Up @@ -210,10 +209,6 @@ class GuardWideningImpl {
bool widenCondCommon(Value *Cond0, Value *Cond1, Instruction *InsertPt,
Value *&Result, bool InvertCondition);

/// Adds freeze to Orig and push it as far as possible very aggressively.
/// Also replaces all uses of frozen instruction with frozen version.
Value *freezeAndPush(Value *Orig, Instruction *InsertPt);

/// Represents a range check of the form \c Base + \c Offset u< \c Length,
/// with the constraint that \c Length is not negative. \c CheckInst is the
/// pre-existing instruction in the IR that computes the result of this range
Expand Down Expand Up @@ -563,80 +558,8 @@ void GuardWideningImpl::makeAvailableAt(Value *V, Instruction *Loc) const {
makeAvailableAt(Op, Loc);

Inst->moveBefore(Loc);
}

// Return Instruction before which we can insert freeze for the value V as close
// to def as possible. If there is no place to add freeze, return nullptr.
static Instruction *getFreezeInsertPt(Value *V, const DominatorTree &DT) {
auto *I = dyn_cast<Instruction>(V);
if (!I)
return &*DT.getRoot()->getFirstNonPHIOrDbgOrAlloca();

auto *Res = I->getInsertionPointAfterDef();
// If there is no place to add freeze - return nullptr.
if (!Res || !DT.dominates(I, Res))
return nullptr;

// If there is a User dominated by original I, then it should be dominated
// by Freeze instruction as well.
if (any_of(I->users(), [&](User *U) {
Instruction *User = cast<Instruction>(U);
return Res != User && DT.dominates(I, User) && !DT.dominates(Res, User);
}))
return nullptr;
return Res;
}

Value *GuardWideningImpl::freezeAndPush(Value *Orig, Instruction *InsertPt) {
if (isGuaranteedNotToBePoison(Orig, nullptr, InsertPt, &DT))
return Orig;
Instruction *InsertPtAtDef = getFreezeInsertPt(Orig, DT);
if (!InsertPtAtDef)
return new FreezeInst(Orig, "gw.freeze", InsertPt);
SmallSet<Value *, 16> Visited;
SmallVector<Value *, 16> Worklist;
SmallSet<Instruction *, 16> DropPoisonFlags;
SmallSet<Value *, 16> NeedFreeze;
Worklist.push_back(Orig);
while (!Worklist.empty()) {
Value *V = Worklist.pop_back_val();
if (!Visited.insert(V).second)
continue;

if (isGuaranteedNotToBePoison(V, nullptr, InsertPt, &DT))
continue;

Instruction *I = dyn_cast<Instruction>(V);
if (!I || canCreateUndefOrPoison(cast<Operator>(I),
/*ConsiderFlagsAndMetadata*/ false)) {
NeedFreeze.insert(V);
continue;
}
// Check all operands. If for any of them we cannot insert Freeze,
// stop here. Otherwise, iterate.
if (any_of(I->operands(), [&](Value *Op) {
return isa<Instruction>(Op) && !getFreezeInsertPt(Op, DT);
})) {
NeedFreeze.insert(I);
continue;
}
DropPoisonFlags.insert(I);
append_range(Worklist, I->operands());
}
for (Instruction *I : DropPoisonFlags)
I->dropPoisonGeneratingFlagsAndMetadata();

Value *Result = Orig;
for (Value *V : NeedFreeze) {
auto *FreezeInsertPt = getFreezeInsertPt(V, DT);
FreezeInst *FI = new FreezeInst(V, V->getName() + ".gw.fr", FreezeInsertPt);
++FreezeAdded;
if (V == Orig)
Result = FI;
V->replaceUsesWithIf(FI, [&](Use & U)->bool { return U.getUser() != FI; });
}

return Result;
// If we moved instruction before guard we must clean poison generating flags.
Inst->dropPoisonGeneratingFlags();
}

bool GuardWideningImpl::widenCondCommon(Value *Cond0, Value *Cond1,
Expand Down Expand Up @@ -698,7 +621,6 @@ bool GuardWideningImpl::widenCondCommon(Value *Cond0, Value *Cond1,
}
assert(Result && "Failed to find result value");
Result->setName("wide.chk");
Result = freezeAndPush(Result, InsertPt);
}
return true;
}
Expand All @@ -711,7 +633,6 @@ bool GuardWideningImpl::widenCondCommon(Value *Cond0, Value *Cond1,
makeAvailableAt(Cond1, InsertPt);
if (InvertCondition)
Cond1 = BinaryOperator::CreateNot(Cond1, "inverted", InsertPt);
Cond1 = freezeAndPush(Cond1, InsertPt);
Result = BinaryOperator::CreateAnd(Cond0, Cond1, "wide.chk", InsertPt);
}

Expand Down
15 changes: 5 additions & 10 deletions llvm/test/Transforms/GuardWidening/basic-loop.ll
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,11 @@ declare void @llvm.experimental.guard(i1,...)
define void @widen_within_loop(i1 %cond_0, i1 %cond_1, i1 %cond_2) {
; CHECK-LABEL: @widen_within_loop(
; CHECK-NEXT: entry:
; CHECK-NEXT: [[COND_2_GW_FR:%.*]] = freeze i1 [[COND_2:%.*]]
; CHECK-NEXT: [[COND_1_GW_FR:%.*]] = freeze i1 [[COND_1:%.*]]
; CHECK-NEXT: br label [[LOOP:%.*]]
; CHECK: loop:
; CHECK-NEXT: store i32 0, ptr @G, align 4
; CHECK-NEXT: [[WIDE_CHK:%.*]] = and i1 [[COND_0:%.*]], [[COND_1_GW_FR]]
; CHECK-NEXT: [[WIDE_CHK1:%.*]] = and i1 [[WIDE_CHK]], [[COND_2_GW_FR]]
; CHECK-NEXT: [[WIDE_CHK:%.*]] = and i1 [[COND_0:%.*]], [[COND_1:%.*]]
; CHECK-NEXT: [[WIDE_CHK1:%.*]] = and i1 [[WIDE_CHK]], [[COND_2:%.*]]
; CHECK-NEXT: call void (i1, ...) @llvm.experimental.guard(i1 [[WIDE_CHK1]]) [ "deopt"(i32 0) ]
; CHECK-NEXT: store i32 1, ptr @G, align 4
; CHECK-NEXT: store i32 2, ptr @G, align 4
Expand All @@ -40,11 +38,9 @@ loop:
define void @widen_into_preheader(i1 %cond_0, i1 %cond_1, i1 %cond_2) {
; CHECK-LABEL: @widen_into_preheader(
; CHECK-NEXT: entry:
; CHECK-NEXT: [[COND_2_GW_FR:%.*]] = freeze i1 [[COND_2:%.*]]
; CHECK-NEXT: [[COND_1_GW_FR:%.*]] = freeze i1 [[COND_1:%.*]]
; CHECK-NEXT: store i32 0, ptr @G, align 4
; CHECK-NEXT: [[WIDE_CHK:%.*]] = and i1 [[COND_0:%.*]], [[COND_1_GW_FR]]
; CHECK-NEXT: [[WIDE_CHK1:%.*]] = and i1 [[WIDE_CHK]], [[COND_2_GW_FR]]
; CHECK-NEXT: [[WIDE_CHK:%.*]] = and i1 [[COND_0:%.*]], [[COND_1:%.*]]
; CHECK-NEXT: [[WIDE_CHK1:%.*]] = and i1 [[WIDE_CHK]], [[COND_2:%.*]]
; CHECK-NEXT: call void (i1, ...) @llvm.experimental.guard(i1 [[WIDE_CHK1]]) [ "deopt"(i32 0) ]
; CHECK-NEXT: br label [[LOOP:%.*]]
; CHECK: loop:
Expand Down Expand Up @@ -106,9 +102,8 @@ exit:
define void @widen_over_common_exit_to_ph(i1 %cond_0, i1 %cond_1, i1 %cond_2) {
; CHECK-LABEL: @widen_over_common_exit_to_ph(
; CHECK-NEXT: entry:
; CHECK-NEXT: [[COND_2_GW_FR:%.*]] = freeze i1 [[COND_2:%.*]]
; CHECK-NEXT: store i32 0, ptr @G, align 4
; CHECK-NEXT: [[WIDE_CHK:%.*]] = and i1 [[COND_0:%.*]], [[COND_2_GW_FR]]
; CHECK-NEXT: [[WIDE_CHK:%.*]] = and i1 [[COND_0:%.*]], [[COND_2:%.*]]
; CHECK-NEXT: call void (i1, ...) @llvm.experimental.guard(i1 [[WIDE_CHK]]) [ "deopt"(i32 0) ]
; CHECK-NEXT: br label [[LOOP:%.*]]
; CHECK: loop:
Expand Down
24 changes: 8 additions & 16 deletions llvm/test/Transforms/GuardWidening/basic.ll
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,7 @@ declare void @llvm.experimental.guard(i1,...)
define void @f_0(i1 %cond_0, i1 %cond_1) {
; CHECK-LABEL: @f_0(
; CHECK-NEXT: entry:
; CHECK-NEXT: [[COND_1_GW_FR:%.*]] = freeze i1 [[COND_1:%.*]]
; CHECK-NEXT: [[WIDE_CHK:%.*]] = and i1 [[COND_0:%.*]], [[COND_1_GW_FR]]
; CHECK-NEXT: [[WIDE_CHK:%.*]] = and i1 [[COND_0:%.*]], [[COND_1:%.*]]
; CHECK-NEXT: call void (i1, ...) @llvm.experimental.guard(i1 [[WIDE_CHK]]) [ "deopt"() ]
; CHECK-NEXT: ret void
;
Expand All @@ -25,8 +24,7 @@ entry:
define void @f_1(i1 %cond_0, i1 %cond_1) {
; CHECK-LABEL: @f_1(
; CHECK-NEXT: entry:
; CHECK-NEXT: [[COND_1_GW_FR:%.*]] = freeze i1 [[COND_1:%.*]]
; CHECK-NEXT: [[WIDE_CHK:%.*]] = and i1 [[COND_0:%.*]], [[COND_1_GW_FR]]
; CHECK-NEXT: [[WIDE_CHK:%.*]] = and i1 [[COND_0:%.*]], [[COND_1:%.*]]
; CHECK-NEXT: call void (i1, ...) @llvm.experimental.guard(i1 [[WIDE_CHK]]) [ "deopt"() ]
; CHECK-NEXT: br i1 undef, label [[LEFT:%.*]], label [[RIGHT:%.*]]
; CHECK: left:
Expand Down Expand Up @@ -57,9 +55,8 @@ merge:
define void @f_2(i32 %a, i32 %b) {
; CHECK-LABEL: @f_2(
; CHECK-NEXT: entry:
; CHECK-NEXT: [[B_GW_FR:%.*]] = freeze i32 [[B:%.*]]
; CHECK-NEXT: [[COND_0:%.*]] = icmp ult i32 [[A:%.*]], 10
; CHECK-NEXT: [[COND_1:%.*]] = icmp ult i32 [[B_GW_FR]], 10
; CHECK-NEXT: [[COND_1:%.*]] = icmp ult i32 [[B:%.*]], 10
; CHECK-NEXT: [[WIDE_CHK:%.*]] = and i1 [[COND_0]], [[COND_1]]
; CHECK-NEXT: call void (i1, ...) @llvm.experimental.guard(i1 [[WIDE_CHK]]) [ "deopt"() ]
; CHECK-NEXT: br i1 undef, label [[LEFT:%.*]], label [[RIGHT:%.*]]
Expand Down Expand Up @@ -125,9 +122,8 @@ right:
define void @f_4(i32 %a, i32 %b) {
; CHECK-LABEL: @f_4(
; CHECK-NEXT: entry:
; CHECK-NEXT: [[B_GW_FR:%.*]] = freeze i32 [[B:%.*]]
; CHECK-NEXT: [[COND_0:%.*]] = icmp ult i32 [[A:%.*]], 10
; CHECK-NEXT: [[COND_1:%.*]] = icmp ult i32 [[B_GW_FR]], 10
; CHECK-NEXT: [[COND_1:%.*]] = icmp ult i32 [[B:%.*]], 10
; CHECK-NEXT: [[WIDE_CHK:%.*]] = and i1 [[COND_0]], [[COND_1]]
; CHECK-NEXT: call void (i1, ...) @llvm.experimental.guard(i1 [[WIDE_CHK]]) [ "deopt"() ]
; CHECK-NEXT: br i1 undef, label [[LOOP:%.*]], label [[LEAVE:%.*]]
Expand Down Expand Up @@ -208,9 +204,8 @@ entry:
define void @f_7(i32 %a, ptr %cond_buf) {
; CHECK-LABEL: @f_7(
; CHECK-NEXT: entry:
; CHECK-NEXT: [[A_GW_FR:%.*]] = freeze i32 [[A:%.*]]
; CHECK-NEXT: [[COND_1:%.*]] = load volatile i1, ptr [[COND_BUF:%.*]], align 1
; CHECK-NEXT: [[COND_3:%.*]] = icmp ult i32 [[A_GW_FR]], 7
; CHECK-NEXT: [[COND_3:%.*]] = icmp ult i32 [[A:%.*]], 7
; CHECK-NEXT: [[WIDE_CHK:%.*]] = and i1 [[COND_1]], [[COND_3]]
; CHECK-NEXT: call void (i1, ...) @llvm.experimental.guard(i1 [[WIDE_CHK]]) [ "deopt"() ]
; CHECK-NEXT: [[COND_2:%.*]] = load volatile i1, ptr [[COND_BUF]], align 1
Expand Down Expand Up @@ -244,13 +239,12 @@ right:
define void @f_8(i32 %a, i1 %cond_1, i1 %cond_2) {
; CHECK-LABEL: @f_8(
; CHECK-NEXT: entry:
; CHECK-NEXT: [[A_GW_FR:%.*]] = freeze i32 [[A:%.*]]
; CHECK-NEXT: br label [[LOOP:%.*]]
; CHECK: loop:
; CHECK-NEXT: call void (i1, ...) @llvm.experimental.guard(i1 [[COND_1:%.*]]) [ "deopt"() ]
; CHECK-NEXT: br i1 undef, label [[LOOP]], label [[LEAVE:%.*]]
; CHECK: leave:
; CHECK-NEXT: [[COND_3:%.*]] = icmp ult i32 [[A_GW_FR]], 7
; CHECK-NEXT: [[COND_3:%.*]] = icmp ult i32 [[A:%.*]], 7
; CHECK-NEXT: [[WIDE_CHK:%.*]] = and i1 [[COND_2:%.*]], [[COND_3]]
; CHECK-NEXT: call void (i1, ...) @llvm.experimental.guard(i1 [[WIDE_CHK]]) [ "deopt"() ]
; CHECK-NEXT: br i1 undef, label [[LOOP2:%.*]], label [[LEAVE2:%.*]]
Expand Down Expand Up @@ -338,10 +332,9 @@ no_loop:
define void @f_11(i32 %a, i1 %cond_0, i1 %cond_1) {
; CHECK-LABEL: @f_11(
; CHECK-NEXT: entry:
; CHECK-NEXT: [[COND_1_GW_FR:%.*]] = freeze i1 [[COND_1:%.*]]
; CHECK-NEXT: br label [[INNER:%.*]]
; CHECK: inner:
; CHECK-NEXT: [[WIDE_CHK:%.*]] = and i1 [[COND_0:%.*]], [[COND_1_GW_FR]]
; CHECK-NEXT: [[WIDE_CHK:%.*]] = and i1 [[COND_0:%.*]], [[COND_1:%.*]]
; CHECK-NEXT: call void (i1, ...) @llvm.experimental.guard(i1 [[WIDE_CHK]]) [ "deopt"() ]
; CHECK-NEXT: br i1 undef, label [[INNER]], label [[OUTER:%.*]]
; CHECK: outer:
Expand All @@ -365,8 +358,7 @@ outer:
define void @f_12(i32 %a0) {
; CHECK-LABEL: @f_12(
; CHECK-NEXT: entry:
; CHECK-NEXT: [[A0_GW_FR:%.*]] = freeze i32 [[A0:%.*]]
; CHECK-NEXT: [[A1:%.*]] = mul i32 [[A0_GW_FR]], [[A0_GW_FR]]
; CHECK-NEXT: [[A1:%.*]] = mul i32 [[A0:%.*]], [[A0]]
; CHECK-NEXT: [[A2:%.*]] = mul i32 [[A1]], [[A1]]
; CHECK-NEXT: [[A3:%.*]] = mul i32 [[A2]], [[A2]]
; CHECK-NEXT: [[A4:%.*]] = mul i32 [[A3]], [[A3]]
Expand Down
Loading

0 comments on commit cb2c510

Please sign in to comment.