[PATCH] D96596: [AArch64] Convert CMP/SELECT sign patterns to OR & ASR.

[Dave Green via Phabricator via llvm-commits]

dmgreen added inline comments.


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:6718
+        LHS.getValueType() == TVal.getValueType()) {
+      auto VT = LHS.getValueType();
+      auto Shift = DAG.getNode(
----------------
auto -> ValueType


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:6719
+      auto VT = LHS.getValueType();
+      auto Shift = DAG.getNode(
+          ISD::SRA, dl, VT, LHS,
----------------
auto -> SDValue


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:6721
+          ISD::SRA, dl, VT, LHS,
+          DAG.getConstant(VT.getSizeInBits().getFixedValue() - 1, dl, VT));
+      auto Or = DAG.getNode(ISD::OR, dl, VT, Shift, DAG.getConstant(1, dl, VT));
----------------
I don't think this needs getFixedValue()


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:6722
+          DAG.getConstant(VT.getSizeInBits().getFixedValue() - 1, dl, VT));
+      auto Or = DAG.getNode(ISD::OR, dl, VT, Shift, DAG.getConstant(1, dl, VT));
+      return Or;
----------------
auto -> SDValue (or just return directly)


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:14990-14991
+  SDValue SetCC = N->getOperand(0);
+  if (SetCC->getOpcode() == ISD::SETCC &&
+      SetCC->getOperand(2) == DAG.getCondCode(ISD::SETGT)) {
+    SDValue CmpLHS = SetCC->getOperand(0);
----------------
SetCC-> -> SetCC.


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:15004
+                          MVT(MVT::v2i64)}),
+            VT.getSimpleVT()) &&
+        SplatLHS->getConstantSplatNode() &&
----------------
VT.getSimpleVT().SimpleTy, then I think you can drop all the extra MVT()'s.


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:15007
+        SplatLHS->getConstantSplatNode()->isOne() &&
+        SplatRHS->getConstantSplatNode() &&
+        SplatRHS->getConstantSplatNode()->isAllOnesValue() && CmpRHS &&
----------------
These is a ISD::isConstantSplatVectorAllOnes, but I don't know if there is one for "IsOne"


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96596