[IVDescriptors] Add assert to isInductionPhi to check for invalid Phis

[vedant-amd]

Summary

Phis that are present inside loop headers can only be Induction Phis
legally. This patch adds an assertion to isInductionPhi which checks for
the said legality and it also updates the docs of the said function to
reflect the given legality.

[vedant-amd]

minor fix to commit message

[vedant-amd]

run clang-format

[vedant-amd]

Removed whitespaces

[jdoerfert]

llvm/lib/Analysis/IVDescriptors.cpp

1491

Just return false?

[fhahn]

LGTM, thanks! IMO all callers in-tree ensure this invariant so the assert seems good to me.

llvm/lib/Analysis/IVDescriptors.cpp

1491

AFAICT all callers in-tree ensure this invariant, so an assert seems good to me. Not sure if the verbose comment is needed in combination with the assert message.

[vedant-amd]

Hey @fhahn I don't have rights to commit this myself, can you commit it on my behalf ? (or give me commit access? edit: I have mailed chris, as per the docs) Thanks !

Name: Vedant Paranjape
email: [e-mail]
github: vedantparanjape-amd

llvm/include/llvm/Analysis/IVDescriptors.h

public:
   Value *getStartValue() const { return StartValue; }
   InductionKind getKind() const { return IK; }
   const SCEV *getStep() const { return Step; }
   BinaryOperator *getInductionBinOp() const { return InductionBinOp; }
   ConstantInt *getConstIntStepValue() const;
 
   /// Returns true if \p Phi is an induction in the loop \p L. If \p Phi is an
   /// induction, the induction descriptor \p D will contain the data describing
-  /// this induction. If by some other means the caller has a better SCEV
+  /// this induction. Since Induction Phis can only be present inside loop
+  /// headers, the function will assert if it is passed a Phi whose parent is
+  /// not the loop header. If by some other means the caller has a better SCEV
   /// expression for \p Phi than the one returned by the ScalarEvolution
   /// analysis, it can be passed through \p Expr. If the def-use chain
   /// associated with the phi includes casts (that we know we can ignore
   /// under proper runtime checks), they are passed through \p CastsToIgnore.
   static bool
   isInductionPHI(PHINode *Phi, const Loop *L, ScalarEvolution *SE,
                  InductionDescriptor &D, const SCEV *Expr = nullptr,
                  SmallVectorImpl<Instruction *> *CastsToIgnore = nullptr);

llvm/lib/Analysis/IVDescriptors.cpp

bool InductionDescriptor::isInductionPHI(
   if (AR->getLoop() != TheLoop) {
     // FIXME: We should treat this as a uniform. Unfortunately, we
     // don't currently know how to handled uniform PHIs.
     LLVM_DEBUG(
         dbgs() << "LV: PHI is a recurrence with respect to an outer loop.\n");
     return false;
   }
 
+  // This function assumes that InductionPhi is called only on Phi nodes
+  // present inside loop headers. Check for the same, and throw an assert if
+  // the current Phi is not present inside the loop header.
+  assert(Phi->getParent() == AR->getLoop()->getHeader()
+    && "Invalid Phi node, not present in loop header");
+
   Value *StartValue =
       Phi->getIncomingValueForBlock(AR->getLoop()->getLoopPreheader());
 
   BasicBlock *Latch = AR->getLoop()->getLoopLatch();
   if (!Latch)
     return false;
 
   const SCEV *Step = AR->getStepRecurrence(*SE);