[mips] Implement .ent, .end, .frame, .mask and .fmask.

[matheusalmeida]

Add missing early return.

[matheusalmeida]

Formatting.

[tomatabacu]

Commandeered to finish off the patch and make it ready for commit.

[tomatabacu]

Changed all uses of MCSymbol to StringRef for the ".end" directive.

Added initialization for CurrentFn in the constructor for MipsAsmParser.

Changed "00000702" to "00000802" in the ".rel.pdr" SectionData in the mips-pdr.s
test to reflect the addition of the ".MIPS.abiflags" section.

Rebased to trunk.

[dsanders]

This patch looks like it's almost ready. There are just a few error messages that could be better, some error cases that will silently succeed, and a few style issues.

lib/Target/Mips/AsmParser/MipsAsmParser.cpp

71

It's preferable to use nullptr instead of NULL. There are quite a few cases of this in the patch.

3056–3059

Instead of eating any remaining tokens we should be testing that we have the end of the statement and reporting an error if we aren't.

3086

Rather than just saying 'unexpected token' could you update the message to say what tokens it was expecting. Please also do this for the other cases added by this patch.

lib/Target/Mips/MCTargetDesc/MipsTargetStreamer.cpp

490–496

OS.EmitIntValue(GPRInfoSet ? GPRBitMask : 0, 4) would be a bit cleaner.

570

Identifiers begining with underscore followed by a capital letter are reserved by the C++ standard. Could you put the underscore at the end?

[tomatabacu]

Addressed comments.
Improved parsing of the .ent extension.
Improved the mips-pdr-bad test.
Ran clang-format and rebased.

[tomatabacu]

Removed unrelated changes.

[dsanders]

Thanks. Just a couple nits and then it will LGTM.

lib/Target/Mips/AsmParser/MipsAsmParser.cpp

71

You missed this instance of NULL.

2955–2956

This explains what we are doing but doesn't explain why. I'd write something like:

// Although we accept the undocumented extended version for compatibility reasons,
// the additional argument has no functional effect so we'd like to discourage its use.
// We therefore pretend that a comma is not accepted here when reporting errors.

2964

I'd also mention it here with something like:

// We want to discourage the use of this syntax but we shouldn't confuse the user when they've already tried to use it
// We therefore admit to accepting it this time and tell the user what we expect.

[tomatabacu]

Addressed comments.

[dsanders]

lib/Target/Mips/AsmParser/MipsAsmParser.cpp

2958

Nit: In this part of the code we haven't seen a comma. Instead we saw that the current token isn't a comma or an end-of-statement.

[tomatabacu]

Addressed comments.

lib/Target/Mips/AsmParser/MipsAsmParser.cpp

class MipsAsmParser : public MCTargetAsmParser {
   MipsTargetStreamer &getTargetStreamer() {
     MCTargetStreamer &TS = *Parser.getStreamer().getTargetStreamer();
     return static_cast<MipsTargetStreamer &>(TS);
   }
 
   MCSubtargetInfo &STI;
   MCAsmParser &Parser;
   MipsAssemblerOptions Options;
+  MCSymbol *CurrentFn; // Pointer to the function being parsed. It may be NULL
+                       // which indicates no function is currently selected.
+                       // This usually happens after an '.end function'
+                       // directive.
 
 #define GET_ASSEMBLER_HEADER
 #include "MipsGenAsmMatcher.inc"
 
   unsigned checkTargetMatchPredicate(MCInst &Inst) override;
 
