[PATCH] D27916: [RFC]Make the canonicalisation on shifts benifit to more case.

[jojo.ma via Phabricator via llvm-commits]

jojo added inline comments.


================
Comment at: lib/CodeGen/SelectionDAG/DAGCombiner.cpp:4548
+  // FIXME: disable this unless the input to the binop is a shift by a constant
+  // or is copy/select.Enable this in other cases when figure out it's exactly profitable.
   SDNode *BinOpLHSVal = LHS->getOperand(0).getNode();
----------------
rengolin wrote:
> Is this still a FIXME? I think this comment could be just about what you're avoiding here, since it should have fixed the pessimized case, no?
Indeed,it has fixed the  pessimized case mentioned in the original FIXME. But I'm afraid there are other profitable cases that are not covered here. So I wonder if it is more clearer leaving a FIXME here.


================
Comment at: test/CodeGen/ARM/shift-combine.ll:6
+define i32 @test_lshr_and1(i32 %x) {
+;CHECK-LABEL: test_lshr_and1
+;CHECK-NOT lsr
----------------
rengolin wrote:
> It would be good, at least as documentation, to `CHECK` for the instructions you want to see. Same on the case below.
Agree.That would be better.


https://reviews.llvm.org/D27916