[mips] [IAS] Refactor the emitDirectiveModuleFP() functions. NFC.

[tomatabacu]

Summary

Simplify emitDirectiveModuleFP() by having it just print the current information
from MipsABIFlagsSection and doing an updateABIInfo() before such calls.

This prevents us from forgetting to update the STI.FeatureBits,
because updateABIInfo() uses those to update the MipsABIFlagsSection object,
and also makes sure we use the update mechanism from MipsABIFlagsSection.

[tomatabacu]

lib/Target/Mips/MipsTargetStreamer.h

87

This is an intentional newline, for a better visual separation of the functions.
I won't really mind if you want it removed, though.

[dsanders]

LGTM with an appropriate commit message. This is really a non-functional change (see below).

However, it looks like we weren't changing the STI.FeatureBits at all before. That doesn't seem right to me.

It's probably best to update the feature bits for the sake of consistency with other .module directives but for FPXX it doesn't really matter to the assembler at the moment. The only assembler level effect of FPXX on IAS is a bit in the ABI flags section. That said, we will want the odd-numbered register warning at some point which will probably need the FPXX flag to be correct.

The fp64 bit is more important since it may select the wrong instruction at the moment. However, despite picking the wrong instruction, it will still get the correct encoding/relocs.

Overall, I think we should update these bits even though the effect is currently non-functional.

This might need a test case, but I don't know what to put in it, as I'm not very knowledgeable about fpxx.
I would appreciate some help with this. I was thinking of something like this:

Set -mfp32 on the command line.
Use .module fp=xx.
At this point, we should be in fpxx mode, but actually we're still in fp32 mode, because we didn't change the STI.FeatureBits, we just updated the flags information.

test/MC/Mips/mips_abi_flags_xx.s covers this.

[dsanders]

That said, we will want the odd-numbered register warning at some point which will probably need the FPXX flag to be correct.

Sorry, I forgot that we already have this warning. It's driven by the nooddspreg flag.

lib/Target/Mips/AsmParser/MipsAsmParser.cpp

bool MipsAsmParser::parseDirectiveModuleFP() {
   if (!parseFpABIValue(FpABI, ".module"))
     return false;
 
   if (getLexer().isNot(AsmToken::EndOfStatement)) {
     reportParseError("unexpected token, expected end of statement");
     return false;
   }
 
-  // Emit appropriate flags.
-  getTargetStreamer().emitDirectiveModuleFP(FpABI, isABI_O32());
+  // 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().emitDirectiveModuleFP();
+
   Parser.Lex(); // Consume the EndOfStatement.
   return false;
 }
 
 bool MipsAsmParser::parseFpABIValue(MipsABIFlagsSection::FpABIKind &FpABI,
                                     StringRef Directive) {
   MCAsmParser &Parser = getParser();
   MCAsmLexer &Lexer = getLexer();
 
   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;
+    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;
-    } else
+      clearFeatureBits(Mips::FeatureFPXX, "fpxx");
+      clearFeatureBits(Mips::FeatureFP64Bit, "fp64");
+    } else {
       FpABI = MipsABIFlagsSection::FpABIKind::S64;
+      clearFeatureBits(Mips::FeatureFPXX, "fpxx");
+      setFeatureBits(Mips::FeatureFP64Bit, "fp64");
+    }
 
     return true;
   }
 
   return false;
 }
 
 bool MipsAsmParser::ParseDirective(AsmToken DirectiveID) {

lib/Target/Mips/MCTargetDesc/MipsTargetStreamer.cpp

 }
 void MipsTargetStreamer::emitDirectiveSetDsp() { forbidModuleDirective(); }
 void MipsTargetStreamer::emitDirectiveSetNoDsp() { forbidModuleDirective(); }
 void MipsTargetStreamer::emitDirectiveCpLoad(unsigned RegNo) {}
 void MipsTargetStreamer::emitDirectiveCpsetup(unsigned RegNo, int RegOrOffset,
                                               const MCSymbol &Sym, bool IsReg) {
 }
 
