[PATCH] D142542: [InstSimplify] Simplify icmp between Left-Shifted VScale Calls

[Nikita Popov via Phabricator via llvm-commits]

nikic added inline comments.


================
Comment at: llvm/lib/Analysis/InstructionSimplify.cpp:3636
+  if (!match(LHS, m_Shl(m_Value(V), m_ConstantInt(ShiftAmountL))) ||
+      !match(RHS, m_Shl(m_Deferred(V), m_ConstantInt(ShiftAmountR))) ||
+      LHS->getType() != RHS->getType() || !isKnownNonZero(V, Q.DL))
----------------
Prefer m_APInt


================
Comment at: llvm/lib/Analysis/InstructionSimplify.cpp:3640
+
+  auto ShiftL = cast<BinaryOperator>(LHS);
+  auto ShiftR = cast<BinaryOperator>(RHS);
----------------
Use `cast<OverflowingBinaryOperator>`, otherwise this might crash for constant expressions.


================
Comment at: llvm/lib/Analysis/InstructionSimplify.cpp:3674
+    return nullptr;
+  };
+}
----------------
There is an ICmpInst::compare() helper that takes a predicate and APInts -- this code can probably be simplified based on that?


================
Comment at: llvm/lib/Analysis/InstructionSimplify.cpp:3991
+  if (Value *V = simplifyICmpLShiftedValues(Pred, LHS, RHS, Q))
+    return V;
+
----------------
Should probably be called from simplifyICmpWithBinOp. Also, it looks like a very similar fold already exists there: https://github.com/llvm/llvm-project/blob/bde5d31e96f556fed30bf9120cc6ff05a2116b64/llvm/lib/Analysis/InstructionSimplify.cpp#L3426-L3436 This seems to be the same, just on different operands of the shift. Possibly that could serve as inspiration on how to implement this more compactly.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142542