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

[Sander de Smalen via Phabricator via llvm-commits]

sdesmalen added inline comments.


================
Comment at: llvm/lib/Analysis/InstructionSimplify.cpp:3258
 
+static Value *simplifyICmpWithShl(CmpInst::Predicate Predicate, Value *LHS,
+                                  Value *RHS, const SimplifyQuery &Q,
----------------
This function isn't really needed, see my other comment further down for details.


================
Comment at: llvm/lib/Analysis/InstructionSimplify.cpp:3414
 
-  if (MaxRecurse && LBO && RBO && LBO->getOpcode() == RBO->getOpcode() &&
-      LBO->getOperand(1) == RBO->getOperand(1)) {
-    switch (LBO->getOpcode()) {
-    default:
-      break;
-    case Instruction::UDiv:
-    case Instruction::LShr:
-      if (ICmpInst::isSigned(Pred) || !Q.IIQ.isExact(LBO) ||
-          !Q.IIQ.isExact(RBO))
+  if (MaxRecurse && LBO && RBO && LBO->getOpcode() == RBO->getOpcode()) {
+    if (LBO->getOperand(0) == RBO->getOperand(0)) {
----------------
It might be useful to bail out here if this part of the condition is false, such that you can have two indented blocks below:

  if (!MaxRecurse || !LBO || !RBO || LBO->getOpcode() != RBO->getOpcode())
    return nullptr;

  if (LBO->getOperand(0) == RBO->getOperand(0)) {
    ..
  }

  if (LBO->getOperand(1) == RBO->getOperand(1)) {
    ..
  }

  return nullptr;

That avoids having to indent the existing block (`LBO->getOperand(1) == RBO->getOperand(1) `) below.


================
Comment at: llvm/lib/Analysis/InstructionSimplify.cpp:3424
+          break;
+        if (Value *V = simplifyICmpWithShl(Pred, LBO, RBO, Q, MaxRecurse - 1))
+          return V;
----------------
Rather than calling `simplifyICmpWithShl` here, you can just compare the shift-amounts directly, i.e.

         if (Value *V = simplifyICmpInst(Pred, LBO->getOperand(1),
                                        RBO->getOperand(1), Q, MaxRecurse - 1))
          return V;

You only need to add an extra check to ensure that LBO->getOperand(0) is known non-zero.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142542