[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

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.

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/MipsMCExpr.h"
 #include "MCTargetDesc/MipsMCTargetDesc.h"
 #include "MipsRegisterInfo.h"
+#include "MipsTargetObjectFile.h"
 #include "MipsTargetStreamer.h"
 #include "llvm/ADT/APInt.h"
 #include "llvm/ADT/StringSwitch.h"
 #include "llvm/ADT/SmallVector.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;
   MCAsmParser &Parser;
   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.
 
   // 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"
 
   unsigned checkTargetMatchPredicate(MCInst &Inst) override;
 
#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 inMips16Mode() const {
     return STI.getFeatureBits() & Mips::FeatureMips16;
   }
+
+  bool inPicMode() {
+    return getContext().getObjectFileInfo()->getRelocM() == Reloc::PIC_;
+  }
+
   // TODO: see how can we get this info.
   bool abiUsesSoftFloat() const { return false; }
 
   /// Warn if RegNo is the current assembler temporary.
   void warnIfAssemblerTemporary(int RegNo, SMLoc Loc);
 };
 }