[PATCH] Combine fcmp + select to fminnum / fmaxnum if no nans and legal

Matt Arsenault Matthew.Arsenault at amd.com
Mon Jan 12 15:39:41 PST 2015

In http://reviews.llvm.org/D6744#107740, @ab wrote:

> The NaN part LGTM, but what happens on 0?
> Say:
>   select +0.0, -0.0, (fcmp lt +0.0, -0.0)
> turns into:
>   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?

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.

> -Ahmed


Comment at: lib/CodeGen/SelectionDAG/DAGCombiner.cpp:4743
@@ +4742,3 @@
+    const TargetOptions &Options = DAG.getTarget().Options;
+    if (Options.NoNaNsFPMath && VT.isFloatingPoint() && N0.hasOneUse()) {
+      ISD::CondCode CC = cast<CondCodeSDNode>(N0.getOperand(2))->get();
ab wrote:
> Optionally, how about checking isKnownNeverNaN for both operands?
> Also, the non-U/-O CondCodes don't care about NaNs, so it would be fine to bypass NoNaNsFPMath in that case as well, no?  Though I'm not sure that combination ever happens anyway.
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.



More information about the llvm-commits mailing list