[mips] Make the MipsAsmParser capable of knowing whether PIC mode is enabled or not.

[tomatabacu]

Summary

This information is needed to decide whether we do the PIC-only JAL expansions or not. It's also needed for an upcoming patch which implements the .cprestore assembler directive (which can only be used effectively in PIC mode).

By making this information available to the MipsAsmParser, we will know when to insert the instructions mandated by the .cprestore assembler directive and we will be able to give some useful warnings when we encounter a potential misuse of this directive.

[dsanders]

I'm not sure Reloc::PIC_ is the right thing to test for since '.option pic0' can be used to temporarily disable PIC in a PIC object. I think that MipsAsmParser needs to keep track of the PIC mode itself and have '.option pic0' and '.option pic2' change it appropriately.

[tomatabacu]

Addressed dsanders' concerns by using the PIC mode value set by '.option pic0/pic2', if these 2 directives are used.

[tomatabacu]

Remove excess whitespace.

[tomatabacu]

Did the diff from a clean branch instead of a development branch.

[tomatabacu]

Renamed variable to have a clearer name.

[dsanders]

This also needs a testcase. I understand there is a follow-up patch that effectively tests this. Could you link to it?

@rafael: We've noticed something odd in the MC class hierarchy for all targets. The details are below, could you take a look?

lib/Target/Mips/AsmParser/MipsAsmParser.cpp

105–106 ↗

(On Diff #15870)

Why not initialize IsOptionDirPicEnabled from 'getContext().getObjectFileInfo()->getRelocM() == Reloc::PIC_' and drop UsePicFromOptionDir?

It turns out that doing this causes crashes and while looking into it again we've noticed something strange. It seems MipsAsmParser::Parser shadows MCAsmParserExtension::Parser. As a result, calling getContext() (which returns getParser().getContext()) may crash when getParser().getContext() is fine. The difference seems to be that getContext() uses MCAsmParserExtension::getParser() and MCAsmParserExtension::Parser whereas getParser().getContext() uses MipsAsmParser::getParser() and MipsAsmParser::Parser. MipsAsmParser::Parser is initialized by the constructor but (I think) MCAsmParserExtension is not.

@rafael: All the targets seem to do the same thing. Was it intentional?

[• rafael]

It turns out that doing this causes crashes and while looking into it again we've noticed something strange. It seems MipsAsmParser::Parser shadows MCAsmParserExtension::Parser. As a result, calling getContext() (which returns getParser().getContext()) may crash when getParser().getContext() is fine. The difference seems to be that getContext() uses MCAsmParserExtension::getParser() and MCAsmParserExtension::Parser whereas getParser().getContext() uses MipsAsmParser::getParser() and MipsAsmParser::Parser. MipsAsmParser::Parser is initialized by the constructor but (I think) MCAsmParserExtension is not.

@rafael: All the targets seem to do the same thing. Was it intentional?

It was a bug. Fixed in r221667.

Cheers,
Rafael

[tomatabacu]

-we now track the status of PIC mode with a single bool member variable.
-made comments clearer.

[tomatabacu]

The inPicMode() function is used in "[mips] Expand JAL instructions when PIC is enabled." (http://reviews.llvm.org/D6231).

[dsanders]

LGTM but please wait until "[mips] Expand JAL instructions when PIC is enabled." (http://reviews.llvm.org/D6231) is reviewed before you commit.

[seanbruno]

Thanks for the work on this.

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

 //===-- MipsAsmParser.cpp - Parse Mips assembly to MCInst instructions ----===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
 #include "MCTargetDesc/MipsABIInfo.h"
 #include "MCTargetDesc/MipsMCExpr.h"
 #include "MCTargetDesc/MipsMCTargetDesc.h"
 #include "MipsRegisterInfo.h"
+#include "MipsTargetObjectFile.h"
 #include "MipsTargetStreamer.h"
 #include "llvm/ADT/APInt.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringSwitch.h"
 #include "llvm/MC/MCContext.h"
 #include "llvm/MC/MCExpr.h"
 #include "llvm/MC/MCInst.h"
 #include "llvm/MC/MCInstBuilder.h"
class MipsAsmParser : public MCTargetAsmParser {
   MCSubtargetInfo &STI;
   MipsABIInfo ABI;
   SmallVector<std::unique_ptr<MipsAssemblerOptions>, 2> AssemblerOptions;
   MCSymbol *CurrentFn; // Pointer to the function being parsed. It may be a
                        // nullptr, which indicates that no function is currently
                        // selected. This usually happens after an '.end func'
                        // directive.
   bool IsLittleEndian;
+  bool IsPicEnabled;
 
   // Print a warning along with its fix-it message at the given range.
   void printWarningWithFixIt(const Twine &Msg, const Twine &FixMsg,
                              SMRange Range, bool ShowColors = true);
 
 #define GET_ASSEMBLER_HEADER
 #include "MipsGenAsmMatcher.inc"
 
MipsAsmParser(MCSubtargetInfo &sti, MCAsmParser &parser,
 
     getTargetStreamer().updateABIInfo(*this);
 
     if (!isABI_O32() && !useOddSPReg() != 0)
       report_fatal_error("-mno-odd-spreg requires the O32 ABI");
 
     CurrentFn = nullptr;
 
+    IsPicEnabled =
+        (getContext().getObjectFileInfo()->getRelocM() == Reloc::PIC_);
+
     Triple TheTriple(sti.getTargetTriple());
     if ((TheTriple.getArch() == Triple::mips) ||
         (TheTriple.getArch() == Triple::mips64))
       IsLittleEndian = false;
     else
       IsLittleEndian = true;
   }
 
#undef GET_OPERAND_DIAGNOSTIC_TYPES
 
   bool hasDSP() const { return STI.getFeatureBits()[Mips::FeatureDSP]; }
   bool hasDSPR2() const { return STI.getFeatureBits()[Mips::FeatureDSPR2]; }
   bool hasMSA() const { return STI.getFeatureBits()[Mips::FeatureMSA]; }
   bool hasCnMips() const {
     return (STI.getFeatureBits()[Mips::FeatureCnMips]);
   }
 
+  bool inPicMode() {
+    return IsPicEnabled;
+  }
+
   bool inMips16Mode() const {
     return STI.getFeatureBits()[Mips::FeatureMips16];
   }
 
   bool useSoftFloat() const {
     return STI.getFeatureBits()[Mips::FeatureSoftFloat];
   }
 
if (Tok.isNot(AsmToken::Identifier)) {
     Error(Parser.getTok().getLoc(), "unexpected token, expected identifier");
     Parser.eatToEndOfStatement();
     return false;
   }
 
   StringRef Option = Tok.getIdentifier();
 
   if (Option == "pic0") {
+    // MipsAsmParser needs to know if the current PIC mode changes.
+    IsPicEnabled = false;
+
     getTargetStreamer().emitDirectiveOptionPic0();
     Parser.Lex();
     if (Parser.getTok().isNot(AsmToken::EndOfStatement)) {
       Error(Parser.getTok().getLoc(),
             "unexpected token, expected end of statement");
       Parser.eatToEndOfStatement();
     }
     return false;
   }
 
   if (Option == "pic2") {
+    // MipsAsmParser needs to know if the current PIC mode changes.
+    IsPicEnabled = true;
+
     getTargetStreamer().emitDirectiveOptionPic2();
     Parser.Lex();
     if (Parser.getTok().isNot(AsmToken::EndOfStatement)) {
       Error(Parser.getTok().getLoc(),
             "unexpected token, expected end of statement");
       Parser.eatToEndOfStatement();
     }
     return false;