[PATCH] D77560: [SCEV] don't try to query getSCEV() for incomplete PHIs

[Max Kazantsev via Phabricator via llvm-commits]

mkazantsev added inline comments.


================
Comment at: llvm/lib/Transforms/Utils/ScalarEvolutionExpander.cpp:1190
+      // PHI has no meaning at all.
+      if (!PN.getParent()->hasNPredecessors(PN.getNumIncomingValues()))
+        continue;
----------------
shchenz wrote:
> shchenz wrote:
> > lebedev.ri wrote:
> > > I don't think this is correct fix. It is perfectly legal to have e.g. the following:
> > > ```
> > > pred:
> > >  <...>
> > >  br label %succ
> > > succ:
> > >  %10 = phi i32 [ 0, %pred ], [ 0, %pred ]
> > > ```
> > Here we want to identify an incomplete PHI.
> > The incomplete PHI is like `%10 = phi i32`, `hasNPredecessors` should be set when the PHI node is created, so it is 2.
> > ```
> >   PHINode *PN = Builder.CreatePHI(ExpandTy, std::distance(HPB, HPE),
> >                                   Twine(IVName) + ".iv");
> > ```
> > 
> > But its `getNumIncomingValues()` is 0 because it is not populated yet.
> > 
> > Do you have any other ways to identify an incomplete PHI? @lebedev.ri 
> should "predecessors number of PHI's parent is bigger than PHI's incoming value number" be a right way? @lebedev.ri 
If we catch phi in mid-construction (when we populated some of inputs but not all of them), it may have more predecessors than inputs (and still be incomplete). I think `PN.isComplete()` is the correct fix. Roman, why do you think querying SCEV in your example is OK?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77560