[PATCH] D30543: [JumpThreading] Perform phi-translation in SimplifyPartiallyRedundantLoad.

[Daniel Berlin via Phabricator via llvm-commits]

dberlin added inline comments.


================
Comment at: lib/Transforms/Scalar/JumpThreading.cpp:940
+
+  if (Instruction *PtrOp = dyn_cast<Instruction>(LoadedPtr)) {
+    // If the loaded operand is a phi not inside LoadBB, we can not
----------------
trentxintong wrote:
> dberlin wrote:
> > This logic looks overly complex.
> > 
> > Isn't this all simply 
> > ```
> > TranslateableLoaddedPhiPtr = LoadedPtr->doPhiTranslation(LoadBB, PredBB) 
> > ```
> > 
> > here, and then
> > 
> > 
> > ```
> > if (TranslateableLoadedPhiPtr == LoadedPtr)
> >     if (Instruction *PtrOp = dyn_cast<Instruction>(LoadedPtr))
> >     return false
> > 
> > ```
> > (IE what it was before).
> > 
> Do not we need the PredBB then ? we are not iterating over the predecessors here. Otherwise, I think doPhiTranslation would be a perfect fit for this.
good point, missed that. I would still simplify the logic


I would just define a helper:


```
static bool isOpDefinedInBlock(Value *Op, BasicBlock *BB)
{
  if (Instruction *OpInst = dyn_cast<Instruction>(Op))
     if (OpInst->getParent() == BB)
       return true;
  return false;
}
```

Then here
 if (opDefinedByBlock(LoadedPtr, LoadBB) && !isa<PHINode>(LoadedPtr))
   return false;

and below, just always phi translate (phi translate returns either the original pointer, or the phi translated version, it never returns null)


https://reviews.llvm.org/D30543