-void
-MipsTargetStreamer::emitDirectiveModuleFP(MipsABIFlagsSection::FpABIKind Value,
-                                          bool Is32BitABI) {
-  ABIFlagsSection.setFpABI(Value, Is32BitABI);
-}
-
-void MipsTargetStreamer::emitDirectiveModuleFP() {
-  emitDirectiveModuleFP(ABIFlagsSection.getFpABI(), ABIFlagsSection.Is32BitABI);
-}
+void MipsTargetStreamer::emitDirectiveModuleFP() {}
 
 void MipsTargetStreamer::emitDirectiveModuleOddSPReg() {
   if (!ABIFlagsSection.OddSPReg && !ABIFlagsSection.Is32BitABI)
     report_fatal_error("+nooddspreg is only valid for O32");
 }
 void MipsTargetStreamer::emitDirectiveSetFp(
     MipsABIFlagsSection::FpABIKind Value) {
   forbidModuleDirective();
else
     OS << RegOrOffset;
 
   OS << ", ";
 
   OS << Sym.getName() << "\n";
   forbidModuleDirective();
 }
 
-void MipsTargetAsmStreamer::emitDirectiveModuleFP(
-    MipsABIFlagsSection::FpABIKind Value, bool Is32BitABI) {
-  MipsTargetStreamer::emitDirectiveModuleFP(Value, Is32BitABI);
-
+void MipsTargetAsmStreamer::emitDirectiveModuleFP() {
   OS << "\t.module\tfp=";
-  OS << ABIFlagsSection.getFpABIString(Value) << "\n";
+  OS << ABIFlagsSection.getFpABIString(ABIFlagsSection.getFpABI()) << "\n";
 }
 
 void MipsTargetAsmStreamer::emitDirectiveSetFp(
     MipsABIFlagsSection::FpABIKind Value) {
   MipsTargetStreamer::emitDirectiveSetFp(Value);
 
   OS << "\t.set\tfp=";
   OS << ABIFlagsSection.getFpABIString(Value) << "\n";

lib/Target/Mips/MipsTargetStreamer.h

public:
   virtual void emitDirectiveSetSoftFloat();
   virtual void emitDirectiveSetHardFloat();
 
   // PIC support
   virtual void emitDirectiveCpLoad(unsigned RegNo);
   virtual void emitDirectiveCpsetup(unsigned RegNo, int RegOrOffset,
                                     const MCSymbol &Sym, bool IsReg);
 
-  /// Emit a '.module fp=value' directive using the given values.
-  /// Updates the .MIPS.abiflags section
-  virtual void emitDirectiveModuleFP(MipsABIFlagsSection::FpABIKind Value,
-                                     bool Is32BitABI);
-  /// Emit a '.module fp=value' directive using the current values of the
-  /// .MIPS.abiflags section.
-  void emitDirectiveModuleFP();
-
+  // FP abiflags directives
+  virtual void emitDirectiveModuleFP();
   virtual void emitDirectiveModuleOddSPReg();
   virtual void emitDirectiveSetFp(MipsABIFlagsSection::FpABIKind Value);
+
   void forbidModuleDirective() { ModuleDirectiveAllowed = false; }
   void reallowModuleDirective() { ModuleDirectiveAllowed = true; }
   bool isModuleDirectiveAllowed() { return ModuleDirectiveAllowed; }
 
   // This method enables template classes to set internal abi flags
   // structure values.
   template <class PredicateLibrary>
   void updateABIInfo(const PredicateLibrary &P) {
public:
   void emitDirectiveSetSoftFloat() override;
   void emitDirectiveSetHardFloat() override;
 
   // PIC support
   void emitDirectiveCpLoad(unsigned RegNo) override;
   void emitDirectiveCpsetup(unsigned RegNo, int RegOrOffset,
                             const MCSymbol &Sym, bool IsReg) override;
 
-  // ABI Flags
-  void emitDirectiveModuleFP(MipsABIFlagsSection::FpABIKind Value,
-                             bool Is32BitABI) override;
+  // FP abiflags directives
+  void emitDirectiveModuleFP() override;
   void emitDirectiveModuleOddSPReg() override;
   void emitDirectiveSetFp(MipsABIFlagsSection::FpABIKind Value) override;
 };
 
 // This part is for ELF object output
 class MipsTargetELFStreamer : public MipsTargetStreamer {
   bool MicroMipsEnabled;
   const MCSubtargetInfo &STI;