Add support for generating MIPS legacy NaN

[vradosavljevic]

Summary

Currently, the NaN values emitted for MIPS architectures, do not cover non-2008 compliant case. This change fixes the issue.

[dsanders]

lib/AST/ExprConstant.cpp

7599–7603 ↗

(On Diff #20678)

Could you add a comment explaining the relevant history and why that leads to QNaN and SNaN being reversed here. It won't be obvious to most readers.

lib/Basic/Targets.cpp

5679 ↗

(On Diff #20678)

This isn't quite right. It should just be IsNan2008 since IsNan2008 is initialized with the result of isNaN2008Default().

test/CodeGen/builtin-nan-legacy.c

2–3 ↗

(On Diff #20678)

New tests should be using FileCheck rather than grep.

[vradosavljevic]

lib/Basic/Targets.cpp

5679 ↗

(On Diff #20678)

When you generating code for mips32r6/mips64r6 with option -mnan=legacy, IsNan2008 will be set to false.
In function handleTargetFeatures there are lines that change that.

else if (*it == "-nan2008")
  IsNan2008 = false;

Because of that, I use the result of isNaN2008Default().

[petarj]

lib/Basic/Targets.cpp

5679 ↗

(On Diff #20678)

We should not allow that case. Clang should (at least) issue a warning if "-mnan=legacy" is passed for the revisions that do not support the NaN legacy encoding.

[dsanders]

lib/Basic/Targets.cpp

5679 ↗

(On Diff #20678)

Agreed. I'd prefer an error* since the user is requesting an impossible combination of options but it should at least warn instead of quietly ignoring the user.

*Ideally with a suggestion for fixing it, e.g. "error: IEEE754(2008) compliant NaN's are mandatory on Mips32r6. Did you mean -mnan=2008?"

[petarj]

lib/Basic/Targets.cpp

5679 ↗

(On Diff #20678)

I would also prefer an error, but gcc seems to produce a warning only.
Furthermore, we should do similar for "-mnan=2008" and earlier mips revisions. But this all can be part of another patch, not this one necessary.

[vradosavljevic]

lib/AST/ExprConstant.cpp

7599–7603 ↗

(On Diff #20678)

Done.

lib/Basic/Targets.cpp

5679 ↗

(On Diff #20678)

Done.

5679 ↗

(On Diff #20678)

Ok. This will be part of the next patch.

test/CodeGen/builtin-nan-legacy.c

2–3 ↗

(On Diff #20678)

Done.

[vradosavljevic]

New patch uploaded.

[dsanders]

lib/Basic/Targets.cpp

5679 ↗

(On Diff #20678)

Furthermore, we should do similar for "-mnan=2008" and earlier mips revisions. But this all can be part of another patch, not this one necessary.

Off hand, I'm not sure exactly which ISA introduced the ABS2008 and NAN2008 config bits but at least some of the ISA's prior to 32r6/64r6 supported 2008 in addition to legacy. I don't think it's seen widespread use though.

[dsanders]

LGTM with a couple tiny nits.

include/clang/Basic/TargetInfo.h

642 ↗

(On Diff #20765)

Small nit: The grammar is a little odd, possibly a missing word? Maybe use 'Only MIPS allows a different encoding'

lib/AST/ExprConstant.cpp

7599–7603 ↗

(On Diff #20765)

Just a couple tiny grammar nits:

"...to choose the first bit of their significand set for qNaN and sNaN." -> "...to choose whether the first bit of their significand was set for qNaN or sNaN."

"MIPS chose different encoding..." -> "MIPS chose a different encoding...".

[petarj]

lib/Basic/Targets.cpp

5679 ↗

(On Diff #20678)

FIR.Has2008 is optional as of R3, so MIPS32R2 and earlier cores are in conflict with -mnan=2008.

[vradosavljevic]

include/clang/Basic/TargetInfo.h

642 ↗

(On Diff #20765)

Done.

lib/AST/ExprConstant.cpp

7599–7603 ↗

(On Diff #20765)

Done.

[vradosavljevic]

Comments addressed.

[dsanders]

LGTM, Thanks. Do you have commit access or do you need someone to commit it for you?

cfe/trunk/include/clang/Basic/TargetInfo.h

public:
   // constraint is longer than one character.
   virtual std::string convertConstraint(const char *&Constraint) const {
     // 'p' defaults to 'r', but can be overridden by targets.
     if (*Constraint == 'p')
       return std::string("r");
     return std::string(1, *Constraint);
   }
 
+  /// \brief Returns true if NaN encoding is IEEE 754-2008.
+  /// Only MIPS allows a different encoding.
+  virtual bool isNan2008() const {
+    return true;
+  }
+
   /// \brief Returns a string of target-specific clobbers, in LLVM format.
   virtual const char *getClobbers() const = 0;
 
 
   /// \brief Returns the target triple of the primary target.
   const llvm::Triple &getTriple() const {
     return Triple;
   }

cfe/trunk/lib/AST/ExprConstant.cpp

static bool TryEvaluateBuiltinNaN(const ASTContext &Context,
   llvm::APInt fill;
 
   // Treat empty strings as if they were zero.
   if (S->getString().empty())
     fill = llvm::APInt(32, 0);
   else if (S->getString().getAsInteger(0, fill))
     return false;
 
+  if (Context.getTargetInfo().isNan2008()) {
     if (SNaN)
       Result = llvm::APFloat::getSNaN(Sem, false, &fill);
     else
       Result = llvm::APFloat::getQNaN(Sem, false, &fill);
+  } else {
+    // Prior to IEEE 754-2008, architectures were allowed to choose whether
+    // the first bit of their significand was set for qNaN or sNaN. MIPS chose
+    // a different encoding to what became a standard in 2008, and for pre-
+    // 2008 revisions, MIPS interpreted sNaN-2008 as qNan and qNaN-2008 as
+    // sNaN. This is now known as "legacy NaN" encoding.
+    if (SNaN)
+      Result = llvm::APFloat::getQNaN(Sem, false, &fill);
+    else
+      Result = llvm::APFloat::getSNaN(Sem, false, &fill);
+  }
+
   return true;
 }
 
 bool FloatExprEvaluator::VisitCallExpr(const CallExpr *E) {
   switch (E->getBuiltinCallee()) {
   default:
     return ExprEvaluatorBaseTy::VisitCallExpr(E);
 

cfe/trunk/lib/Basic/Targets.cpp

public:
   bool isNaN2008Default() const {
     return CPU == "mips32r6" || CPU == "mips64r6";
   }
 
   bool isFP64Default() const {
     return CPU == "mips32r6" || ABI == "n32" || ABI == "n64" || ABI == "64";
   }
 
+  bool isNan2008() const override {
+    return IsNan2008;
+  }
+
   StringRef getABI() const override { return ABI; }
   bool setCPU(const std::string &Name) override {
     bool IsMips32 = getTriple().getArch() == llvm::Triple::mips ||
                     getTriple().getArch() == llvm::Triple::mipsel;
     CPU = Name;
     return llvm::StringSwitch<bool>(Name)
         .Case("mips1", IsMips32)
         .Case("mips2", IsMips32)

cfe/trunk/test/CodeGen/builtin-nan-legacy.c

+// RUN: %clang -target mipsel-unknown-linux -mnan=legacy -emit-llvm -S %s -o - | FileCheck %s
+// CHECK: float 0x7FF4000000000000, float 0x7FF8000000000000
+// CHECK: double 0x7FF4000000000000, double 0x7FF8000000000000
+
+float f[] = {
+  __builtin_nan(""),
+  __builtin_nans(""),
+};
+
+double d[] = {
+  __builtin_nan(""),
+  __builtin_nans(""),
+};