[PATCH] D80706: [DAGCombine] Add hook to allow target specific test for sqrt input

[Sanjay Patel via Phabricator via llvm-commits]

spatel added a comment.

See inline for a minor improvement, otherwise LGTM. Someone from PPC should provide final approval after looking at the test diffs.



================
Comment at: llvm/include/llvm/CodeGen/TargetLowering.h:4197-4199
+  /// Return a value for the input operand to do the test to determine if it is
+  /// NAN, zero, infinity, negative or denormalized value which is target
+  /// dependent.
----------------
The description did not read clearly to me. How about:
"Return a target-dependent comparison result if the input operand is suitable for use with a square root estimate calculation. For example, the comparison may check if the operand is NAN, INF, zero, normal, etc. The result should be used as the condition operand for a select or branch."

If the plan is to generalize this hook for 'ftdiv' as well, then we can make the description less specific.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp:21382
+            // Test = X == 0.0
+            Test = DAG.getSetCC(DL, CCVT, Op, FPZero, ISD::SETEQ);
         }
----------------
nemanjai wrote:
> steven.zhang wrote:
> > Question here: do we miss to propagate the SDFlags for SETCC which might affect how we lower the select_cc ?
> This thread may be relevant here: http://lists.llvm.org/pipermail/llvm-dev/2020-May/141561.html
Yes, I think we are missing propagation of flags on all of the created nodes in this sequence. I don't know if we can induce any test differences from that with current regression tests, but that can be another patch.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D80706/new/

https://reviews.llvm.org/D80706