[PATCH] D47725: [SelectionDAG] Provide default expansion for rotates

[Simon Pilgrim via Phabricator via llvm-commits]

RKSimon added a reviewer: RKSimon.
RKSimon added inline comments.


================
Comment at: lib/CodeGen/SelectionDAG/DAGCombiner.cpp:4985
+    return TLI.isOperationLegalOrCustom(Opc, T);
+  };
+  bool HasROTL = HasOperation(ISD::ROTL, VT);
----------------
This might be useful as general helper function - add it to the DAGCombiner class for reuse?


================
Comment at: lib/CodeGen/SelectionDAG/LegalizeDAG.cpp:3912
+    EVT ShVT = Op1.getValueType();
+    SDValue Width = DAG.getConstant(OpVT.getSizeInBits(), dl, ShVT);
+    SDValue Sub = DAG.getNode(ISD::SUB, dl, ShVT, Width, Op1);
----------------
Use OpVT.getScalarSizeInBits() - hopefully we might be able to use this for vector cases as well at some point soon.


================
Comment at: lib/CodeGen/SelectionDAG/LegalizeDAG.cpp:3918
+    if (TLI.isOperationLegal(RevRot, ResVT)) {
+      Results.push_back(DAG.getNode(RevRot, dl, ResVT, Op0, Sub));
+      break;
----------------
Move the Sub = DAG.getNode into the if() case? I don't think its used anywhere else.


================
Comment at: lib/Target/Hexagon/HexagonISelLowering.cpp:1359
   setOperationAction(ISD::BSWAP, MVT::i64, Legal);
-  setOperationAction(ISD::MUL,   MVT::i64, Legal);
 
----------------
Is this relevant to rotation support?


================
Comment at: lib/Target/Hexagon/HexagonISelLowering.cpp:1366
+    for (MVT VT : MVT::integer_valuetypes())
+      setOperationAction(IntExpOp, VT, Expand);
   }
----------------
Is this relevant to rotation support?


================
Comment at: lib/Target/Hexagon/HexagonISelLowering.cpp:2100
+HexagonTargetLowering::LowerROTL(SDValue Op, SelectionDAG &DAG) const {
+  if (isa<ConstantSDNode>(Op.getOperand(1).getNode()))
+    return Op;
----------------
I don't think you need the getNode() here


Repository:
  rL LLVM

https://reviews.llvm.org/D47725