[SCEV] SCEVExpander::isHighCostExpansion(): assert if TTI is not provided

[lebedev.ri]

Summary

Currently, as per check-llvm, we never call SCEVExpander::isHighCostExpansion() with null TTI,
so this appears to be a safe restriction.

[lebedev.ri]

Rebased, NFC.

[mkazantsev]

I'm not sure about how we should interpret lack of TTI. Maybe we should interpret it like "if there is no model, we don't care about the high costs, and everything is low cost enough", or opposite (like in your patch). Neither of this looks NFC to me (given that we can pass null TTI). Maybe we should merge it *after* we make use of TTI inside the methods?

[lebedev.ri]

In D73712#1858598, @mkazantsev wrote:

I'm not sure about how we should interpret lack of TTI.
Maybe we should interpret it like "if there is no model, we don't care about the high costs, and everything is low cost enough", or opposite (like in your patch).

Ack, this is non-obvious.

Neither of this looks NFC to me (given that we can pass null TTI).

Alternatively, we can simply require TTI - i believe at least in all the current callsites, it *will* be provided.

Maybe we should merge it *after* we make use of TTI inside the methods?

We have to account for the null possibility somehow, if we don't we would just be adding potential null pointer dereference.

[mkazantsev]

Ok, with assert it makes sense. LGTM

[lebedev.ri]

In D73712#1866685, @mkazantsev wrote:

Ok, with assert it makes sense. LGTM

Great, thank you for the review!

[lebedev.ri]

Rebased, NFC.

[lebedev.ri]

Upload correct patch.

llvm/lib/Analysis/ScalarEvolutionExpander.cpp

bool SCEVExpander::isHighCostExpansionHelper(
     const TargetTransformInfo *TTI, SmallPtrSetImpl<const SCEV *> &Processed) {
   // Was the cost of expansion of this expression already accounted for?
   if (!Processed.insert(S).second)
     return false; // We have already accounted for this expression.
 
   // If we can find an existing value for this scev available at the point "At"
   // then consider the expression cheap.
   if (At && getRelatedExistingExpansion(S, At, L))
-    return false;
+    return false; // Consider the expression to be free.
 
-  // Zero/One operand expressions
   switch (S->getSCEVType()) {
   case scUnknown:
   case scConstant:
-    return false;
+    return false; // Assume to be zero-cost.
+  }
+
+  // The rest of the logic is recursive!
+  if (!TTI)
+    return true; // No cost model - give up.
+
+  // Zero/One operand expressions
+  switch (S->getSCEVType()) {
   case scTruncate:
     return isHighCostExpansionHelper(cast<SCEVTruncateExpr>(S)->getOperand(), L,
                                      At, BudgetRemaining, TTI, Processed);
   case scZeroExtend:
     return isHighCostExpansionHelper(cast<SCEVZeroExtendExpr>(S)->getOperand(),
                                      L, At, BudgetRemaining, TTI, Processed);
   case scSignExtend:
     return isHighCostExpansionHelper(cast<SCEVSignExtendExpr>(S)->getOperand(),
                                      L, At, BudgetRemaining, TTI, Processed);
   }
 
-
   if (auto *UDivExpr = dyn_cast<SCEVUDivExpr>(S)) {
     // If the divisor is a power of two and the SCEV type fits in a native
     // integer (and the LHS not expensive), consider the division cheap
     // irrespective of whether it occurs in the user code since it can be
     // lowered into a right shift.
     if (auto *SC = dyn_cast<SCEVConstant>(UDivExpr->getRHS()))
       if (SC->getAPInt().isPowerOf2()) {
         if (isHighCostExpansionHelper(UDivExpr->getLHS(), L, At,