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

Matt Arsenault Matthew.Arsenault at amd.com
Mon Jan 12 17:47:12 PST 2015


================
Comment at: lib/CodeGen/SelectionDAG/DAGCombiner.cpp:4656
@@ +4655,3 @@
+  }
+}
+
----------------
joker.eph wrote:
> Can't you move the code out of the switch and having the switch only initializing the OpCode? 
Yes, but I don't like assigning variables like that and having to track them

================
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:
> arsenm wrote:
> > ab wrote:
> > > joker.eph wrote:
> > > > joker.eph wrote:
> > > > > Isn't the transformation valid for SETGT, SETGE, SETLT, SETLE even without Options.NoNaNsFPMath?
> > > > Why is the limit N0.hasOneUse()?
> > > > 
> > > 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.
> I meant the non-orderered, and non-unordered, compares, as e.g., ISD::SETLT, ISD::SETGT. Those are undefined on NaNs, no?
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. 

================
Comment at: lib/CodeGen/SelectionDAG/DAGCombiner.cpp:4750
@@ +4749,3 @@
+        N1, N2,
+        CC, TLI, DAG);
+      if (FMinMax)
----------------
ab wrote:
> joker.eph wrote:
> > It is the correct way of indenting the arg list here?
> Nit: same, format?
This is what clang-format produced

http://reviews.llvm.org/D6744

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/






More information about the llvm-commits mailing list