[IRCE][NFC] Set Index and End together inside parseRangeCheckICmp

[aleksandr.popov]

Summary

Preparatory refactoring for the upcoming support of new range check form
to parse.

With this change we always set Index and End values together in the same
place.

parseRangeCheckICmp specification updated.

[skatkov]

LGTM with nit.

llvm/lib/Transforms/Scalar/InductiveRangeCheckElimination.cpp

251–252

According to specification of this method if we return false, Index and End must be set to nullptr. I do not see this in the code.

Could please update specification or change to implementation accordingly.

I see only one usage of this method and it does not require to set Index and End to nullptr, so I guess that updating of specification is a right way to go.

llvm/lib/Transforms/Scalar/InductiveRangeCheckElimination.cpp

InductiveRangeCheckElimination(ScalarEvolution &SE,
                                  LoopInfo &LI, GetBFIFunc GetBFI = std::nullopt)
       : SE(SE), BPI(BPI), DT(DT), LI(LI), GetBFI(GetBFI) {}
 
   bool run(Loop *L, function_ref<void(Loop *, bool)> LPMAddNewLoop);
 };
 
 } // end anonymous namespace
 
 /// Parse a single ICmp instruction, `ICI`, into a range check.  If `ICI` cannot
-/// be interpreted as a range check, return false and set `Index` and `End`
-/// to `nullptr`.  Otherwise set `Index` to the SCEV being range checked, and
-/// set `End` to the upper limit `Index` is being range checked.
+/// be interpreted as a range check, return false.  Otherwise set `Index` to the
+/// SCEV being range checked, and set `End` to the upper or lower limit `Index`
+/// is being range checked.
 bool InductiveRangeCheck::parseRangeCheckICmp(Loop *L, ICmpInst *ICI,
                                               ScalarEvolution &SE,
                                               const SCEV *&Index,
                                               const SCEV *&End) {
   auto IsLoopInvariant = [&SE, L](Value *V) {
     return SE.isLoopInvariant(SE.getSCEV(V), L);
   };
 
+  auto SIntMaxSCEV = [&](Type *T) {
+    unsigned BitWidth = cast<IntegerType>(T)->getBitWidth();
+    return SE.getConstant(APInt::getSignedMaxValue(BitWidth));
+  };
+
   ICmpInst::Predicate Pred = ICI->getPredicate();
   Value *LHS = ICI->getOperand(0);
   Value *RHS = ICI->getOperand(1);
 
   // Canonicalize to the `Index Pred Invariant` comparison
   if (IsLoopInvariant(LHS)) {
     std::swap(LHS, RHS);
     Pred = CmpInst::getSwappedPredicate(Pred);
   } else if (!IsLoopInvariant(RHS))
     // Both LHS and RHS are loop variant
     return false;
 
+  // We strengthen "0 <= I" to "0 <= I < INT_SMAX" and "I < L" to "0 <= I < L".
+  // We can potentially do much better here.
+  // If we want to adjust upper bound for the unsigned range check as we do it
+  // for signed one, we will need to pick Unsigned max
   switch (Pred) {
   default:
     return false;
 
   case ICmpInst::ICMP_SGE:
     if (match(RHS, m_ConstantInt<0>())) {
       Index = SE.getSCEV(LHS);
+      End = SIntMaxSCEV(Index->getType());
       return true;
     }
     return false;
 
   case ICmpInst::ICMP_SGT:
     if (match(RHS, m_ConstantInt<-1>())) {
       Index = SE.getSCEV(LHS);
+      End = SIntMaxSCEV(Index->getType());
       return true;
     }
     return false;
 
   case ICmpInst::ICMP_SLT:
   case ICmpInst::ICMP_ULT:
     Index = SE.getSCEV(LHS);
     End = SE.getSCEV(RHS);
void InductiveRangeCheck::extractRangeChecksFromCond(
   ICmpInst *ICI = dyn_cast<ICmpInst>(Condition);
   if (!ICI)
     return;
 
   const SCEV *End = nullptr, *Index = nullptr;
   if (!parseRangeCheckICmp(L, ICI, SE, Index, End))
     return;
 
+  assert(Index && "Index was not computed");
+  assert(End && "End was not computed");
+
   const auto *IndexAddRec = dyn_cast<SCEVAddRecExpr>(Index);
   bool IsAffineIndex =
       IndexAddRec && (IndexAddRec->getLoop() == L) && IndexAddRec->isAffine();
 
   if (!IsAffineIndex)
     return;
 
-  // We strengthen "0 <= I" to "0 <= I < INT_SMAX" and "I < L" to "0 <= I < L".
-  // We can potentially do much better here.
-  if (!End) {
-    // So far we can only reach this point for Signed range check. This may
-    // change in future. In this case we will need to pick Unsigned max for the
-    // unsigned range check.
-    unsigned BitWidth = cast<IntegerType>(IndexAddRec->getType())->getBitWidth();
-    const SCEV *SIntMax = SE.getConstant(APInt::getSignedMaxValue(BitWidth));
-    End = SIntMax;
-  }
-
   InductiveRangeCheck IRC;
   IRC.End = End;
   IRC.Begin = IndexAddRec->getStart();
   IRC.Step = IndexAddRec->getStepRecurrence(SE);
   IRC.CheckUse = &ConditionUse;
   Checks.push_back(IRC);
 }