[PATCH] D49229: [InstCombine] Fold redundant masking operations of shifted value

[Roman Lebedev via Phabricator via llvm-commits]

lebedev.ri added inline comments.


================
Comment at: lib/Transforms/InstCombine/InstCombineAndOrXor.cpp:2022
+// is taken, then this function can be removed.
+static Instruction *foldRedundantMask(BinaryOperator &I, Value *A, Value *B,
+                                      InstCombiner::BuilderTy &Builder) {
----------------
I don't think `A` and `B` are used anymore?


================
Comment at: lib/Transforms/InstCombine/InstCombineAndOrXor.cpp:2028
+  // 1st: Check for desired pattern
+  if (match(&I, m_c_Or(m_And(m_OneUse(m_CombineAnd(
+                                 m_Shift(m_Value(X), m_APInt(ShftAmt)),
----------------
We create two new instructions, but we only make sure that one instruction (`Shift`, i think?) goes away.
The outermost `m_And()` should too be `m_OneUse()`, i think.


================
Comment at: lib/Transforms/InstCombine/InstCombineAndOrXor.cpp:2033
+                       m_CombineAnd(m_And(m_Deferred(X), m_APInt(AndC)),
+                                    m_BinOp(And))))) {
+    const Instruction::BinaryOps Opcode = Shift->getOpcode();
----------------
Early return please.


https://reviews.llvm.org/D49229