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

[Qing Shan Zhang via Phabricator via llvm-commits]

steven.zhang marked 2 inline comments as done.
steven.zhang added inline comments.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp:21382
+            // Test = X == 0.0
+            Test = DAG.getSetCC(DL, CCVT, Op, FPZero, ISD::SETEQ);
         }
----------------
spatel wrote:
> 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.
PowerPC backend depends on the flags to determine how to lower select_cc between select and cmp+branch. I believe we can see the test difference. And yes, it is another patch.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp:21382
+            // Test = X == 0.0
+            Test = DAG.getSetCC(DL, CCVT, Op, FPZero, ISD::SETEQ);
         }
----------------
steven.zhang wrote:
> spatel wrote:
> > 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.
> PowerPC backend depends on the flags to determine how to lower select_cc between select and cmp+branch. I believe we can see the test difference. And yes, it is another patch.
> This thread may be relevant here: http://lists.llvm.org/pipermail/llvm-dev/2020-May/141561.html
Yeah, this seems to be a big hole of DAGCombine. We even don't have parameter to pass the flags for getSetCC now, though it could be set later. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80706