[PATCH] D44102: Teach CorrelatedValuePropagation to reduce the width of udiv/urem instructions.

[Justin Lebar via Phabricator via llvm-commits]

jlebar added a comment.

> I have a hunch that using the getConstantRange call will "fix" this issue

It does, thanks!  It also simplifies the code.

> though getPredicateAt definitely should be not weaker than calling getConstantRange and inferring the predicate manually.

Agree.  This is a bit of a Chesterton's Fence for me, though.  Why would we ever want to do getValueAt as opposed to getValueInBlock?  getValueAt is called only by getPredicateAt, and...getPredicateAt *has* a BB, namely CtxI->getParent().  So why not just call getValueInBlock from there, and delete getValueAt?  Further adding to my confusion is the fact that the header says that getPredicateAt (only?) looks at assume intrinsics.  So it sort of seems intentional, but I have no idea why...

The only users of getPredicateAt are CVP and jump threading.  I tried switching getPredicateAt to call getValueInBlock, and it seems to be fine for CVP, but it breaks jump threading tests in what seems to me to be a real way.

  diff --git a/llvm/lib/Analysis/LazyValueInfo.cpp b/llvm/lib/Analysis/LazyValueInfo.cpp
  index 65fd007dc0b2..04d2143bb904 100644
  --- a/llvm/lib/Analysis/LazyValueInfo.cpp
  +++ b/llvm/lib/Analysis/LazyValueInfo.cpp
  @@ -1701,7 +1701,8 @@ LazyValueInfo::getPredicateAt(unsigned Pred, Value *V, Constant *C,
       else if (Pred == ICmpInst::ICMP_NE)
         return LazyValueInfo::True;
     }
  -  ValueLatticeElement Result = getImpl(PImpl, AC, &DL, DT).getValueAt(V, CxtI);
  +  ValueLatticeElement Result =
  +      getImpl(PImpl, AC, &DL, DT).getValueInBlock(V, CxtI->getParent(), CxtI);
     Tristate Ret = getPredicateResult(Pred, C, Result, DL, TLI);
     if (Ret != Unknown)
       return Ret;

breaks

  LLVM :: Analysis/LazyValueAnalysis/lvi-after-jumpthreading.ll
  LLVM :: Transforms/JumpThreading/induction.ll

I stared at the jump threading code for a while and it's not at all clear to me why this new implementation of getPredicateAt is wrong for it.


https://reviews.llvm.org/D44102