[Mips] Convert -mnan=legacy to nan2008 when architecture support nan2008

[yingopq]

Because we now do not really support legacy, so let us do the following:
a. Convert -mnan=legacy to nan2008 when architecture support nan2008;
b. Ignore -mnan=legacy when architecture does not support nan2008;
c. All the above report a warning.

Fix #100495.

[llvmbot]

@llvm/pr-subscribers-clang-driver
@llvm/pr-subscribers-clang
@llvm/pr-subscribers-llvm-selectiondag

@llvm/pr-subscribers-backend-mips

Author: None (yingopq)

Changes

The correct value is:
Single floating-point 0x7fbf ffff(legacy) 0x7fc0 0000(nan2008)
Double floating-point 0x7ff7 ffff ffff ffff(legacy) 0x7ff8 0000 0000 0000(nan2008)

---

Full diff: https://github.com/llvm/llvm-project/pull/153777.diff

2 Files Affected:

• (modified) llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp (+29-2)
• (added) llvm/test/CodeGen/Mips/fminimum-fmaximum.ll (+18)

diff --git a/llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp b/llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp
index 911bbabc42aa3..9758602685c00 100644
--- a/llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp
@@ -8773,8 +8773,35 @@ SDValue TargetLowering::expandFMINIMUM_FMAXIMUM(SDNode *N,
   // Propagate any NaN of both operands
   if (!N->getFlags().hasNoNaNs() &&
       (!DAG.isKnownNeverNaN(RHS) || !DAG.isKnownNeverNaN(LHS))) {
-    ConstantFP *FPNaN = ConstantFP::get(*DAG.getContext(),
-                                        APFloat::getNaN(VT.getFltSemantics()));
+    ConstantFP *FPNaN = NULL;
+    bool hasNaN2008 = false;
+    if (DAG.getTarget().getTargetTriple().isMIPS()) {
+      const MCSubtargetInfo *STI = getTargetMachine().getMCSubtargetInfo();
+      StringRef FeatureString = STI->getFeatureString();
+      SubtargetFeatures Features(FeatureString);
+      for (const std::string &Feature : Features.getFeatures()) {
+        if (Feature == "+nan2008")
+          hasNaN2008 = true;
+      }
+      if (hasNaN2008 == false) {
+        if (&VT.getFltSemantics() == &APFloat::IEEEsingle()) {
+          APInt intPayload(64, 0xbfffff);
+          FPNaN = ConstantFP::get(
+              *DAG.getContext(),
+              APFloat::getSNaN(VT.getFltSemantics(), false, &intPayload));
+        } else if (&VT.getFltSemantics() == &APFloat::IEEEdouble()) {
+          APInt intPayload(64, 0x7ffffffffffff);
+          FPNaN = ConstantFP::get(
+              *DAG.getContext(),
+              APFloat::getSNaN(VT.getFltSemantics(), false, &intPayload));
+        }
+      } else
+        FPNaN = ConstantFP::get(*DAG.getContext(),
+                                APFloat::getNaN(VT.getFltSemantics()));
+    } else
+      FPNaN = ConstantFP::get(*DAG.getContext(),
+                              APFloat::getNaN(VT.getFltSemantics()));
+
     MinMax = DAG.getSelect(DL, VT, DAG.getSetCC(DL, CCVT, LHS, RHS, ISD::SETUO),
                            DAG.getConstantFP(*FPNaN, DL, VT), MinMax, Flags);
   }
diff --git a/llvm/test/CodeGen/Mips/fminimum-fmaximum.ll b/llvm/test/CodeGen/Mips/fminimum-fmaximum.ll
new file mode 100644
index 0000000000000..0121e8905303f
--- /dev/null
+++ b/llvm/test/CodeGen/Mips/fminimum-fmaximum.ll
@@ -0,0 +1,18 @@
+; RUN: llc -mtriple=mips64el-unknown-linux-gnuabi64 -mcpu=mips64r2 -mattr=+nan2008 < %s | FileCheck -check-prefixes=NAN2008 %s
+; RUN: llc -mtriple=mips64el-unknown-linux-gnuabi64 -mcpu=mips64r2 -mattr=-nan2008 < %s | FileCheck -check-prefixes=LEGACY %s
+
+define float @test_fminimum_f32(float %a, float %b) {
+; NAN2008:    .4byte 0x7fc00000    # float NaN
+;
+; LEGACY:    .4byte 0x7fbfffff    # float NaN
+  %val = tail call float @llvm.minimum.f32(float %a, float %b)
+  ret float %val
+}
+
+define double @test_fminimum_f64(double %a, double %b) {
+; NAN2008:    .8byte 0x7ff8000000000000    # double NaN
+;
+; LEGACY:    .8byte 0x7ff7ffffffffffff    # double NaN
+  %val = tail call double @llvm.minimum.f64(double %a, double %b)
+  ret double %val
+}

[RKSimon]

Its not acceptable to have target specific code like this in the generic DAG combines - either you need to convert the NaNs in MIPS ISD::ConstantFP custom lowering or in MipsISelDAGToDAG or (if we have to......) consider a TLI.getNaN() callback

[arsenm]

LLVM as a whole does not support the legacy mips nan encoding. At best you're doing spot fixes for specific issues someone encounters. We should probably just turn the flag into an error; it would be a huge undertaking to actually support it correctly

[yingopq]

Thanks, my current idea is to fix the display problem first.

[RKSimon]

Missing a short dxoygen description

[RKSimon]

why do you need a (self) TLI argument when its a TLI method?

[yingopq]

OK, I would modify it.

[RKSimon]

ConstantFP *FPNaN = ConstantFP::get(*DAG.getContext(), getNaNValue(VT));

[arsenm]

Avoid repeated getFltSemantics calls. Plus this should be rewritten generically to not depend on the exact format, but derived from the format properties

[arsenm]

I do not want to add this hook. This is not how you would begin handling this properly. If you really, really want to hack this, it needs to be fully isolated in the mips backend. I do not want to taint any generic code without a strategy and intention to really support the old encoding. No code should have an obligation to try to handle or use this hook

[yingopq]

OK, thanks!
I would delete this hook and make this code only in the backend.

[nikic]

It might make sense to start a discussion on discourse on what to do with legacy MIPS NaN. What you do here is going to be a very partial solution, because we have tons of code that assumes IEEE NaNs.

I'm not even sure what it would take to really support this. I guess the proper way would be to add a new APFloat fltSemantics for legacy MIPS floats, but then it's unclear how those would map to the IR floating point types.

We should probably also consider making -mnan=legacy an error and stop pretending that we support it.

[yingopq]

We should probably also consider making -mnan=legacy an error and stop pretending that we support it.

Now, based on your opinions and the issue, I agree with this approach: did not support and report an error.

[RKSimon]

Is this any better?

"ignoring unsupported '-mnan=legacy' option and instead set to `-mnan=2008` option because the '%0' architecture supports it"

[yingopq]

More clear, thanks!

[yingopq]

OK, applied. Can you help review again?

[nikic]

I think this change needs an RFC on discourse.

[yingopq]

I think this change needs an RFC on discourse.

OK, I haven't modified the RFC yet, I'll study it.