[PATCH] D43759: [SCEV] Smarter logic in computeConstantDifference

[Sanjoy Das via Phabricator via llvm-commits]

sanjoy requested changes to this revision.
sanjoy added inline comments.
This revision now requires changes to proceed.


================
Comment at: lib/Analysis/ScalarEvolution.cpp:9396
+  if (auto *Difference = dyn_cast<SCEVConstant>(
+          getMinusSCEV(More, Less, SCEV::FlagAnyWrap, MaxArithDepth - 1)))
+    return Difference->getAPInt();
----------------
I'd much rather not put all of this (albeit non-recursive) work here when it is possible to easily extend the existing code to handle your pattern.


================
Comment at: lib/Analysis/ScalarEvolution.cpp:9455
 
   Optional<APInt> LDiff = computeConstantDifference(LHS, FoundLHS);
   Optional<APInt> RDiff = computeConstantDifference(RHS, FoundRHS);
----------------
mkazantsev wrote:
> Also incorrect, because types of LHS and FoundLHS doesn't have to match.
Do you have a test case for this?  `ScalarEvolution::isImpliedCond` should be zero/sign extending LHS/FoundLHS to make both of them have the same type before calling `ScalarEvolution::isImpliedCondOperands`.  If there are other callsites that don't respect this invariant they should zero/sign extend as well.


https://reviews.llvm.org/D43759