[PATCH] [Mips] Generate warning for invalid combination of '-mnan' and '-march' options.

[Daniel Sanders]

================
Comment at: lib/Basic/Targets.cpp:5769
@@ +5768,3 @@
+        .Case("mips32r3", NanLegacy | Nan2008)
+        .Case("mips32r5", NanLegacy | Nan2008)
+        .Case("mips32r6", Nan2008)
----------------
petarj wrote:
> It might be me only, but seeing a function that returns enum but can actually return non-enumerated value seems wrong (I am not saying it is illegal though).
> I see this is already done for ArchDefineTypes within PPCTargetInfo, but it still looks wrong (to me).
> Would not this trigger a runtime error in clang if it was compiled with "-fsanitize=enum"?
I think this is the main remaining comment. I agree in principle that enum types should have the value of one of the enum values. Like Petar, I don't think it's technically wrong though. Have you tried the -fsanitize=enum option that was suggested?

Elsewhere, an integer type is used instead but given that there's only two values and I would be very surprised if we ever add more, we should probably just define NanBoth = 3 if it's a problem.

================
Comment at: lib/Driver/Tools.cpp:1119-1121
@@ +1118,5 @@
+    if (Val == "2008") {
+      if (mips::getSupportedNanEncoding(CPUName) & mips::Nan2008) {
+        Features.push_back("+nan2008");
+      } else {
+        Features.push_back("-nan2008");
----------------
Style nit: Unecessary braces around single-line if-statement.

================
Comment at: lib/Driver/Tools.cpp:1126-1128
@@ +1125,5 @@
+    } else if (Val == "legacy") {
+      if (mips::getSupportedNanEncoding(CPUName) & mips::NanLegacy) {
+        Features.push_back("-nan2008");
+      } else {
+        Features.push_back("+nan2008");
----------------
Style nit: Unecessary braces around single-line if-statement.

================
Comment at: test/Preprocessor/init.c:4422
@@ -4421,3 @@
-// RUN:   | FileCheck -check-prefix MIPS-NAN2008 %s
-// MIPS-NAN2008:#define __mips_nan2008 1
-//
----------------
vradosavljevic wrote:
> atanasyan wrote:
> > Are there any other tests that check `__mips_nan2008` macro generation?
> In the same test there is a case that check __mips_nan2008 macro.
For reference: It's on line 4457 with the patch applied.

http://reviews.llvm.org/D8170

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