[GuardWidening] Refactor to work with the list of checks to widen/hoist

[aleksandr.popov]

Summary

Currently we hoist conditions from widenable branch which are joined to
the widenable_condition by And operation. E.g if we have
br(WC && (c1 && c2)) we will operate with (c1 && c2) unsplitted.

This patch adds more flexibility to that mechanism by supporting work
with the list of checks parsed from the widenable branch.

On that stage patch doesn't change the logic of checks hoisting. In the
example above we will either hoist both checks [c1, c2] or none of them.
But in the future we would improve that logic analyzing each check
separately.

[anna]

do you need review for this? Could you pls state the review stack in the description so it is clear the order of reviews. Thanks.

[aleksandr.popov]

Hi Anna, more priority https://reviews.llvm.org/D157502 to review. It's the root patch of the stack to land.

[anna]

llvm/lib/Transforms/Scalar/GuardWidening.cpp

126–127

214–215

Pls update comment.

273

Fix comment.

293–294

Fix comment pls.

301–302

Update comment.

[aleksandr.popov]

Updated comments

[aleksandr.popov]

llvm/lib/Transforms/Scalar/GuardWidening.cpp

214–215

Thanks!

[anna]

llvm/lib/Transforms/Scalar/GuardWidening.cpp

552

Nit: unrelated change.

699

Can we return the result instead of returning a boolean and storing the result in a separate argument?
i.e.

std::optional<Value *> GuardWideningImpl::mergeChecks(SmallVectorImpl<Value *> &ChecksToHoist,
                                    SmallVectorImpl<Value *> &ChecksToWiden,
                                    Instruction *InsertPt)

