The NaN part LGTM, but what happens on 0?
select +0.0, -0.0, (fcmp lt +0.0, -0.0)
fminnum +0.0, -0.0
My understanding is, the first returns -0.0 (because they compare equal, and not "lt").
But it's unspecified which the second returns, so if the implementation returns the first operand when both are zero, it would return a "different" result here, +0.0.
Does that make sense?
Nit: clang-format? (or at least the same formatting as the call, with e.g. LHS/RHS on the same line.)
Optionally, how about checking isKnownNeverNaN for both operands?
Nit: same, format?
Yes, however I am unclear on what guarantees there are for signed zeros. The DAG TargetOptions are also missing the equivalent of No Signed Zeros. I can weaken this to unsafe FP Math as well.
I didn't know about isKnownNeverNaN, I'll switch to using it.
They do care about NaN. The ordered compares fail if either operand is a NaN, and the unordered succeed if either is.
I believe the X86 equivalent of this combine swaps operands to make sure it gets the same result, but that's only possible because the operation is specified there.
Other knowledgeable FP people, an opinion?
I meant the non-orderered, and non-unordered, compares, as e.g., ISD::SETLT, ISD::SETGT. Those are undefined on NaNs, no?
What about renaming True TrueValue and False FalseValue or something like that. I was confused with c++ true/false keyword when reading this in the first place.
Can't you move the code out of the switch and having the switch only initializing the OpCode?
Isn't the transformation valid for SETGT, SETGE, SETLT, SETLE even without Options.NoNaNsFPMath?
Why is the limit N0.hasOneUse()?
It is the correct way of indenting the arg list here?
Yes, but I don't like assigning variables like that and having to track them
Oh, those. Yes, i it would be OK for those. However, I don't think I've ever actually seen one of those produced for FP compares before.
This is what clang-format produced