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

[Sanjay Patel via Phabricator via llvm-commits]

spatel added inline comments.


================
Comment at: lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp:2981
+    case SPF_ABS:
+      Opc = VT.isInteger() ? ISD::ABS : ISD::FABS;
+      IsUnary = true;
----------------
shchenz wrote:
> I think matchSelectionPattern can only find integer abs for now. Maybe we should only consider ISD::ABS for now and add ISD::FABS when matchSelectionPattern supports fabs?
This was discussed in PR37743. We already have an llvm.fabs intrinsic, so I don't see why matchSelectPattern() would ever need to match fabs(). 

matchSelectPattern() is used for common operations that do *not* have LLVM intrinsics, so they are implemented using 'select' IR.

The fabs code here should be removed from this patch.


================
Comment at: lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp:2985
+    case SPF_NABS:
+      // TODO: What DAG should be produced?
+      break;
----------------
I think SPF_NABS will be as described in the summary - sub(0, abs(X)).

Unless there is evidence that the negation gets separated from the ABS dag node and we're failing to produce 'nabs' codegen for targets that support that op? In that case, we might add a ISD::NABS node.

Either way, let's leave nabs for a follow-up patch.


Repository:
  rL LLVM

https://reviews.llvm.org/D49837