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

[Vladimir Radosavljevic]

================
Comment at: lib/Basic/Targets.cpp:5729
@@ +5728,3 @@
+        .Case("mips5", true)
+        .Case("mips32", true)
+        .Case("mips64", true)
----------------
atanasyan wrote:
> dsanders wrote:
> > petarj wrote:
> > > According to the documentation, Has2008 field in FIR Register is optional as of R3. So, if I understand this correctly, mips32r2 always uses legacy encoding. Is that correct? If so, should we add mips32r2 case as well?
> > That's my understanding. Although it's worth pointing out that we also accept microMIPS in mips32r2 at the moment even though it was added in mips32r3 as far as I know.
> > 
> > For -mnan, I think we should add mips32r2 to the legacy-only list and explain the reason to anyone who asks about it.
> I think it is better to explicitly express 2008/legacy support status for each CPU. Let's consider the following code:
> 
> ```
> enum NanMode { NanLegacy = 1, Nan2008 = 2 };
> 
> NanMode getSupportedNanMode() {
>     return llvm::StringSwitch<int>(CPU)
>         .Case("mips1", NanLegacy)
>         ....
>         .Case("mips32r2", NanLegacy | Nan2008)
>         ...
>         .Case("mips32r6", Nan2008)
> }
> 
> ...
> 
> IsNan2008 = getSupportedNanMode() == Nan2008;
> 
> ...
> 
> else if (*it == "+nan2008") {
>   if (getSupportedNanMode() & Nan2008)
>     IsNan2008 = true;
>   else
>     Diags.Report(diag::warn_target_unsupported_nan2008) << CPU;
> }
> ```
Mips32r2 and Mips64r2 are added to the legacy-only list.

================
Comment at: lib/Basic/Targets.cpp:5729
@@ +5728,3 @@
+        .Case("mips5", true)
+        .Case("mips32", true)
+        .Case("mips64", true)
----------------
vradosavljevic wrote:
> atanasyan wrote:
> > dsanders wrote:
> > > petarj wrote:
> > > > According to the documentation, Has2008 field in FIR Register is optional as of R3. So, if I understand this correctly, mips32r2 always uses legacy encoding. Is that correct? If so, should we add mips32r2 case as well?
> > > That's my understanding. Although it's worth pointing out that we also accept microMIPS in mips32r2 at the moment even though it was added in mips32r3 as far as I know.
> > > 
> > > For -mnan, I think we should add mips32r2 to the legacy-only list and explain the reason to anyone who asks about it.
> > I think it is better to explicitly express 2008/legacy support status for each CPU. Let's consider the following code:
> > 
> > ```
> > enum NanMode { NanLegacy = 1, Nan2008 = 2 };
> > 
> > NanMode getSupportedNanMode() {
> >     return llvm::StringSwitch<int>(CPU)
> >         .Case("mips1", NanLegacy)
> >         ....
> >         .Case("mips32r2", NanLegacy | Nan2008)
> >         ...
> >         .Case("mips32r6", Nan2008)
> > }
> > 
> > ...
> > 
> > IsNan2008 = getSupportedNanMode() == Nan2008;
> > 
> > ...
> > 
> > else if (*it == "+nan2008") {
> >   if (getSupportedNanMode() & Nan2008)
> >     IsNan2008 = true;
> >   else
> >     Diags.Report(diag::warn_target_unsupported_nan2008) << CPU;
> > }
> > ```
> Mips32r2 and Mips64r2 are added to the legacy-only list.
Done. Thanks for suggestion.

================
Comment at: lib/Basic/Targets.cpp:5962-5974
@@ -5949,5 +5961,15 @@
         HasFP64 = false;
-      else if (*it == "+nan2008")
-        IsNan2008 = true;
-      else if (*it == "-nan2008")
-        IsNan2008 = false;
+      else if (*it == "+nan2008") {
+        if (hasOnlyNaNLegacyEncoding()) {
+          unsigned DiagID = Diags.getCustomDiagID(DiagnosticsEngine::Warning,
+                    "the '%0' architecture does not support '-mnan=2008'");
+          Diags.Report(DiagID) << CPU;
+        } else
+          IsNan2008 = true;
+      } else if (*it == "-nan2008") {
+        if (hasOnlyNaN2008Encoding()) {
+          unsigned DiagID = Diags.getCustomDiagID(DiagnosticsEngine::Warning,
+                    "the '%0' architecture does not support '-mnan=legacy'");
+          Diags.Report(DiagID) << CPU;
+        } else
+          IsNan2008 = false;
----------------
dsanders wrote:
> The diagnostics should mention that we're ignoring the request.
Done.

================
Comment at: lib/Basic/Targets.cpp:5964
@@ +5963,3 @@
+        if (hasOnlyNaNLegacyEncoding()) {
+          unsigned DiagID = Diags.getCustomDiagID(DiagnosticsEngine::Warning,
+                    "the '%0' architecture does not support '-mnan=2008'");
----------------
atanasyan wrote:
> Why do we write a warning message right here instead to put it to the DiagnosticXXX.td file?
Warning messages are putted in the DiagnosticCommonKinds.td file.

================
Comment at: test/CodeGen/mips-unsupported-nan.c:8
@@ +7,3 @@
+
+// CHECK-WARNING: warning: the '{{mips.*}}' architecture does not support '-mnan={{2008|legacy}}'
+// CHECK-LEGACY: float 0x7FF4000000000000
----------------
dsanders wrote:
> We should check that the right warning is emitted for the right cases.
Done.

http://reviews.llvm.org/D8170

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