[mips] [IAS] Make .module directives change AssemblerOptions->front().

[tomatabacu]

I was thinking of the following test:

# RUN: not llvm-mc %s -arch=mips -mcpu=mips32 -mattr=+fp64,-nooddspreg 2> %t0 | \
# RUN:   FileCheck %s

  .module nooddspreg
  add.s $f1, $f2, $f4
# CHECK: :[[@LINE-1]]:15: error: -mno-odd-spreg prohibits the use of odd FPU registers

  .set oddspreg
  add.s $f1, $f2, $f4
# CHECK-NOT: :[[@LINE-1]]:15: error: -mno-odd-spreg prohibits the use of odd FPU registers

  .set mips0
  add.s $f1, $f2, $f4
# CHECK: :[[@LINE-1]]:15: error: -mno-odd-spreg prohibits the use of odd FPU registers

Currently, the last check should fail because AssemblerOptions->front() wasn't updated when we processed the ".module nooddspreg".

However, seeing as the IAS doesn't have support for .set oddspreg/nooddspreg, the only option left is to use .module/.set fp=23/64/xx, which I don't really know how to.
I would appreciate some help with this.

[dsanders]

In D10643#192556, @tomatabacu wrote:

I was thinking of the following test:

# RUN: not llvm-mc %s -arch=mips -mcpu=mips32 -mattr=+fp64,-nooddspreg 2> %t0 | \
# RUN:   FileCheck %s

  .module nooddspreg
  add.s $f1, $f2, $f4
# CHECK: :[[@LINE-1]]:15: error: -mno-odd-spreg prohibits the use of odd FPU registers

  .set oddspreg
  add.s $f1, $f2, $f4
# CHECK-NOT: :[[@LINE-1]]:15: error: -mno-odd-spreg prohibits the use of odd FPU registers

  .set mips0
  add.s $f1, $f2, $f4
# CHECK: :[[@LINE-1]]:15: error: -mno-odd-spreg prohibits the use of odd FPU registers

Currently, the last check should fail because AssemblerOptions->front() wasn't updated when we processed the ".module nooddspreg".

However, seeing as the IAS doesn't have support for .set oddspreg/nooddspreg, the only option left is to use .module/.set fp=23/64/xx, which I don't really know how to.
I would appreciate some help with this.

So the '.set mips0' currently reverts to the state before the '.module nooddspreg'. That sounds like a bug to me.

The suggested test case looks sensible to me and it should be easy to implement '.set nooddspreg'. Could you add the test case to the patch?

[tomatabacu]

Fixed a dumb bug in my original {set,clear}ModuleFeatureBits():
*The if was preventing the setting of the front options.
Added my proposed test case.
Added dependency on D10657.

[dsanders]

LGTM with a nit

test/MC/Mips/update-module-level-options.s

10 ↗

