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

[Sanjay Patel via Phabricator via llvm-commits]

spatel 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);
         }
----------------
steven.zhang wrote:
> steven.zhang wrote:
> > 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. 
> Can we add a verifier in DAG to verify the flag ? i.e. Checking that, there is no flags missed during the dagcombine.
I'm not seeing how it would work because the flags are always optional and not necessarily propagated from the operands. But if you see a way to do it, that would be a nice enhancement.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80706