[PATCH] D70582: [FPEnv][X86] Constrained FCmp intrinsics enabling on X86

[Pengfei Wang via Phabricator via llvm-commits]

pengfei marked an inline comment as done.
pengfei added inline comments.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/LegalizeDAG.cpp:3577
     EVT VT = Node->getValueType(0);
+    SDValue Chain;
     SDValue CC = Node->getOperand(4);
----------------
pengfei wrote:
> craig.topper wrote:
> > pengfei wrote:
> > > uweigand wrote:
> > > > I guess this needs an input chain, right?   I think you'll probably have to add a whole new STRICT_FSELECT_CC node to get this right ...
> > > Since we are supporting both signaling and quite STRICT intrinsics, I think there's no need to expand the STRICT version of `SETCC`, `SELECT_CC` and `BR_CC` now.
> > > That's because `setCondCodeAction` doesn't distinguish signaling and quite. Obviously, some CC that is legal in signaling or quite might be not in another.
> > > The only way is customizing them, otherwise falling back to non STRICT node.
> > Is it never legal to fall back to a non strict node. Once all targets support strict fp all mutation code will be removed from the tree. We might need to enhance setCondCodeAction to understand signaling.
> We can enhance setCondCodeAction, but it's hard to enhance the expansion math here.
> I did some research work on SSE and found we could happen expand to all 32 cases using the 8 legal CCs. But it is based on we know the behavior of each 8 CCs, so it can be hard coded. I think it's hard to give a general expansion in common code and may be easily result into infinite loop because we have to expand more than once for some CCs in SSE.
> I agreed falling back to a non strict node is meanness. I think currently we have provided scalarization for vector operations' falling back. For targets support strict fp, they should at least support all cases in scalar operations either legal or custom.
I think you are correct! After some research, I believe we still need to enhance setCondCodeAction to at least get the action is legal or custom. And a minimal expansion like what non strict node doing is still useful. Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70582