[PATCH] D38037: [InstCombine] Compacting or instructions whose operands are shift instructions

[Omer Paparo Bivas via Phabricator via llvm-commits]

opaparo marked 4 inline comments as done.
opaparo added inline comments.


================
Comment at: lib/Transforms/InstCombine/InstCombineAndOrXor.cpp:2237
+    // match (V&C1)<<C2
+    static auto MatchShiftOfAnd =
+        [](Value *V, Value *&Source, Instruction::BinaryOps &ShiftOpcode,
----------------
craig.topper wrote:
> Shift of Ands are canonicalized to And of Shifts unless the 'and' has an additional use. I don't think your test cases cover that.
The lambda MatchShiftOfAnd was indeed unnecessary. I've removed it.


================
Comment at: lib/Transforms/InstCombine/InstCombineAndOrXor.cpp:2305
+            Builder.CreateBinOp(ShiftOpcode0, MaskedSource, ShiftBy0);
+        Op0->replaceAllUsesWith(NewOp0);
+        Value *NewOp1 =
----------------
craig.topper wrote:
> Why the replaceAllUsesWith call? We don't normally do this in InstCombine. Are you trying to replace other uses than this OR? Do you have test cases for that?
The calls to replaceAllUsesWith were indeed unnecessary. I removed them.


https://reviews.llvm.org/D38037