If this works, pls land this separately before this patch (don't need explicit review) and then rebase on this revision. Makes it easier for review. Thanks.

In places where only the boolean is required, you can just check the optional has value (In isWideningCondProfitable for example).

[aleksandr.popov]

Rebased

[aleksandr.popov]

llvm/lib/Transforms/Scalar/GuardWidening.cpp

699

I've extracted widenCondCommon splitting in a separate patch, PTAL: https://reviews.llvm.org/D159009

[anna]

LGTM.

llvm/lib/Transforms/Scalar/GuardWidening.cpp

 #include "llvm/Analysis/GuardUtils.h"
 #include "llvm/Analysis/LoopInfo.h"
 #include "llvm/Analysis/LoopPass.h"
 #include "llvm/Analysis/MemorySSAUpdater.h"
 #include "llvm/Analysis/PostDominators.h"
 #include "llvm/Analysis/ValueTracking.h"
 #include "llvm/IR/ConstantRange.h"
 #include "llvm/IR/Dominators.h"
+#include "llvm/IR/IRBuilder.h"
 #include "llvm/IR/IntrinsicInst.h"
 #include "llvm/IR/PatternMatch.h"
 #include "llvm/InitializePasses.h"
 #include "llvm/Pass.h"
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/Debug.h"
 #include "llvm/Support/KnownBits.h"
 #include "llvm/Transforms/Scalar.h"

 /// Find a point at which the widened condition of \p Guard should be inserted.
 /// When it is represented as intrinsic call, we can do it right before the call
 /// instruction. However, when we are dealing with widenable branch, we must
 /// account for the following situation: widening should not turn a
 /// loop-invariant condition into a loop-variant. It means that if
 /// widenable.condition() call is invariant (w.r.t. any loop), the new wide
 /// condition should stay invariant. Otherwise there can be a miscompile, like
 /// the one described at https://github.com/llvm/llvm-project/issues/60234. The
 /// safest way to do it is to expand the new condition at WC's block.
-static Instruction *findInsertionPointForWideCondition(Instruction *Guard) {
-  if (auto WC = extractWidenableCondition(Guard))
+static Instruction *findInsertionPointForWideCondition(Instruction *WCOrGuard) {
+  if (isGuard(WCOrGuard))
+    return WCOrGuard;
+  if (auto WC = extractWidenableCondition(WCOrGuard))
     return cast<Instruction>(WC);
-  return Guard;
+  return nullptr;
 }
 
 class GuardWideningImpl {
   DominatorTree &DT;
   PostDominatorTree *PDT;
   LoopInfo &LI;
   AssumptionCache &AC;
   MemorySSAUpdater *MSSAU;
enum WideningScore {
     /// Widening is very profitable.  Not significantly different from \c
     /// WS_Positive, except by the order.
     WS_VeryPositive
   };
 
   static StringRef scoreTypeToString(WideningScore WS);
 
   /// Compute the score for widening the condition in \p DominatedInstr
-  /// into \p DominatingGuard.
+  /// into \p WideningPoint.
   WideningScore computeWideningScore(Instruction *DominatedInstr,
-                                     Instruction *DominatingGuard);
+                                     Instruction *WideningPoint,
+                                     SmallVectorImpl<Value *> &ChecksToHoist,
+                                     SmallVectorImpl<Value *> &ChecksToWiden);
 
   /// Helper to check if \p V can be hoisted to \p InsertPos.
   bool canBeHoistedTo(const Value *V, const Instruction *InsertPos) const {
     SmallPtrSet<const Instruction *, 8> Visited;
     return canBeHoistedTo(V, InsertPos, Visited);
   }
 
   bool canBeHoistedTo(const Value *V, const Instruction *InsertPos,
                       SmallPtrSetImpl<const Instruction *> &Visited) const;
 
+  bool canBeHoistedTo(const SmallVectorImpl<Value *> &Checks,
+                      const Instruction *InsertPos) const {
+    return all_of(Checks,
+                  [&](const Value *V) { return canBeHoistedTo(V, InsertPos); });
+  }
   /// Helper to hoist \p V to \p InsertPos.  Guaranteed to succeed if \c
   /// canBeHoistedTo returned true.
   void makeAvailableAt(Value *V, Instruction *InsertPos) const;
 
+  void makeAvailableAt(const SmallVectorImpl<Value *> &Checks,
+                       Instruction *InsertPos) const {
+    for_each(Checks, [&](Value *V) { makeAvailableAt(V, InsertPos); });
+  }
+
   /// Common helper used by \c widenGuard and \c isWideningCondProfitable.  Try
-  /// to generate an expression computing the logical AND of \p Cond0 and Cond1.
-  /// Return true if the expression computing the AND is only as
-  /// expensive as computing one of the two. If \p InsertPt is true then
-  /// actually generate the resulting expression, make it available at \p
-  /// InsertPt and return it in \p Result (else no change to the IR is made).
-  std::optional<Value *> mergeChecks(Value *Cond0, Value *Cond1,
+  /// to generate an expression computing the logical AND of \p ChecksToHoist
+  /// and \p ChecksToWiden. Return true if the expression computing the AND is
+  /// only as expensive as computing one of the set of expressions. If \p
+  /// InsertPt is true then actually generate the resulting expression, make it
+  /// available at \p InsertPt and return it in \p Result (else no change to the
+  /// IR is made).
+  std::optional<Value *> mergeChecks(SmallVectorImpl<Value *> &ChecksToHoist,
+                                     SmallVectorImpl<Value *> &ChecksToWiden,
                                      Instruction *InsertPt);
 
-  /// Generate the logical AND of \p Cond0 and \p Cond1 and make it available at
-  /// \p InsertPt
-  Value *hoistChecks(Value *Cond0, Value *Cond1, Instruction *InsertPt);
+  /// Generate the logical AND of \p ChecksToHoist and \p OldCondition and make
+  /// it available at InsertPt
+  Value *hoistChecks(SmallVectorImpl<Value *> &ChecksToHoist,
+                     Value *OldCondition, Instruction *InsertPt);
 
   /// 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
public:
     }
 
     LLVM_DUMP_METHOD void dump() {
       print(dbgs());
       dbgs() << "\n";
     }
   };
 
-  /// Parse \p CheckCond into a conjunction (logical-and) of range checks; and
+  /// Parse \p ToParse into a conjunction (logical-and) of range checks; and
   /// append them to \p Checks.  Returns true on success, may clobber \c Checks
   /// on failure.
-  bool parseRangeChecks(Value *CheckCond, SmallVectorImpl<RangeCheck> &Checks) {
-    SmallPtrSet<const Value *, 8> Visited;
-    return parseRangeChecks(CheckCond, Checks, Visited);
+  bool parseRangeChecks(SmallVectorImpl<Value *> &ToParse,
+                        SmallVectorImpl<RangeCheck> &Checks) {
+    for (auto CheckCond : ToParse) {
+      if (!parseRangeChecks(CheckCond, Checks))
+        return false;
+    }
+    return true;
   }
 
-  bool parseRangeChecks(Value *CheckCond, SmallVectorImpl<RangeCheck> &Checks,
-                        SmallPtrSetImpl<const Value *> &Visited);
+  bool parseRangeChecks(Value *CheckCond, SmallVectorImpl<RangeCheck> &Checks);
 
   /// Combine the checks in \p Checks into a smaller set of checks and append
   /// them into \p CombinedChecks.  Return true on success (i.e. all of checks
   /// in \p Checks were combined into \p CombinedChecks).  Clobbers \p Checks
   /// and \p CombinedChecks on success and on failure.
   bool combineRangeChecks(SmallVectorImpl<RangeCheck> &Checks,
                           SmallVectorImpl<RangeCheck> &CombinedChecks) const;
 
-  /// Can we compute the logical AND of \p Cond0 and \p Cond1 for the price of
-  /// computing only one of the two expressions?
-  bool isWideningCondProfitable(Value *Cond0, Value *Cond1) {
-    return mergeChecks(Cond0, Cond1, /*InsertPt=*/nullptr).has_value();
+  /// Can we compute the logical AND of \p ChecksToHoist and \p ChecksToWiden
+  /// for the price of computing only one of the set of expressions?
+  bool isWideningCondProfitable(SmallVectorImpl<Value *> &ChecksToHoist,
+                                SmallVectorImpl<Value *> &ChecksToWiden) {
+    return mergeChecks(ChecksToHoist, ChecksToWiden, /*InsertPt=*/nullptr)
+        .has_value();
   }
 
-  /// Widen \p ToWiden to fail if \p NewCondition is false
-  void widenGuard(Instruction *ToWiden, Value *NewCondition) {
+  /// Widen \p ChecksToWiden to fail if any of \p ChecksToHoist is false
+  void widenGuard(SmallVectorImpl<Value *> &ChecksToHoist,
+                  SmallVectorImpl<Value *> &ChecksToWiden,
+                  Instruction *ToWiden) {
     Instruction *InsertPt = findInsertionPointForWideCondition(ToWiden);
-    auto MergedCheck =
-        mergeChecks(getCondition(ToWiden), NewCondition, InsertPt);
+    auto MergedCheck = mergeChecks(ChecksToHoist, ChecksToWiden, InsertPt);
     Value *Result = MergedCheck ? *MergedCheck
-                                : hoistChecks(getCondition(ToWiden),
-                                              NewCondition, InsertPt);
+                                : hoistChecks(ChecksToHoist,
+                                              getCondition(ToWiden), InsertPt);
 
     if (isGuardAsWidenableBranch(ToWiden)) {
       setWidenableBranchCond(cast<BranchInst>(ToWiden), Result);
       return;
     }
     setCondition(ToWiden, Result);
   }
 
bool GuardWideningImpl::run() {
 
   return Changed;
 }
 
 bool GuardWideningImpl::eliminateInstrViaWidening(
     Instruction *Instr, const df_iterator<DomTreeNode *> &DFSI,
     const DenseMap<BasicBlock *, SmallVector<Instruction *, 8>>
         &GuardsInBlock) {
+  SmallVector<Value *> ChecksToHoist;
+  parseWidenableGuard(Instr, ChecksToHoist);
   // Ignore trivial true or false conditions. These instructions will be
   // trivially eliminated by any cleanup pass. Do not erase them because other
   // guards can possibly be widened into them.
-  if (isa<ConstantInt>(getCondition(Instr)))
+  if (ChecksToHoist.empty() ||
+      (ChecksToHoist.size() == 1 && isa<ConstantInt>(ChecksToHoist.front())))
     return false;
 
   Instruction *BestSoFar = nullptr;
   auto BestScoreSoFar = WS_IllegalOrNegative;
 
   // In the set of dominating guards, find the one we can merge GuardInst with
   // for the most profit.
   for (unsigned i = 0, e = DFSI.getPathLength(); i != e; ++i) {
#ifndef NDEBUG
       assert(Index == GuardsInCurBB.size() &&
              "Guards expected to be in order!");
     }
 #endif
 
     assert((i == (e - 1)) == (Instr->getParent() == CurBB) && "Bad DFS?");
 
     for (auto *Candidate : make_range(I, E)) {
-      auto Score = computeWideningScore(Instr, Candidate);
-      LLVM_DEBUG(dbgs() << "Score between " << *getCondition(Instr)
-                        << " and " << *getCondition(Candidate) << " is "
-                        << scoreTypeToString(Score) << "\n");
+      auto *WideningPoint = findInsertionPointForWideCondition(Candidate);
+      if (!WideningPoint)
+        continue;
+      SmallVector<Value *> CandidateChecks;
+      parseWidenableGuard(Candidate, CandidateChecks);
+      auto Score = computeWideningScore(Instr, WideningPoint, ChecksToHoist,
+                                        CandidateChecks);
+      LLVM_DEBUG(dbgs() << "Score between " << *Instr << " and " << *Candidate
+                        << " is " << scoreTypeToString(Score) << "\n");
       if (Score > BestScoreSoFar) {
         BestScoreSoFar = Score;
         BestSoFar = Candidate;
       }
     }
   }
 
   if (BestScoreSoFar == WS_IllegalOrNegative) {
     LLVM_DEBUG(dbgs() << "Did not eliminate guard " << *Instr << "\n");
     return false;
   }
 
   assert(BestSoFar != Instr && "Should have never visited same guard!");
   assert(DT.dominates(BestSoFar, Instr) && "Should be!");
 
   LLVM_DEBUG(dbgs() << "Widening " << *Instr << " into " << *BestSoFar
                     << " with score " << scoreTypeToString(BestScoreSoFar)
                     << "\n");
-  widenGuard(BestSoFar, getCondition(Instr));
+  SmallVector<Value *> ChecksToWiden;
+  parseWidenableGuard(BestSoFar, ChecksToWiden);
+  widenGuard(ChecksToHoist, ChecksToWiden, BestSoFar);
   auto NewGuardCondition = ConstantInt::getTrue(Instr->getContext());
   setCondition(Instr, NewGuardCondition);
   EliminatedGuardsAndBranches.push_back(Instr);
   WidenedGuards.insert(BestSoFar);
   return true;
 }
 
-GuardWideningImpl::WideningScore
-GuardWideningImpl::computeWideningScore(Instruction *DominatedInstr,
-                                        Instruction *DominatingGuard) {
+GuardWideningImpl::WideningScore GuardWideningImpl::computeWideningScore(
+    Instruction *DominatedInstr, Instruction *WideningPoint,
+    SmallVectorImpl<Value *> &ChecksToHoist,
+    SmallVectorImpl<Value *> &ChecksToWiden) {
   Loop *DominatedInstrLoop = LI.getLoopFor(DominatedInstr->getParent());
-  Loop *DominatingGuardLoop = LI.getLoopFor(DominatingGuard->getParent());
+  Loop *DominatingGuardLoop = LI.getLoopFor(WideningPoint->getParent());
   bool HoistingOutOfLoop = false;
 
   if (DominatingGuardLoop != DominatedInstrLoop) {
     // Be conservative and don't widen into a sibling loop.  TODO: If the
     // sibling is colder, we should consider allowing this.
     if (DominatingGuardLoop &&
         !DominatingGuardLoop->contains(DominatedInstrLoop))
       return WS_IllegalOrNegative;
 
     HoistingOutOfLoop = true;
   }
 
-  auto *WideningPoint = findInsertionPointForWideCondition(DominatingGuard);
-  if (!canBeHoistedTo(getCondition(DominatedInstr), WideningPoint))
+  if (!canBeHoistedTo(ChecksToHoist, WideningPoint))
     return WS_IllegalOrNegative;
-  if (!canBeHoistedTo(getCondition(DominatingGuard), WideningPoint))
+  if (!canBeHoistedTo(ChecksToWiden, WideningPoint))
     return WS_IllegalOrNegative;
 
   // If the guard was conditional executed, it may never be reached
   // dynamically.  There are two potential downsides to hoisting it out of the
   // conditionally executed region: 1) we may spuriously deopt without need and
   // 2) we have the extra cost of computing the guard condition in the common
   // case.  At the moment, we really only consider the second in our heuristic
   // here.  TODO: evaluate cost model for spurious deopt
   // NOTE: As written, this also lets us hoist right over another guard which
   // is essentially just another spelling for control flow.
-  if (isWideningCondProfitable(getCondition(DominatedInstr),
-                               getCondition(DominatingGuard)))
+  if (isWideningCondProfitable(ChecksToHoist, ChecksToWiden))
     return HoistingOutOfLoop ? WS_VeryPositive : WS_Positive;
 
   if (HoistingOutOfLoop)
     return WS_Positive;
 
   // For a given basic block \p BB, return its successor which is guaranteed or
   // highly likely will be taken as its successor.
   auto GetLikelySuccessor = [](const BasicBlock * BB)->const BasicBlock * {
auto GetLikelySuccessor = [](const BasicBlock * BB)->const BasicBlock * {
     return nullptr;
   };
 
   // Returns true if we might be hoisting above explicit control flow into a
   // considerably hotter block.  Note that this completely ignores implicit
   // control flow (guards, calls which throw, etc...).  That choice appears
   // arbitrary (we assume that implicit control flow exits are all rare).
   auto MaybeHoistingToHotterBlock = [&]() {
-    const auto *DominatingBlock = DominatingGuard->getParent();
+    const auto *DominatingBlock = WideningPoint->getParent();
     const auto *DominatedBlock = DominatedInstr->getParent();
 
     // Descend as low as we can, always taking the likely successor.
     assert(DT.isReachableFromEntry(DominatingBlock) && "Unreached code");
     assert(DT.isReachableFromEntry(DominatedBlock) && "Unreached code");
     assert(DT.dominates(DominatingBlock, DominatedBlock) && "No dominance");
     while (DominatedBlock != DominatingBlock) {
       auto *LikelySucc = GetLikelySuccessor(DominatingBlock);
auto MaybeHoistingToHotterBlock = [&]() {
     // Found?
     if (DominatedBlock == DominatingBlock)
       return false;
     // We followed the likely successor chain and went past the dominated
     // block. It means that the dominated guard is in dead/very cold code.
     if (!DT.dominates(DominatingBlock, DominatedBlock))
       return true;
     // TODO: diamond, triangle cases
-    if (!PDT) return true;
+    if (!PDT)
+      return true;
     return !PDT->dominates(DominatedBlock, DominatingBlock);
   };
 
   return MaybeHoistingToHotterBlock() ? WS_IllegalOrNegative : WS_Neutral;
 }
 
 bool GuardWideningImpl::canBeHoistedTo(
     const Value *V, const Instruction *Loc,
if (V == Orig)
       Result = FI;
     V->replaceUsesWithIf(
         FI, [&](const Use & U)->bool { return U.getUser() != FI; });
   }
 
   return Result;
 }
 
-std::optional<Value *> GuardWideningImpl::mergeChecks(Value *Cond0,
-                                                      Value *Cond1,
-                                                      Instruction *InsertPt) {
+std::optional<Value *>
+GuardWideningImpl::mergeChecks(SmallVectorImpl<Value *> &ChecksToHoist,
+                               SmallVectorImpl<Value *> &ChecksToWiden,
   using namespace llvm::PatternMatch;
 
   Value *Result = nullptr;
   {
     // L >u C0 && L >u C1  ->  L >u max(C0, C1)
     ConstantInt *RHS0, *RHS1;
     Value *LHS;
     ICmpInst::Predicate Pred0, Pred1;
-    if (match(Cond0, m_ICmp(Pred0, m_Value(LHS), m_ConstantInt(RHS0))) &&
-        match(Cond1, m_ICmp(Pred1, m_Specific(LHS), m_ConstantInt(RHS1)))) {
+    // TODO: Support searching for pairs to merge from both whole lists of
+    // ChecksToHoist and ChecksToWiden.
+    if (ChecksToWiden.size() == 1 && ChecksToHoist.size() == 1 &&
+        match(ChecksToWiden.front(),
+              m_ICmp(Pred0, m_Value(LHS), m_ConstantInt(RHS0))) &&
+        match(ChecksToHoist.front(),
+              m_ICmp(Pred1, m_Specific(LHS), m_ConstantInt(RHS1)))) {
 
       ConstantRange CR0 =
           ConstantRange::makeExactICmpRegion(Pred0, RHS0->getValue());
       ConstantRange CR1 =
           ConstantRange::makeExactICmpRegion(Pred1, RHS1->getValue());
 
       // Given what we're doing here and the semantics of guards, it would
       // be correct to use a subset intersection, but that may be too
       // aggressive in cases we care about.
       if (std::optional<ConstantRange> Intersect =
               CR0.exactIntersectWith(CR1)) {
         APInt NewRHSAP;
         CmpInst::Predicate Pred;
         if (Intersect->getEquivalentICmp(Pred, NewRHSAP)) {
           if (InsertPt) {
             ConstantInt *NewRHS =
-                ConstantInt::get(Cond0->getContext(), NewRHSAP);
+                ConstantInt::get(InsertPt->getContext(), NewRHSAP);
             assert(canBeHoistedTo(LHS, InsertPt) && "must be");
             makeAvailableAt(LHS, InsertPt);
             Result = new ICmpInst(InsertPt, Pred, LHS, NewRHS, "wide.chk");
           }
           return Result;
         }
       }
     }
   }
 
   {
     SmallVector<GuardWideningImpl::RangeCheck, 4> Checks, CombinedChecks;
-    if (parseRangeChecks(Cond0, Checks) && parseRangeChecks(Cond1, Checks) &&
+    if (parseRangeChecks(ChecksToWiden, Checks) &&
+        parseRangeChecks(ChecksToHoist, Checks) &&
         combineRangeChecks(Checks, CombinedChecks)) {
       if (InsertPt) {
         for (auto &RC : CombinedChecks) {
           makeAvailableAt(RC.getCheckInst(), InsertPt);
           if (Result)
             Result = BinaryOperator::CreateAnd(RC.getCheckInst(), Result, "",
                                                InsertPt);
           else
             Result = RC.getCheckInst();
         }
         assert(Result && "Failed to find result value");
         Result->setName("wide.chk");
         Result = freezeAndPush(Result, InsertPt);
       }
       return Result;
     }
   }
-  // We were not able to compute Cond0 AND Cond1 for the price of one.
+  // We were not able to compute ChecksToHoist AND ChecksToWiden for the price
+  // of one.
   return std::nullopt;
 }
 
-Value *GuardWideningImpl::hoistChecks(Value *Cond0, Value *Cond1,
+Value *GuardWideningImpl::hoistChecks(SmallVectorImpl<Value *> &ChecksToHoist,
+                                      Value *OldCondition,
                                       Instruction *InsertPt) {
-  makeAvailableAt(Cond0, InsertPt);
-  makeAvailableAt(Cond1, InsertPt);
-  Cond1 = freezeAndPush(Cond1, InsertPt);
-  return BinaryOperator::CreateAnd(Cond0, Cond1, "wide.chk", InsertPt);
+  assert(!ChecksToHoist.empty());
+  IRBuilder<> Builder(InsertPt);
+  makeAvailableAt(ChecksToHoist, InsertPt);
+  makeAvailableAt(OldCondition, InsertPt);
+  Value *Result = Builder.CreateAnd(ChecksToHoist);
+  Result = freezeAndPush(Result, InsertPt);
+  Result = Builder.CreateAnd(OldCondition, Result);
+  Result->setName("wide.chk");
+  return Result;
 }
 
 bool GuardWideningImpl::parseRangeChecks(
-    Value *CheckCond, SmallVectorImpl<GuardWideningImpl::RangeCheck> &Checks,
-    SmallPtrSetImpl<const Value *> &Visited) {
-  if (!Visited.insert(CheckCond).second)
-    return true;
-
+    Value *CheckCond, SmallVectorImpl<GuardWideningImpl::RangeCheck> &Checks) {
   using namespace llvm::PatternMatch;
 
-  {
-    Value *AndLHS, *AndRHS;
-    if (match(CheckCond, m_And(m_Value(AndLHS), m_Value(AndRHS))))
-      return parseRangeChecks(AndLHS, Checks) &&
-             parseRangeChecks(AndRHS, Checks);
-  }
-
   auto *IC = dyn_cast<ICmpInst>(CheckCond);
   if (!IC || !IC->getOperand(0)->getType()->isIntegerTy() ||
       (IC->getPredicate() != ICmpInst::ICMP_ULT &&
        IC->getPredicate() != ICmpInst::ICMP_UGT))
     return false;
 
   const Value *CmpLHS = IC->getOperand(0), *CmpRHS = IC->getOperand(1);
   if (IC->getPredicate() == ICmpInst::ICMP_UGT)

llvm/test/Transforms/GuardWidening/range-check-merging.ll

 ; CHECK-NEXT:  entry:
 ; CHECK-NEXT:    [[X_GW_FR:%.*]] = freeze i32 [[X:%.*]]
 ; CHECK-NEXT:    [[LENGTH_A:%.*]] = load volatile i32, ptr [[LENGTH_BUF:%.*]], align 4, !range [[RNG0]], !noundef !1
 ; CHECK-NEXT:    [[LENGTH_B:%.*]] = load volatile i32, ptr [[LENGTH_BUF]], align 4, !range [[RNG0]], !noundef !1
 ; CHECK-NEXT:    [[CHK0_A:%.*]] = icmp ult i32 [[X_GW_FR]], [[LENGTH_A]]
 ; CHECK-NEXT:    [[CHK0_B:%.*]] = icmp ult i32 [[X_GW_FR]], [[LENGTH_B]]
 ; CHECK-NEXT:    [[CHK0:%.*]] = and i1 [[CHK0_A]], [[CHK0_B]]
 ; CHECK-NEXT:    [[X_INC1:%.*]] = add i32 [[X_GW_FR]], 1
-; CHECK-NEXT:    [[CHK1_A:%.*]] = icmp ult i32 [[X_INC1]], [[LENGTH_A]]
 ; CHECK-NEXT:    [[CHK1_B:%.*]] = icmp ult i32 [[X_INC1]], [[LENGTH_B]]
-; CHECK-NEXT:    [[CHK1:%.*]] = and i1 [[CHK1_A]], [[CHK1_B]]
-; CHECK-NEXT:    [[WIDE_CHK:%.*]] = and i1 [[CHK0]], [[CHK1]]
+; CHECK-NEXT:    [[CHK1_A:%.*]] = icmp ult i32 [[X_INC1]], [[LENGTH_A]]
+; CHECK-NEXT:    [[TMP0:%.*]] = and i1 [[CHK1_B]], [[CHK1_A]]
+; CHECK-NEXT:    [[WIDE_CHK:%.*]] = and i1 [[CHK0]], [[TMP0]]
 ; CHECK-NEXT:    [[X_INC2:%.*]] = add i32 [[X_GW_FR]], 2
 ; CHECK-NEXT:    [[CHK2_A:%.*]] = icmp ult i32 [[X_INC2]], [[LENGTH_A]]
-; CHECK-NEXT:    [[TMP0:%.*]] = and i1 [[CHK2_A]], [[CHK0_A]]
-; CHECK-NEXT:    [[TMP1:%.*]] = and i1 [[CHK0_B]], [[TMP0]]
+; CHECK-NEXT:    [[TMP1:%.*]] = and i1 [[CHK2_A]], [[CHK0_A]]
+; CHECK-NEXT:    [[TMP2:%.*]] = and i1 [[CHK0_B]], [[TMP1]]
 ; CHECK-NEXT:    [[CHK2_B:%.*]] = icmp ult i32 [[X_INC2]], [[LENGTH_B]]
-; CHECK-NEXT:    [[WIDE_CHK1:%.*]] = and i1 [[CHK2_B]], [[TMP1]]
+; CHECK-NEXT:    [[WIDE_CHK1:%.*]] = and i1 [[CHK2_B]], [[TMP2]]
 ; CHECK-NEXT:    [[X_INC3:%.*]] = add i32 [[X_GW_FR]], 3
-; CHECK-NEXT:    [[CHK3_B:%.*]] = icmp ult i32 [[X_INC3]], [[LENGTH_B]]
-; CHECK-NEXT:    [[TMP2:%.*]] = and i1 [[CHK3_B]], [[CHK0_B]]
-; CHECK-NEXT:    [[TMP3:%.*]] = and i1 [[CHK0_A]], [[TMP2]]
 ; CHECK-NEXT:    [[CHK3_A:%.*]] = icmp ult i32 [[X_INC3]], [[LENGTH_A]]
-; CHECK-NEXT:    [[WIDE_CHK2:%.*]] = and i1 [[CHK3_A]], [[TMP3]]
+; CHECK-NEXT:    [[TMP3:%.*]] = and i1 [[CHK3_A]], [[CHK0_A]]
+; CHECK-NEXT:    [[TMP4:%.*]] = and i1 [[CHK0_B]], [[TMP3]]
+; CHECK-NEXT:    [[CHK3_B:%.*]] = icmp ult i32 [[X_INC3]], [[LENGTH_B]]
+; CHECK-NEXT:    [[WIDE_CHK2:%.*]] = and i1 [[CHK3_B]], [[TMP4]]
 ; CHECK-NEXT:    call void (i1, ...) @llvm.experimental.guard(i1 [[WIDE_CHK2]]) [ "deopt"() ]
+; CHECK-NEXT:    [[CHK1:%.*]] = and i1 [[CHK1_A]], [[CHK1_B]]
 ; CHECK-NEXT:    [[CHK2:%.*]] = and i1 [[CHK2_A]], [[CHK2_B]]
 ; CHECK-NEXT:    [[CHK3:%.*]] = and i1 [[CHK3_A]], [[CHK3_B]]
 ; CHECK-NEXT:    ret void
 ;
 
 
 entry:
   %length_a = load volatile i32, ptr %length_buf, !range !0, !noundef !{}