[PATCH] D30708: [ARM] SMULW [T|B] DAG Combine

[Eli Friedman via Phabricator via llvm-commits]

efriedma added a comment.

Please add some testcases covering the improved isS16 function.



================
Comment at: lib/Target/ARM/ARMISelLowering.cpp:1482
+
+static bool isS16(const SDValue &Op, SelectionDAG &DAG) {
+  if (isSRA16(Op))
----------------
Add a comment describing what exactly this function is checking for?  It's not obvious why a function called "isS16" is special-casing SRA nodes.


================
Comment at: lib/Target/ARM/ARMISelLowering.cpp:9967
+  SDNode *SMULLOHI = SRL.getOperand(0).getNode();
+  if (SRL.getOperand(0) != SDValue(SMULLOHI, 0) ||
+      SHL.getOperand(0) != SDValue(SMULLOHI, 1))
----------------
getResNo, maybe?


================
Comment at: lib/Target/ARM/ARMISelLowering.cpp:11835
     return PerformARMBUILD_VECTORCombine(N, DCI);
+  case ARMISD::SMULWB:
+    if (simplifyS16(N, 1, DCI))
----------------
You could also simplify SMULWT nodes in a similar manner.


https://reviews.llvm.org/D30708