[PATCH] D73194: [FPEnv][ARM] Handle expansion of strict SETCC with legal condition code

Thu Jan 23 05:53:56 PST 2020

john.brawn marked an inline comment as done.
john.brawn added inline comments.

================
Comment at: llvm/lib/CodeGen/SelectionDAG/LegalizeDAG.cpp:3672
+    // using that condition code. If we have a strict SETCC the condition code
+    // may already be legal, in which case we similarly convert to a non-strict
+    // SETCC with the condition code we already have.
----------------
craig.topper wrote:
> john.brawn wrote:
> > craig.topper wrote:
> > > Why is it ok to turn a STRICT node into a non-strict node?
> > Well, it's what we're doing already in the case that a STRICT_FSETCCS has a condition code that LegalizeSetCCCondCode can make legal by swapping the operands - or at least I think so from reading the code, the situation where I'm seeing FSETCCS is from the expansion of STRICT_FP_TO_UINT where that doesn't happen so I don't know of a way to check if that is the case.
> > 
> > If converting to non-strict isn't right, then what should be done instead?
> That does look like a bug, but probably not tested.
> 
> The ARM target will need to replicate whatever it does for floating point ISD::SETCC to also handle ISD::STRICT_FSETCC/FSETCCS. That may require fixing the SELECT_CC FIXME below if ARM uses that. You may need new STRICT_ target specific nodes that have chain input and output. And new isel patterns to match them.
The ARM target has SETCC set to expand which means it gets converted to SELECT_CC. I would expect that a similar expansion for STRICT_FSETCC would require adding a STRICT_SELECT_CC node if we don't want to lose strictness.

I think probably it's easier for the ARM target to have lowering for STRICT_FSETCC and STRICT_FSETCCS, given that we have the VCMP and VCMPE instructions that correspond to the two, so I'll do that.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73194