[PATCH] D49837: [SelectionDAG][X86] Handle unary SelectPatternFlavor for ABS case in SelectionDAGBuilder::visitSelect.

[Ivan Kulagin via Phabricator via llvm-commits]

ikulagin added inline comments.


================
Comment at: lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp:3004
+
+    if (IsUnary) {
+      OpCode = Opc;
----------------
RKSimon wrote:
> Shouldn't we only do this if TLI.isOperationLegalOrCustom(Opc, VT) ? That probably removes the need for some of the legalization code from this initial patch.
Yes, I think so too, It removes the need for some legalization, but in this case for the absolute value code the DAGBuilder can produce 2 different DAGs (one of which is the select node and the other is the ABS node). This will lead to the more complex DAGCombiner, because the DAGCombiner should support the same patterns with different DAGs. For example, the X86 recognizer of SAD pattern will be needed to handle the case based on the SELECT node and the case based on the ABS node. I think that this makes code bloat with the redundant supported patterns.
Also, the DAG based on the SELECT has 3 nodes (SELECT, SUB, SETCC) and on the ABS only 1.

I have gone deeper into this task and I have some thoughts.
What if we make the DAGBuilder and the Legalization for the ABS more general? I mean that we can always generate the ABS and on the Legalization stage we can perform a simple transformation, but on the Combine stage we can produce the optimal architecture-specific DAG with more complexity transformations.
I think it will allow building a DAG with a smaller number of nodes and performing a legalization of a smaller number of nodes too.
What do you think about this?


Repository:
  rL LLVM

https://reviews.llvm.org/D49837