[PATCH] D142542: [InstSimplify] Simplify icmp between Shl instructions of the same value

[Matt Devereau via Phabricator via llvm-commits]

MattDevereau marked an inline comment as done.
MattDevereau added inline comments.


================
Comment at: llvm/lib/Analysis/InstructionSimplify.cpp:3273
+  if (ICmpInst::isSigned(Predicate) &&
+      (!LargestShift->hasNoUnsignedWrap() || !LargestShift->hasNoSignedWrap()))
+    return nullptr;
----------------
nikic wrote:
> These checks should go through Q.IIQ.
I've made them use Q.IIQ now, and I've moved the checks a level higher, since similar code in simplifyICmpWithBinOp exists for when the shift amounts are equal for an ICmp with two shl operands there.


================
Comment at: llvm/lib/Analysis/InstructionSimplify.cpp:3281
+    return getTrue(getCompareTy(LHS));
+  return getFalse(getCompareTy(RHS));
+}
----------------
nikic wrote:
> I believe we could `return simplifyICmpInst(Pred, ShiftAmountL, ShiftAmountR, Q, MaxRecurse -1)` here, without the limitation to constant shift amounts. The only disadvantage to that would be that we would have to require the nuw/nsw flags on both shifts, rather than only on the largest one (as we wouldn't know which one is the largest is without further effort).
> 
> This would allow handling comparisons between `shl %v, %s` and `%shl %v, (add nuw %s, 1)` or similar.
Hopefully I've understood what you mean here and implemented this correctly. I inverted the logic of the matches and replaced `return getFalse(getCompareTy(LHS));` with your suggestion which works ok, however returning your suggestion for any non match caused several test failures elsewhere and resulted in some miscompiles


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142542