(On Diff #28254)

It's probably best to be less strict on the column. This check will succeed if the error appeared for column 10 for example.

llvm/trunk/lib/Target/Mips/AsmParser/MipsAsmParser.cpp

#include "MipsGenAsmMatcher.inc"
   void clearFeatureBits(uint64_t Feature, StringRef FeatureString) {
     if (STI.getFeatureBits()[Feature]) {
       setAvailableFeatures(
           ComputeAvailableFeatures(STI.ToggleFeature(FeatureString)));
       AssemblerOptions.back()->setFeatures(STI.getFeatureBits());
     }
   }
 
+  void setModuleFeatureBits(uint64_t Feature, StringRef FeatureString) {
+    setFeatureBits(Feature, FeatureString);
+    AssemblerOptions.front()->setFeatures(STI.getFeatureBits());
+  }
+
+  void clearModuleFeatureBits(uint64_t Feature, StringRef FeatureString) {
+    clearFeatureBits(Feature, FeatureString);
+    AssemblerOptions.front()->setFeatures(STI.getFeatureBits());
+  }
+
 public:
   enum MipsMatchResultTy {
     Match_RequiresDifferentSrcAndDst = FIRST_TARGET_MATCH_RESULT_TY
 #define GET_OPERAND_DIAGNOSTIC_TYPES
 #include "MipsGenAsmMatcher.inc"
 #undef GET_OPERAND_DIAGNOSTIC_TYPES
 
   };
bool MipsAsmParser::parseDirectiveModule() {
 
   StringRef Option;
   if (Parser.parseIdentifier(Option)) {
     reportParseError("expected .module option identifier");
     return false;
   }
 
   if (Option == "oddspreg") {
-    clearFeatureBits(Mips::FeatureNoOddSPReg, "nooddspreg");
+    clearModuleFeatureBits(Mips::FeatureNoOddSPReg, "nooddspreg");
 
     // Synchronize the abiflags information with the FeatureBits information we
     // changed above.
     getTargetStreamer().updateABIInfo(*this);
 
     // If printing assembly, use the recently updated abiflags information.
     // If generating ELF, don't do anything (the .MIPS.abiflags section gets
     // emitted at the end).
     getTargetStreamer().emitDirectiveModuleOddSPReg();
 
     // If this is not the end of the statement, report an error.
     if (getLexer().isNot(AsmToken::EndOfStatement)) {
       reportParseError("unexpected token, expected end of statement");
       return false;
     }
 
     return false; // parseDirectiveModule has finished successfully.
   } else if (Option == "nooddspreg") {
     if (!isABI_O32()) {
       Error(L, "'.module nooddspreg' requires the O32 ABI");
       return false;
     }
 
-    setFeatureBits(Mips::FeatureNoOddSPReg, "nooddspreg");
+    setModuleFeatureBits(Mips::FeatureNoOddSPReg, "nooddspreg");
 
     // Synchronize the abiflags information with the FeatureBits information we
     // changed above.
     getTargetStreamer().updateABIInfo(*this);
 
     // If printing assembly, use the recently updated abiflags information.
     // If generating ELF, don't do anything (the .MIPS.abiflags section gets
     // emitted at the end).
bool MipsAsmParser::parseDirectiveModuleFP() {
   Parser.Lex(); // Consume the EndOfStatement.
   return false;
 }
 
 bool MipsAsmParser::parseFpABIValue(MipsABIFlagsSection::FpABIKind &FpABI,
                                     StringRef Directive) {
   MCAsmParser &Parser = getParser();
   MCAsmLexer &Lexer = getLexer();
+  bool ModuleLevelOptions = Directive == ".module";
 
   if (Lexer.is(AsmToken::Identifier)) {
     StringRef Value = Parser.getTok().getString();
     Parser.Lex();
 
     if (Value != "xx") {
       reportParseError("unsupported value, expected 'xx', '32' or '64'");
       return false;
     }
 
     if (!isABI_O32()) {
       reportParseError("'" + Directive + " fp=xx' requires the O32 ABI");
       return false;
     }
 
     FpABI = MipsABIFlagsSection::FpABIKind::XX;
+    if (ModuleLevelOptions) {
+      setModuleFeatureBits(Mips::FeatureFPXX, "fpxx");
+      clearModuleFeatureBits(Mips::FeatureFP64Bit, "fp64");
+    } else {
       setFeatureBits(Mips::FeatureFPXX, "fpxx");
       clearFeatureBits(Mips::FeatureFP64Bit, "fp64");
+    }
     return true;
   }
 
   if (Lexer.is(AsmToken::Integer)) {
     unsigned Value = Parser.getTok().getIntVal();
     Parser.Lex();
 
     if (Value != 32 && Value != 64) {
       reportParseError("unsupported value, expected 'xx', '32' or '64'");
       return false;
     }
 
     if (Value == 32) {
       if (!isABI_O32()) {
         reportParseError("'" + Directive + " fp=32' requires the O32 ABI");
         return false;
       }
 
       FpABI = MipsABIFlagsSection::FpABIKind::S32;
+      if (ModuleLevelOptions) {
+        clearModuleFeatureBits(Mips::FeatureFPXX, "fpxx");
+        clearModuleFeatureBits(Mips::FeatureFP64Bit, "fp64");
+      } else {
         clearFeatureBits(Mips::FeatureFPXX, "fpxx");
         clearFeatureBits(Mips::FeatureFP64Bit, "fp64");
+      }
     } else {
       FpABI = MipsABIFlagsSection::FpABIKind::S64;
+      if (ModuleLevelOptions) {
+        clearModuleFeatureBits(Mips::FeatureFPXX, "fpxx");
+        setModuleFeatureBits(Mips::FeatureFP64Bit, "fp64");
+      } else {
         clearFeatureBits(Mips::FeatureFPXX, "fpxx");
         setFeatureBits(Mips::FeatureFP64Bit, "fp64");
       }
+    }
 
     return true;
   }
 
   return false;
 }
 
 bool MipsAsmParser::ParseDirective(AsmToken DirectiveID) {

llvm/trunk/test/MC/Mips/update-module-level-options.s

+# RUN: not llvm-mc %s -arch=mips -mcpu=mips32 -mattr=+fp64,-nooddspreg 2>&1 | \
+# RUN:   FileCheck %s
+
+  .module nooddspreg
+  add.s $f1, $f2, $f4
+# CHECK: :[[@LINE-1]]:9: error: -mno-odd-spreg prohibits the use of odd FPU registers
+
+  .set oddspreg
+  add.s $f1, $f2, $f4
+# CHECK-NOT: :[[@LINE-1]]:{{[0-9]+}}: error: -mno-odd-spreg prohibits the use of odd FPU registers
+
+  .set mips0
+  add.s $f1, $f2, $f4
+# CHECK: :[[@LINE-1]]:9: error: -mno-odd-spreg prohibits the use of odd FPU registers