[PATCH] D149027: [X86] Add peephole to convert `(Cmp Op32/Op64, Imm8)` -> `(Cmp Op16/Op8, Imm8)`

[Craig Topper via Phabricator via llvm-commits]

craig.topper added inline comments.


================
Comment at: llvm/lib/Target/X86/X86ISelDAGToDAG.cpp:5644
+    // during lowering.
+    if (OpVT.isScalarInteger() && OpVT.getScalarSizeInBits() > 8 &&
+        isa<ConstantSDNode>(N1) && !isNullConstant(N1) &&
----------------
OpVT.isScalarInteger() should always bee true for X86ISD::CMP I think. The isa<ConstantSDNode>(N1) would guarantee it too.


================
Comment at: llvm/lib/Target/X86/X86ISelDAGToDAG.cpp:5646
+        isa<ConstantSDNode>(N1) && !isNullConstant(N1) &&
+        hasNoSignFlagUses(SDValue(Node, 0))) {
+      const APInt &C = cast<ConstantSDNode>(N1)->getAPIntValue();
----------------
Do we need to check overflow flag uses too?


================
Comment at: llvm/lib/Target/X86/X86ISelDAGToDAG.cpp:5650
+      // cause LCP stalls in the frontend.
+      // TODO: Enable imm16 transform aswell if -Os is set?
+      if (C.getSignificantBits() <= 8) {
----------------
aswell -> as well


================
Comment at: llvm/lib/Target/X86/X86ISelDAGToDAG.cpp:5653
+          MVT NewVT;
+          if(CurDAG->MaskedValueIsZero(
+                 N0, APInt::getBitsSetFrom(OpVT.getScalarSizeInBits(), 8)))
----------------
Space after if


================
Comment at: llvm/lib/Target/X86/X86ISelDAGToDAG.cpp:5656
+              NewVT = MVT::i8;
+          else if(CurDAG->MaskedValueIsZero(
+                 N0, APInt::getBitsSetFrom(OpVT.getScalarSizeInBits(), 16)))
----------------
space after if


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149027