[PATCH] D129653: isInductionPHI - Add some safety checks

[Nikita Popov via Phabricator via llvm-commits]

nikic added inline comments.


================
Comment at: llvm/lib/Analysis/IVDescriptors.cpp:1537
 
+  if (!(AR->getLoop()->getLoopPreheader()))
+      return false;
----------------
syzaara wrote:
> nickdesaulniers wrote:
> > syzaara wrote:
> > > fhahn wrote:
> > > > This is overly restrictive, whether the loop has a pre-header or not shouldn't impact whether a phi is an induction or now.
> > > On the line below, we query for AR->getLoop()->getLoopPreheader(). If it turns out that this is null, we will see a crash. I thought adding this would be safer rather than restrictive.
> > maybe Phi::getIncomingValueForBlock should return `nullptr` if passed `nullptr`?
> Yes, the other option is it move this check into Phi::getIncomingValueForBlock. @fhahn would that be more preferable?
It would be possible to relax this to `getLoopPredecessor()`. Without a loop predecessor we would have to guard against multiple starting values, which is probably not worthwhile?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D129653/new/

https://reviews.llvm.org/D129653