[PATCH][AsmParser][MC] Allow asm parser to emit elf header flags
Jack Carter
Jack.Carter at imgtec.com
Wed Oct 2 14:39:02 PDT 2013
Ping :-)
________________________________
From: Vladimir Medic
Sent: Tuesday, September 10, 2013 12:52 AM
To: llvm-commits at cs.uiuc.edu
Cc: Jack Carter; Jim Grosbach
Subject: RE: [PATCH][AsmParser][MC] Allow asm parser to emit elf header flags
Hi all,
This patch handles LLVM standalone assembler (llvm-mc) ELF flag setting based on input file
directive processing.
Mips assembly requires processing inline directives that directly and
indirectly affect the output ELF header flags. This patch handles one
".abicalls".
To process these directives we are following the model the code generator
uses by storing state in a container as we go through processing and when
we detect the end of input file processing, AsmParser is notified and we
update the ELF header flags through a MipsELFStreamer method with a call from
MCTargetAsmParser::emitEndOfAsmFile(MCStreamer &OutStreamer).
It has been suggested that we use AsmPrinter::EmitEndOfAsmFile() to
trigger the updating of the ELF header flags, but as far as we can tell
MipsAsmPrinter is not available for the AsmParser.
Also, the location of the call to emitEndOfAsmFile() is our best guess for when
the input assembler file processing is at an end. If there is a better location
for this please be specific and we will make the change.
We have separate enum values for the relocation models than are in the ELF
defines for Mips because static (non-shared) model is an active state, but is
represented by 0 in the flags. Also we want to have a "default" state.
Kind regards
Vladimir
________________________________
From: Jim Grosbach [grosbach at apple.com]
Sent: Monday, August 26, 2013 11:08 PM
To: Vladimir Medic
Cc: llvm-commits at cs.uiuc.edu; Jack Carter
Subject: Re: [PATCH][AsmParser][MC] Allow asm parser to emit elf header flags
Hi Vladimir,
Detailed comments inline below.
Generally speaking, please run new code through clang-format. There are lots of small formatting problems here and there that tool will help clean up without having to fuss about the details.
I don’t have much to comment on regarding the Mips specific side of things. You guys know that stuff better than I do.
Index: test/MC/Mips/mips_directives.s
===================================================================
--- test/MC/Mips/mips_directives.s (revision 188764)
+++ test/MC/Mips/mips_directives.s (working copy)
@@ -3,7 +3,10 @@
# CHECK: .text
# CHECK: $BB0_2:
$BB0_2:
- .ent directives_test
+ .ent directives_test
+ .abicalls
+ .option pic0
+ .set mips16
.frame $sp,0,$ra
.mask 0x00000000,0
.fmask 0x00000000,0
Index: include/llvm/MC/MCTargetAsmParser.h
===================================================================
--- include/llvm/MC/MCTargetAsmParser.h (revision 188764)
+++ include/llvm/MC/MCTargetAsmParser.h (working copy)
@@ -174,6 +174,10 @@
virtual void convertToMapAndConstraints(unsigned Kind,
const SmallVectorImpl<MCParsedAsmOperand*> &Operands) = 0;
+
+ /// setAssemblerDialect - Target specific cleanup such as Ehdr flag setting.
+ /// Analogous to MipsAsmPrinter::EmitEndOfAsmFile().
+ virtual void emitEndOfObjFile(MCStreamer &Out) { }
The comment doesn’t match the function.
Relatedly, please refer to http://llvm.org/docs/CodingStandards.html#doxygen-use-in-documentation-comments for the latest style preferences for these comments. I wouldn’t trust that the surrounding code is up-to-date.
That said, why is this function necessary at all? EmitEndOfAsmFile() is called even for object files.
};
} // End llvm namespace
Index: lib/Target/Mips/AsmParser/MipsAsmParser.cpp
===================================================================
--- lib/Target/Mips/AsmParser/MipsAsmParser.cpp (revision 188764)
+++ lib/Target/Mips/AsmParser/MipsAsmParser.cpp (working copy)
@@ -7,6 +7,8 @@
//
//===----------------------------------------------------------------------===//
+#include "AsmParser/MipsAsmFlags.h"
+#include "MCTargetDesc/MipsELFStreamer.h"
#include "MCTargetDesc/MipsMCTargetDesc.h"
#include "MipsRegisterInfo.h"
#include "llvm/ADT/StringSwitch.h"
@@ -63,6 +65,7 @@
MCSubtargetInfo &STI;
MCAsmParser &Parser;
MipsAssemblerOptions Options;
+ MipsMCAsmFlags Flags;
bool hasConsumedDollar;
#define GET_ASSEMBLER_HEADER
@@ -166,6 +169,8 @@
bool parseDirectiveWord(unsigned Size, SMLoc L);
+ bool parseDirectiveOption();
+
MCSymbolRefExpr::VariantKind getVariantKind(StringRef Symbol);
bool isMips64() const {
@@ -198,11 +203,20 @@
bool processInstruction(MCInst &Inst, SMLoc IDLoc,
SmallVectorImpl<MCInst> &Instructions);
+ void emitEndOfObjFile(MCStreamer &Out);
+
+ void setAbiTy();
+ void setArchTy();
+ void setAseTy();
setXXX() is the usual name for the setter of a getter/setter pair. These are a different sort of thing, and should be named differently. Comments about what they do would be good.
+
public:
MipsAsmParser(MCSubtargetInfo &sti, MCAsmParser &parser)
: MCTargetAsmParser(), STI(sti), Parser(parser), hasConsumedDollar(false) {
// Initialize the set of available features.
setAvailableFeatures(ComputeAvailableFeatures(STI.getFeatureBits()));
+ setAbiTy();
+ setArchTy();
+ setAseTy();
Since these are just called once, and all in order, why have three separate functions at all?
}
MCAsmParser &getParser() const { return Parser; }
@@ -446,6 +460,43 @@
return MipsInsts[Opcode];
}
+void MipsAsmParser::setAbiTy() {
+ uint64_t FeatureBits = STI.getFeatureBits();
+ unsigned AbiType = MipsMCAsmFlags::MAF_ABI_DEFAULT;
+ if (FeatureBits & Mips::FeatureN32)
+ AbiType = MipsMCAsmFlags::MAF_ABI_N32;
+ else if (FeatureBits & Mips::FeatureO32)
+ AbiType = MipsMCAsmFlags::MAF_ABI_O32;
+ else if (FeatureBits & Mips::FeatureN64)
+ AbiType = MipsMCAsmFlags::MAF_ABI_N64;
+
+ Flags.setAbi(AbiType);
+}
+void MipsAsmParser::setArchTy() {
+ uint64_t FeatureBits = STI.getFeatureBits();
+ unsigned ArchType = MipsMCAsmFlags::MAF_ARCH_DEFAULT;
+ if (FeatureBits & Mips::FeatureMips64r2)
+ ArchType = MipsMCAsmFlags::MAF_ARCH_64R2;
+ else if (FeatureBits & Mips::FeatureMips64)
+ ArchType = MipsMCAsmFlags::MAF_ARCH_64R1;
+ else if (FeatureBits & Mips::FeatureMips32r2)
+ ArchType = MipsMCAsmFlags::MAF_ARCH_32R2;
+ else if (FeatureBits & Mips::FeatureMips32)
+ ArchType = MipsMCAsmFlags::MAF_ARCH_32R1;
+
+ Flags.setArch(ArchType);
+
+}
+void MipsAsmParser::setAseTy() {
+ uint64_t FeatureBits = STI.getFeatureBits();
+ unsigned AseType = MipsMCAsmFlags::MAF_ASE_DEFAULT;
+ if (FeatureBits & Mips::FeatureMips16)
+ AseType = MipsMCAsmFlags::MAF_ASE_MIPS16;
+ else if (FeatureBits & Mips::FeatureMicroMips)
+ AseType = MipsMCAsmFlags::MAF_ASE_MICROMIPS;
+ Flags.setAse(AseType);
+}
+
bool MipsAsmParser::processInstruction(MCInst &Inst, SMLoc IDLoc,
SmallVectorImpl<MCInst> &Instructions) {
const MCInstrDesc &MCID = getInstDesc(Inst.getOpcode());
@@ -1807,14 +1858,30 @@
} else if (Tok.getString() == "nomacro") {
return parseSetNoMacroDirective();
} else if (Tok.getString() == "nomips16") {
- // Ignore this directive for now.
- Parser.eatToEndOfStatement();
+ if ((STI.getFeatureBits() & Mips::FeatureMips16))
+ STI.ToggleFeature(Mips::FeatureMips16);
+ Flags.setAse(MipsMCAsmFlags::MAF_ASE_DEFAULT);
+ Parser.Lex();
return false;
- } else if (Tok.getString() == "nomicromips") {
- // Ignore this directive for now.
- Parser.eatToEndOfStatement();
+ } else if (Tok.getString() == "mips16") {
+ if (!(STI.getFeatureBits() & Mips::FeatureMips16))
+ STI.ToggleFeature(Mips::FeatureMips16);
+ Flags.setAse(MipsMCAsmFlags::MAF_ASE_MIPS16);
+ Parser.Lex();
return false;
- } else {
+ }else if (Tok.getString() == "nomicromips") {
+ if (STI.getFeatureBits() & Mips::FeatureMicroMips)
+ STI.ToggleFeature(Mips::FeatureMicroMips);
+ Flags.setAse(MipsMCAsmFlags::MAF_ASE_DEFAULT);
+ Parser.Lex();
+ return false;
+ } else if (Tok.getString() == "micromips") {
+ if (!(STI.getFeatureBits() & Mips::FeatureMicroMips))
+ STI.ToggleFeature(Mips::FeatureMicroMips);
+ Flags.setAse(MipsMCAsmFlags::MAF_ASE_MICROMIPS);
+ Parser.Lex();
+ return false;
+ }else {
// It is just an identifier, look for an assignment.
parseSetAssignment();
return false;
@@ -1848,6 +1915,36 @@
return false;
}
+bool MipsAsmParser::parseDirectiveOption() {
+ // Get the option token
+ AsmToken Tok = Parser.getTok();
+ // At the moment only identifiers are supported
+ if (Tok.isNot(AsmToken::Identifier)) {
+ // Clear line
+ Parser.eatToEndOfStatement();
+ return Error(Tok.getLoc(), "unexpected token in directive");
+ }
+ StringRef Option = Tok.getIdentifier();
+ if (Option == "pic0") {
+ Flags.setRelocationModel(MipsMCAsmFlags::MAF_RM_STATIC);
+ Parser.Lex();
+ if (Parser.getTok().isNot(AsmToken::EndOfStatement))
+ return Error(Parser.getTok().getLoc(), "unexpected token in directive");
+ return false;
+ }
+
+ if (Option == "pic2") {
+ Flags.setRelocationModel(MipsMCAsmFlags::MAF_RM_PIC);
+ Parser.Lex();
+ if (Parser.getTok().isNot(AsmToken::EndOfStatement))
+ return Error(Parser.getTok().getLoc(), "unexpected token in directive");
+ return false;
+ }
+ // Unknown option
+ Parser.eatToEndOfStatement();
+ return Warning(Parser.getTok().getLoc(), "unknown option in directive");
+}
+
bool MipsAsmParser::ParseDirective(AsmToken DirectiveID) {
StringRef IDVal = DirectiveID.getString();
@@ -1897,9 +1994,29 @@
return false;
}
+ if (IDVal == ".abicalls") {
+ // TODO: Set the Reloc::PIC_
+ Flags.setRelocationModel(MipsMCAsmFlags::MAF_RM_PIC);
+ if (Parser.getTok().isNot(AsmToken::EndOfStatement))
+ return Error(Parser.getTok().getLoc(), "unexpected token in directive");
+ return false;
+ }
+ if (IDVal == ".option") {
+ return parseDirectiveOption();
+ }
+
return true;
}
+/// setAssemblerDialect -
+/// Analogous to MipsAsmPrinter::EmitEndOfAsmFile().
Incorrect comment again.
+void MipsAsmParser::emitEndOfObjFile(MCStreamer &OutStreamer)
+{
+ if (MipsELFStreamer *MES = dyn_cast<MipsELFStreamer>(&OutStreamer))
+ MES->emitELFHeaderFlagsAsm(Flags);
+ MCTargetAsmParser::emitEndOfObjFile(OutStreamer);
+}
This can definitely be done without this function being added. See how ARM handles the EHDR flags.
+
extern "C" void LLVMInitializeMipsAsmParser() {
RegisterMCAsmParser<MipsAsmParser> X(TheMipsTarget);
RegisterMCAsmParser<MipsAsmParser> Y(TheMipselTarget);
Index: lib/Target/Mips/AsmParser/MipsAsmFlags.h
===================================================================
--- lib/Target/Mips/AsmParser/MipsAsmFlags.h (revision 0)
+++ lib/Target/Mips/AsmParser/MipsAsmFlags.h (revision 0)
@@ -0,0 +1,93 @@
+//=== MipsMCAsmFlags.h - MipsMCAsmFlags --------------------------------===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENCE.TXT for details.
+//
+//===-------------------------------------------------------------------===//
+#ifndef MIPSMCASMFLAGS_H_
+#define MIPSMCASMFLAGS_H_
+
+namespace llvm {
+class MipsMCAsmFlags;
+
+class MipsMCAsmFlags {
+public:
+ enum MAFAbiTy {
+ MAF_ABI_DEFAULT,
+ MAF_ABI_O32,
+ MAF_ABI_N32,
+ MAF_ABI_N64
+ } MAFAbi;
+
+ // These act as bit flags because more that one can be
+ // active at the same time, sometimes ;-)
+ enum MAFRelocationModelTy {
+ MAF_RM_DEFAULT = 0,
+ MAF_RM_STATIC = 1,
+ MAF_RM_CPIC = 2,
+ MAF_RM_PIC = 4
+ } MAFRelocationModel;
+
+ enum MAFArchTy {
+ MAF_ARCH_DEFAULT,
+ MAF_ARCH_32R1,
+ MAF_ARCH_32R2,
+ MAF_ARCH_64R1,
+ MAF_ARCH_64R2
+ } MAFArch;
+
+ enum MAFAseTy {
+ MAF_ASE_DEFAULT,
+ MAF_ASE_MIPS16,
+ MAF_ASE_MICROMIPS
+ } MAFAse;
+
Shouldn’t these be part of the MIPS ELF object file information if they’re not already? They’re not really internal to the MIPS AsmParser, right?
+public:
+ MipsMCAsmFlags()
+ : abi(MAF_ABI_DEFAULT),
+ model(MAF_RM_DEFAULT),
+ arch(MAF_ARCH_DEFAULT),
+ ase(MAF_ASE_DEFAULT) {}
+
+ ~MipsMCAsmFlags() {}
+
+ // This is simplistic. Mips16 may need on off switches.
+ void setAbi(unsigned Abi) {(abi = Abi);}
+ void setRelocationModel(unsigned RM) {(model |= RM);}
+ void setArch(unsigned Arch) {(arch = Arch);}
+ void setAse(unsigned Ase) {(ase = Ase);}
+
+ bool isAbiO32() const {return (abi == MAF_ABI_O32);}
+ bool isAbiN32() const {return (abi == MAF_ABI_N32);}
+ bool isAbiN64() const {return (abi == MAF_ABI_N64);}
+ bool isAbiDefault() const {return (abi == MAF_ABI_DEFAULT);}
+
+ bool isModelCpic() const {return (model & MAF_RM_CPIC) == MAF_RM_CPIC;}
+ bool isModelPic() const {return (model & MAF_RM_PIC) == MAF_RM_PIC;}
+ bool isModelStatic() const {return (model & MAF_RM_STATIC) == MAF_RM_STATIC;}
+ bool isModelDefault() const {
+ return (model & MAF_RM_DEFAULT) == MAF_RM_DEFAULT;
+ }
+
+ bool isArch32r1() const {return (arch == MAF_ARCH_32R1);}
+ bool isArch32r2() const {return (arch == MAF_ARCH_32R2);}
+ bool isArch64r1() const {return (arch == MAF_ARCH_64R1);}
+ bool isArch64r2() const {return (arch == MAF_ARCH_64R2);}
+ bool isArchDefault() const {return (arch == MAF_ARCH_DEFAULT);}
+
+ bool isAseMips16() const {return (ase == MAF_ASE_MIPS16);}
+ bool isAseMicroMips() const {return (ase == MAF_ASE_MICROMIPS);}
+ bool isAseDefault() const {return (ase == MAF_ASE_DEFAULT);}
+
+private:
+ unsigned abi; // o32, n32, n64, etc.
+ unsigned model; // pic, cpic, etc.
+ unsigned arch; // 32, 32r2, 64,etc.
+ unsigned ase; // 16, MicroMips, etc.
Class member variables shouldn’t be all-lower-case. See http://llvm.org/docs/CodingStandards.html#name-types-functions-variables-and-enumerators-properly
+};
+
+}
+
+#endif /* MIPSMCASMFLAGS_H_ */
Index: lib/Target/Mips/MCTargetDesc/MipsELFStreamer.cpp
===================================================================
--- lib/Target/Mips/MCTargetDesc/MipsELFStreamer.cpp (revision 188764)
+++ lib/Target/Mips/MCTargetDesc/MipsELFStreamer.cpp (working copy)
@@ -6,6 +6,7 @@
// License. See LICENSE.TXT for details.
//
//===-------------------------------------------------------------------===//
+#include "AsmParser/MipsAsmFlags.h"
#include "MCTargetDesc/MipsELFStreamer.h"
#include "MipsSubtarget.h"
#include "llvm/MC/MCAssembler.h"
@@ -74,6 +75,59 @@
MCA.setELFHeaderEFlags(EFlags);
}
+ // For llvm-mc. Set a group of ELF header flags
+ void
+ MipsELFStreamer::emitELFHeaderFlagsAsm(const MipsMCAsmFlags &Flags) {
+
+ // Update e_header flags
+ MCAssembler& MCA = getAssembler();
+ unsigned EFlags = MCA.getELFHeaderEFlags();
+
+ if (Flags.isAseMips16())
+ EFlags |= ELF::EF_MIPS_ARCH_ASE_M16;
+ else if (Flags.isAseMicroMips())
+ EFlags |= ELF::EF_MIPS_ARCH_ASE_M16;
+ else
+ EFlags |= ELF::EF_MIPS_NOREORDER;
+
+ // Architecture
+ if (Flags.isArch64r2())
+ EFlags |= ELF::EF_MIPS_ARCH_64R2;
+ else if (Flags.isArch64r1())
+ EFlags |= ELF::EF_MIPS_ARCH_64;
+ else if (Flags.isArch32r2())
+ EFlags |= ELF::EF_MIPS_ARCH_32R2;
+ else
+ EFlags |= ELF::EF_MIPS_ARCH_32;
+
+ if (Flags.isAseMicroMips())
+ EFlags |= ELF::EF_MIPS_MICROMIPS;
+
+ // ABI
+ if (Flags.isAbiO32() || Flags.isAbiDefault())
+ EFlags |= ELF::EF_MIPS_ABI_O32;
+// else if (Flags.isAbiN32())
+// EFlags |= ELF::EF_MIPS_ABI2;
+// else if (Flags.isAbiN64())
+// EFlags |= ELF::EF_MIPS_ABI2;
Please remove commented out code.
+ else
+ llvm_unreachable("Unsupported ABI for e_flags");
+
+ // Relocation Model
+ // TODO: Need to add -mabicalls and -mno-abicalls flags.
+ // Currently we assume that -mabicalls is the default.
+ if (Flags.isModelCpic() || Flags.isModelDefault())
+ EFlags |= ELF::EF_MIPS_CPIC;
+ if (Flags.isModelPic() || Flags.isModelDefault())
+ EFlags |= ELF::EF_MIPS_PIC;
+ else if (Flags.isModelStatic())
+ ; // Do nothing for Reloc::Static
+ else
+ llvm_unreachable("Unsupported relocation model for e_flags");
+
+ MCA.setELFHeaderEFlags(EFlags);
+ }
+
// For llc. Set a symbol's STO flags
void
MipsELFStreamer::emitMipsSTOCG(const MipsSubtarget &Subtarget,
Index: lib/Target/Mips/MCTargetDesc/MipsELFStreamer.h
===================================================================
--- lib/Target/Mips/MCTargetDesc/MipsELFStreamer.h (revision 188764)
+++ lib/Target/Mips/MCTargetDesc/MipsELFStreamer.h (working copy)
@@ -13,6 +13,7 @@
namespace llvm {
class MipsAsmPrinter;
+class MipsMCAsmFlags;
class MipsSubtarget;
class MCSymbol;
@@ -26,6 +27,7 @@
~MipsELFStreamer() {}
void emitELFHeaderFlagsCG(const MipsSubtarget &Subtarget);
+ void emitELFHeaderFlagsAsm(const MipsMCAsmFlags &MAFlags);
void emitMipsSTOCG(const MipsSubtarget &Subtarget,
MCSymbol *Sym,
unsigned Val);
On Aug 20, 2013, at 7:28 AM, Vladimir Medic <Vladimir.Medic at imgtec.com<mailto:Vladimir.Medic at imgtec.com>> wrote:
The assembler effects changes to the ELF environment through commandline
options and inline directives. These need to be written out into the ELF
header's eflags.
The mechanism for this is through the MCELFStreamer. We have modeled the
assembler's version after the previously implemented codegen one.
I'm looking forward to your comments and remarks
Vladimir
<MipsElfFlags.patch>_______________________________________________
llvm-commits mailing list
llvm-commits at cs.uiuc.edu<mailto:llvm-commits at cs.uiuc.edu>
http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20131002/c252560e/attachment.html>
More information about the llvm-commits
mailing list