   bool MatchAndEmitInstruction(SMLoc IDLoc, unsigned &Opcode,
                                OperandVector &Operands, MCStreamer &Out,
MipsAsmParser(MCSubtargetInfo &sti, MCAsmParser &parser,
     // Assert exactly one ABI was chosen.
     assert((((STI.getFeatureBits() & Mips::FeatureO32) != 0) +
             ((STI.getFeatureBits() & Mips::FeatureEABI) != 0) +
             ((STI.getFeatureBits() & Mips::FeatureN32) != 0) +
             ((STI.getFeatureBits() & Mips::FeatureN64) != 0)) == 1);
 
     if (!isABI_O32() && !useOddSPReg() != 0)
       report_fatal_error("-mno-odd-spreg requires the O32 ABI");
+
+    CurrentFn = NULL;
   }
 
   MCAsmParser &getParser() const { return Parser; }
   MCAsmLexer &getLexer() const { return Parser.getLexer(); }
 
   /// True if all of $fcc0 - $fcc7 exist for the current ISA.
   bool hasEightFccRegisters() const { return hasMips4() || hasMips32(); }
 
bool MipsAsmParser::ParseDirective(AsmToken DirectiveID) {
   StringRef IDVal = DirectiveID.getString();
 
   if (IDVal == ".cpload")
     return parseDirectiveCPLoad(DirectiveID.getLoc());
   if (IDVal == ".dword") {
     parseDataDirective(8, DirectiveID.getLoc());
     return false;
   }
-
   if (IDVal == ".ent") {
-    // Ignore this directive for now.
-    Parser.Lex();
+    StringRef SymbolName;
+
+    if (Parser.parseIdentifier(SymbolName)) {
+      reportParseError("expected identifier after .ent");
+      return false;
+    }
+
+    // There's an undocumented extension that allows an integer to
+    // follow the name of the procedure which AFAICS is ignored by GAS.
+    // Example: .ent foo,2
+    if (getLexer().isNot(AsmToken::EndOfStatement)) {
+      if (getLexer().isNot(AsmToken::Comma)) {
+        reportParseError("unexpected token in directive .ent");
+        return false;
+      }
+      Parser.Lex(); // Eat the comma.
+      const MCExpr *DummyNumber;
+      if (Parser.parseExpression(DummyNumber))
+        return false;
+      Parser.eatToEndOfStatement();
+    }
+
+    MCSymbol *Sym = getContext().GetOrCreateSymbol(SymbolName);
+
+    getTargetStreamer().emitDirectiveEnt(*Sym);
+    CurrentFn = Sym;
     return false;
   }
 
   if (IDVal == ".end") {
-    // Ignore this directive for now.
-    Parser.Lex();
+    StringRef SymbolName;
+
+    if (Parser.parseIdentifier(SymbolName)) {
+      reportParseError("expected identifier after .end");
+      return false;
+    }
+
+    if (getLexer().isNot(AsmToken::EndOfStatement)) {
+      reportParseError("unexpected token in directive .end");
+      return false;
+    }
+
+    if (CurrentFn == NULL) {
+      reportParseError(".end used without .ent");
+      return false;
+    }
+
+    if ((SymbolName != CurrentFn->getName())) {
+      reportParseError(".end symbol does not match .ent symbol");
+      return false;
+    }
+
+    getTargetStreamer().emitDirectiveEnd(SymbolName);
+    CurrentFn = NULL;
     return false;
   }
 
   if (IDVal == ".frame") {
-    // Ignore this directive for now.
+    // .frame $stack_reg, frame_size_in_bytes, $return_reg
+    SmallVector<std::unique_ptr<MCParsedAsmOperand>, 1> TmpReg;
+    OperandMatchResultTy ResTy = ParseAnyRegister(TmpReg);
+    if (ResTy == MatchOperand_NoMatch || ResTy == MatchOperand_ParseFail) {
+      reportParseError("expected stack register");
+      return false;
+    }
+
+    MipsOperand &StackRegOpnd = static_cast<MipsOperand &>(*TmpReg[0]);
+    if (!StackRegOpnd.isGPRAsmReg()) {
+      reportParseError(StackRegOpnd.getStartLoc(), "invalid register");
+      return false;
+    }
+    unsigned StackReg = StackRegOpnd.getGPR32Reg();
+
+    if (Parser.getTok().is(AsmToken::Comma))
+      Parser.Lex();
+    else {
+      reportParseError("unexpected token in .frame directive");
+      return false;
+    }
+
+    // Parse the frame size.
+    const MCExpr *FrameSize;
+    int64_t FrameSizeVal;
+
+    if (Parser.parseExpression(FrameSize))
+      return false;
+
+    if (!FrameSize->EvaluateAsAbsolute(FrameSizeVal)) {
+      reportParseError("frame size not an absolute expression");
+      return false;
+    }
+
+    if (Parser.getTok().is(AsmToken::Comma))
+      Parser.Lex();
+    else {
+      reportParseError("unexpected token in .frame directive");
+      return false;
+    }
+
+    // Parse the return register.
+    TmpReg.clear();
+    ResTy = ParseAnyRegister(TmpReg);
+    if (ResTy == MatchOperand_NoMatch || ResTy == MatchOperand_ParseFail) {
+      reportParseError("expected return register");
+      return false;
+    }
+
+    MipsOperand &ReturnRegOpnd = static_cast<MipsOperand &>(*TmpReg[0]);
+    if (!ReturnRegOpnd.isGPRAsmReg()) {
+      reportParseError(ReturnRegOpnd.getStartLoc(), "invalid register");
+      return false;
+    }
+
     Parser.eatToEndOfStatement();
+    getTargetStreamer().emitFrame(StackReg, FrameSizeVal,
+                                  ReturnRegOpnd.getGPR32Reg());
     return false;
   }
 
   if (IDVal == ".set") {
     return parseDirectiveSet();
   }
 
-  if (IDVal == ".fmask") {
-    // Ignore this directive for now.
-    Parser.eatToEndOfStatement();
+  if (IDVal == ".mask" || IDVal == ".fmask") {
+    // .mask bitmask, frame_offset
+    // bitmask: One bit for each register used.
+    // frame_offset: Offset from Canonical Frame Address ($sp on entry) where
+    //               first register is expected to be saved.
+    // Examples:
+    //   .mask 0x80000000, -4
+    //   .fmask 0x80000000, -4
+    //
+
+    // Parse the bitmask
+    int64_t BitMaskVal;
+    if (Parser.parseAbsoluteExpression(BitMaskVal)) {
+      reportParseError("bitmask not an absolute expression");
       return false;
     }
 
-  if (IDVal == ".mask") {
-    // Ignore this directive for now.
-    Parser.eatToEndOfStatement();
+    if (Parser.getTok().is(AsmToken::Comma))
+      Parser.Lex();
+    else {
+      reportParseError("unexpected token in directive");
+      return false;
+    }
+
+    // Parse the frame_offset
+    int64_t FrameOffsetVal;
+    if (Parser.parseAbsoluteExpression(FrameOffsetVal)) {
+      reportParseError("frame offset not an absolute expression");
+      return false;
+    }
+
+    if (IDVal == ".mask")
+      getTargetStreamer().emitMask(BitMaskVal, FrameOffsetVal);
+    else
+      getTargetStreamer().emitFMask(BitMaskVal, FrameOffsetVal);
     return false;
   }
 
   if (IDVal == ".nan")
     return parseDirectiveNaN();
 
   if (IDVal == ".gpword") {
     parseDirectiveGpWord();

lib/Target/Mips/MCTargetDesc/MipsTargetStreamer.cpp

 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/ELF.h"
 #include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/FormattedStream.h"
 
 using namespace llvm;
 
 MipsTargetStreamer::MipsTargetStreamer(MCStreamer &S)
-    : MCTargetStreamer(S), canHaveModuleDirective(true) {}
+    : MCTargetStreamer(S), canHaveModuleDirective(true) {
+        GPRInfoSet = FPRInfoSet = FrameInfoSet = false;
+    }
 void MipsTargetStreamer::emitDirectiveSetMicroMips() {}
 void MipsTargetStreamer::emitDirectiveSetNoMicroMips() {}
 void MipsTargetStreamer::emitDirectiveSetMips16() {}
 void MipsTargetStreamer::emitDirectiveSetNoMips16() {}
 void MipsTargetStreamer::emitDirectiveSetReorder() {}
 void MipsTargetStreamer::emitDirectiveSetNoReorder() {}
 void MipsTargetStreamer::emitDirectiveSetMacro() {}
 void MipsTargetStreamer::emitDirectiveSetNoMacro() {}

 }
 
 void MipsTargetELFStreamer::emitDirectiveSetNoAt() {
   // FIXME: implement.
   setCanHaveModuleDir(false);
 }
 
 void MipsTargetELFStreamer::emitDirectiveEnd(StringRef Name) {
-  // FIXME: implement.
+  MCAssembler &MCA = getStreamer().getAssembler();
+  MCContext &Context = MCA.getContext();
+  MCStreamer &OS = getStreamer();
+
+  const MCSectionELF *Sec = Context.getELFSection(".pdr", ELF::SHT_PROGBITS,
+                                                  ELF::SHF_ALLOC | ELF::SHT_REL,
+                                                  SectionKind::getMetadata());
+
+  const MCSymbolRefExpr *ExprRef = 
+    MCSymbolRefExpr::Create(Name, MCSymbolRefExpr::VK_None, Context);
+
+  MCSectionData &SecData = MCA.getOrCreateSectionData(*Sec);
+  SecData.setAlignment(4);
+
+  OS.PushSection();
+
+  OS.SwitchSection(Sec);
+
+  OS.EmitValueImpl(ExprRef, 4);
+
+  if (GPRInfoSet) {
+    OS.EmitIntValue(GPRBitMask, 4);
+    OS.EmitIntValue(GPROffset, 4);
+  } else {
+    OS.EmitIntValue(0, 4); // reg_mask
+    OS.EmitIntValue(0, 4); // reg_offset
+  }
+
+  if (FPRInfoSet) {
+    OS.EmitIntValue(FPRBitMask, 4);
+    OS.EmitIntValue(FPROffset, 4);
+  } else {
+    OS.EmitIntValue(0, 4); // fpreg_mask
+    OS.EmitIntValue(0, 4); // fpreg_offset
+  }
+
+  if (FrameInfoSet) {
+    OS.EmitIntValue(FrameOffset, 4);
+    OS.EmitIntValue(FrameReg, 4);
+    OS.EmitIntValue(ReturnReg, 4);
+  } else {
+    OS.EmitIntValue(0, 4); // frame_offset
+    OS.EmitIntValue(0, 4); // frame_reg
+    OS.EmitIntValue(0, 4); // return_reg
+  }
+  
+  // The .end directive marks the end of a procedure. Invalidate
+  // the information gathered up until this point.
+  GPRInfoSet = FPRInfoSet = FrameInfoSet = false;
+
+  OS.PopSection();
 }
 
 void MipsTargetELFStreamer::emitDirectiveEnt(const MCSymbol &Symbol) {
-  // FIXME: implement.
+  GPRInfoSet = FPRInfoSet = FrameInfoSet = false;
 }
 
 void MipsTargetELFStreamer::emitDirectiveAbiCalls() {
   MCAssembler &MCA = getStreamer().getAssembler();
   unsigned Flags = MCA.getELFHeaderEFlags();
   Flags |= ELF::EF_MIPS_CPIC | ELF::EF_MIPS_PIC;
   MCA.setELFHeaderEFlags(Flags);
 }
void MipsTargetELFStreamer::emitDirectiveOptionPic2() {
   // 'pic2' also sets the CPIC bit in the ELF header. This is different from
   // what is stated in the SYSV ABI which consider the bits EF_MIPS_PIC and
   // EF_MIPS_CPIC to be mutually exclusive.
   Flags |= ELF::EF_MIPS_PIC | ELF::EF_MIPS_CPIC;
   MCA.setELFHeaderEFlags(Flags);
 }
 
 void MipsTargetELFStreamer::emitFrame(unsigned StackReg, unsigned StackSize,
-                                      unsigned ReturnReg) {
-  // FIXME: implement.
+                                      unsigned _ReturnReg) {
+  MCContext &Context = getStreamer().getAssembler().getContext();
+  const MCRegisterInfo *RegInfo = Context.getRegisterInfo();
+
+  FrameInfoSet = true;
+  FrameReg = RegInfo->getEncodingValue(StackReg);
+  FrameOffset = StackSize;
+  ReturnReg = RegInfo->getEncodingValue(_ReturnReg);
 }
 
 void MipsTargetELFStreamer::emitMask(unsigned CPUBitmask,
                                      int CPUTopSavedRegOff) {
-  // FIXME: implement.
+  GPRInfoSet = true;
+  GPRBitMask = CPUBitmask;
+  GPROffset = CPUTopSavedRegOff;
 }
 
 void MipsTargetELFStreamer::emitFMask(unsigned FPUBitmask,
                                       int FPUTopSavedRegOff) {
-  // FIXME: implement.
+  FPRInfoSet = true;
+  FPRBitMask = FPUBitmask;
+  FPROffset = FPUTopSavedRegOff;
 }
 
 void MipsTargetELFStreamer::emitDirectiveSetMips32R2() {
   setCanHaveModuleDir(false);
 }
 
 void MipsTargetELFStreamer::emitDirectiveSetMips64() {
   setCanHaveModuleDir(false);

lib/Target/Mips/MipsTargetStreamer.h

 //===-- MipsTargetStreamer.h - Mips Target Streamer ------------*- C++ -*--===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef MIPSTARGETSTREAMER_H
 #define MIPSTARGETSTREAMER_H
 
 #include "llvm/MC/MCELFStreamer.h"
+#include "llvm/MC/MCRegisterInfo.h"
 #include "llvm/MC/MCStreamer.h"
 #include "MCTargetDesc/MipsABIFlagsSection.h"
 
 namespace llvm {
 
 struct MipsABIFlagsSection;
 
 class MipsTargetStreamer : public MCTargetStreamer {
public:
   void updateABIInfo(const PredicateLibrary &P) {
     ABIFlagsSection.setAllFromPredicates(P);
   }
 
   MipsABIFlagsSection &getABIFlagsSection() { return ABIFlagsSection; }
 
 protected:
   MipsABIFlagsSection ABIFlagsSection;
   
+  bool GPRInfoSet;
+  unsigned GPRBitMask;
+  int GPROffset;
+
+  bool FPRInfoSet;
+  unsigned FPRBitMask;
+  int FPROffset;
+  
+  bool FrameInfoSet;
+  int FrameOffset;
+  unsigned FrameReg;
+  unsigned ReturnReg;
+
 private:
   bool canHaveModuleDirective;
 };
 
 // This part is for ascii assembly output
 class MipsTargetAsmStreamer : public MipsTargetStreamer {
   formatted_raw_ostream &OS;
 

test/MC/Mips/mips-pdr-bad.s

+# RUN: not llvm-mc %s -arch=mips -mcpu=mips32r2 2>%t1
+# RUN: FileCheck %s < %t1 -check-prefix=ASM
+
+        .text
+        .ent # ASM: :[[@LINE]]:14: error: expected identifier after .ent
+        .ent bar, # ASM: :[[@LINE]]:19: error: unknown token in expression
+        .frame # ASM: :[[@LINE]]:16: error: expected stack register
+        .frame bar # ASM: :[[@LINE]]:16: error: expected stack register
+        .frame $sp, bar # ASM: :[[@LINE]]:25: error: frame size not an absolute expression
+        .frame $sp, 8, # ASM: :[[@LINE]]:24: error: expected return register
+        .mask  # ASM: :[[@LINE]]:16: error: bitmask not an absolute expression
+        .mask 0x80000000 # ASM: :[[@LINE]]:26: error: unexpected token in directive
+        .mask 0x80000000, # ASM: :[[@LINE]]:27: error: frame offset not an absolute expression
+        .fmask 0x80000000 # ASM: :[[@LINE]]:27: error: unexpected token in directive
+        .fmask 0x80000000, # ASM: :[[@LINE]]:28: error: frame offset not an absolute expression
+        .end _local_foo # ASM: :[[@LINE]]:25: error: .end used without .ent
+        .ent _local_foo, 2
+        .end _local_foo_bar # ASM: :[[@LINE]]:29: error: .end symbol does not match .ent symbol

test/MC/Mips/mips-pdr.s

+# RUN: llvm-mc %s -arch=mips -mcpu=mips32r2 -filetype=asm | \
+# RUN:   FileCheck %s -check-prefix=ASMOUT
+
+# RUN: llvm-mc %s -arch=mips -mcpu=mips32r2 -filetype=obj -o - | \
+# RUN:   llvm-readobj -s -section-data | \
+# RUN:     FileCheck %s -check-prefix=OBJOUT
+
+# ASMOUT: .text
+# ASMOUT:        .type _local_foo,@function
+# ASMOUT:        .ent _local_foo
+# ASMOUT:_local_foo:
+# ASMOUT:        .frame $fp,16,$ra
+# ASMOUT:        .mask 0x10101010,-4
+# ASMOUT:        .fmask 0x01010101,-8
+# ASMOUT:        .end _local_foo
+# ASMOUT:        .size local_foo,
+
+# OBJOUT: Section {
+# OBJOUT:     Name: .pdr
+# OBJOUT:     Type: SHT_PROGBITS (0x1)
+# OBJOUT:     Flags [ (0xB)
+# OBJOUT:       SHF_ALLOC (0x2)
+# OBJOUT:       SHF_WRITE (0x1)
+# OBJOUT:     ]
+# OBJOUT:     Size: 64
+# OBJOUT:     SectionData (
+# OBJOUT:       0000: 00000000 10101010 FFFFFFFC 01010101
+# OBJOUT:       0010: FFFFFFF8 00000010 0000001E 0000001F
+# OBJOUT:       0020: 00000000 10101010 FFFFFFFC 01010101
+# OBJOUT:       0030: FFFFFFF8 00000010 0000001E 0000001F
+# OBJOUT:     )
+# OBJOUT:   }
+
+# We should also check if relocation information was correctly generated.
+# OBJOUT:   Section {
+# OBJOUT:     Name: .rel.pdr
+# OBJOUT:     Type: SHT_REL (0x9)
+# OBJOUT:     Flags [ (0x0)
+# OBJOUT:     ]
+# OBJOUT:     Size: 16
+# OBJOUT:     SectionData (
+# OBJOUT:       0000: 00000000 00000202 00000020 00000802
+# OBJOUT:     )
+# OBJOUT:   }
+
+.text
+        .type _local_foo,@function
+        .ent _local_foo
+_local_foo:
+        .frame $fp,16,$ra
+        .mask 0x10101010,-4
+        .fmask 0x01010101,-8
+        .end _local_foo
+        .size local_foo,.-_local_foo
+
+        .globl _global_foo
+        .type _global_foo,@function
+        .ent _global_foo
+_global_foo:
+        .frame $fp,16,$ra
+        .mask 0x10101010,-4
+        .fmask 0x01010101,-8
+        .end _global_foo
+        .size global_foo,.-_global_foo