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

Qing Shan Zhang via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri May 29 00:30:23 PDT 2020


steven.zhang marked an inline comment 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);
         }
----------------
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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80706





More information about the llvm-commits mailing list