[PATCH] D114555: [ScalarEvolution] Add bailout to avoid zext of pointer.

[Eli Friedman via Phabricator via llvm-commits]

efriedma added a comment.

The testcase isn't that complicated in terms of the size of the IR, but it would be hard to ensure it continues to test the codepath in question. The actual crash involves many levels of recursive calls in SCEV, and we aren't expecting any useful information out of the isImpliedCond analysis in this case.  I could include it anyway, I guess.



================
Comment at: llvm/lib/Analysis/ScalarEvolution.cpp:10800
       getTypeSizeInBits(FoundLHS->getType())) {
-    if (FoundLHS->getType()->isPointerTy())
+    if (FoundLHS->getType()->isPointerTy() || FoundRHS->getType()->isPointerTy())
       return false;
----------------
mkazantsev wrote:
> This doesn't make sense to me. I'd expect that both `FoundLHS` and `FoundRHS` should be of the same type. How come they are different? Isn't it the real reason of the bug?
Like we found doing the review for bfa2a81e, the pointer-ness of the LHS/RHS should match for an icmp instruction, but ScalarEvolution makes other calls recursively.  See, for example, ScalarEvolution::isImpliedCondOperandsHelper.

At one point, I was considering using ptrtoint on all pointer inputs to isImpliedCond, but that turned out to be a little more complicated than I expected, and not really necessary.  I could revive that, I guess.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114555