[PATCH] D141653: [X86] Improve instruction ordering of constant `srl/shl` with `and` to get better and-masks

[Craig Topper via Phabricator via llvm-commits]

craig.topper added inline comments.


================
Comment at: llvm/lib/Target/X86/X86ISelLowering.cpp:47494
+  // thats easier to create.
+  if (VT != MVT::i8 && VT != MVT::i16 && VT != MVT::i32 && VT != MVT::i64)
+    return SDValue();
----------------
You could check `!VT.isScalarInteger()` instead.


================
Comment at: llvm/lib/Target/X86/X86ISelLowering.cpp:47554
+                         DAG.getConstant(NewAndMask, DL, VT));
+    else
+      return DAG.getNode(ShiftOp->getOpcode(), DL, VT,
----------------
Drop else after return.


================
Comment at: llvm/lib/Target/X86/X86ISelLowering.cpp:56345
+  if (N->getOpcode() == ISD::SRL || N->getOpcode() == ISD::SHL)
+    for (unsigned OpIdx = 0; OpIdx < 2; ++OpIdx)
+      if (N->getOperand(OpIdx).getOpcode() == ISD::AND)
----------------
Why checking both operands of the shift? Isn't only the first one interesting? An AND on the shift amount is very different.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141653