[PATCH] D29489: Optimize SETCC + VSEL of incompatible or illegal types

[Eli Friedman via Phabricator via llvm-commits]

efriedma added a comment.

Feel free to keep this together for now if it's easier, but eventually the changes to ComputeNumSignBits should be split into separate commits, with their own testcases.



================
Comment at: lib/CodeGen/SelectionDAG/LegalizeVectorTypes.cpp:2835
+
+  if (N->getOpcode() != ISD::VSELECT || Cond->getOpcode() != ISD::SETCC)
+    return SDValue();
----------------
Have you tried simplifying the handling of Cond using WidenTargetBoolean?


================
Comment at: lib/CodeGen/SelectionDAG/SelectionDAG.cpp:3026
+  case ISD::VECTOR_SHUFFLE: {
+  case ISD::BUILD_VECTOR:
+    unsigned Min = UINT_MAX;
----------------
It doesn't really make sense to handle VECTOR_SHUFFLE and BUILD_VECTOR together.  Maybe for VECTOR_SHUFFLE you can reuse the code from CONCAT_VECTORS?


================
Comment at: lib/CodeGen/SelectionDAG/SelectionDAG.cpp:3036
+        unsigned ExtrOpBits = Extr.getOperand(0).getScalarValueSizeInBits();
+        if (ExtrOpBits >= VTBits)
+          Tmp = ComputeNumSignBits(Op.getOperand(i)->getOperand(0), Depth + 1);
----------------
You aren't handling the case where ExtrOpBits > VTBits correctly (you have to subtract "ExtrOpBits - VTBits" bits from the result).

Can we avoid making BUILD_VECTORS in the cases where this would be necessary?


================
Comment at: lib/CodeGen/SelectionDAG/SelectionDAG.cpp:3049
+  case ISD::BITCAST:
+    return ComputeNumSignBits(Op.getOperand(0), Depth + 1);
   }
----------------
Bitcasts aren't transparent in general: the integer width changes, or the operand might not even be an integer.


https://reviews.llvm.org/D29489