[llvm-commits] Initial cut of ARM MC ELF emitter (PATCH)
Jim Grosbach
grosbach at apple.com
Wed Sep 22 18:48:11 PDT 2010
On Sep 19, 2010, at 10:36 PM, Jason Kim wrote:
> Double oops.
> I somehow missed a compile breaking typo.
> Apologies for the noise.
> Tested against clean build dir.
> <arm-mc-elf-s01.patch4>
> Index: lib/Target/X86/X86TargetMachine.cpp
> ===================================================================
> --- lib/Target/X86/X86TargetMachine.cpp (revision 114214)
> +++ lib/Target/X86/X86TargetMachine.cpp (working copy)
> @@ -39,11 +39,11 @@
> }
> }
>
> -static MCStreamer *createMCStreamer(const Target &T, const std::string &TT,
> - MCContext &Ctx, TargetAsmBackend &TAB,
> - raw_ostream &_OS,
> - MCCodeEmitter *_Emitter,
> - bool RelaxAll) {
> +static MCStreamer *createX86MCStreamer(const Target &T, const std::string &TT,
> + MCContext &Ctx, TargetAsmBackend &TAB,
> + raw_ostream &_OS,
> + MCCodeEmitter *_Emitter,
> + bool RelaxAll) {
Why is this rename necessary? It's a static function, so there's no namespace collision with anything in another target.
> Triple TheTriple(TT);
> switch (TheTriple.getOS()) {
> case Triple::Darwin:
> @@ -58,7 +58,7 @@
> }
> }
>
> -extern "C" void LLVMInitializeX86Target() {
> +extern "C" void LLVMInitializeX86Target() {
> // Register the target.
> RegisterTargetMachine<X86_32TargetMachine> X(TheX86_32Target);
> RegisterTargetMachine<X86_64TargetMachine> Y(TheX86_64Target);
> @@ -81,9 +81,9 @@
>
> // Register the object streamer.
> TargetRegistry::RegisterObjectStreamer(TheX86_32Target,
> - createMCStreamer);
> + createX86MCStreamer);
> TargetRegistry::RegisterObjectStreamer(TheX86_64Target,
> - createMCStreamer);
> + createX86MCStreamer);
> }
>
>
> @@ -100,9 +100,9 @@
>
> /// X86TargetMachine ctor - Create an X86 target.
> ///
> -X86TargetMachine::X86TargetMachine(const Target &T, const std::string &TT,
> +X86TargetMachine::X86TargetMachine(const Target &T, const std::string &TT,
> const std::string &FS, bool is64Bit)
> - : LLVMTargetMachine(T, TT),
> + : LLVMTargetMachine(T, TT),
> Subtarget(TT, FS, is64Bit),
> DataLayout(Subtarget.getDataLayout()),
> FrameInfo(TargetFrameInfo::StackGrowsDown,
> @@ -217,13 +217,13 @@
> JITCodeEmitter &JCE) {
> // FIXME: Move this to TargetJITInfo!
> // On Darwin, do not override 64-bit setting made in X86TargetMachine().
> - if (DefRelocModel == Reloc::Default &&
> + if (DefRelocModel == Reloc::Default &&
> (!Subtarget.isTargetDarwin() || !Subtarget.is64Bit())) {
> setRelocationModel(Reloc::Static);
> Subtarget.setPICStyle(PICStyles::None);
> }
> -
>
> +
Looks like a few whitespace only changes slipped into the patch. It's best if cosmetic changes and functional changes are done in separate patches to make it easier to track which revisions are making "real" changes to the code.
> PM.add(createX86JITCodeEmitterPass(*this, JCE));
>
> return false;
> Index: lib/Target/ARM/ARMTargetMachine.cpp
> ===================================================================
> --- lib/Target/ARM/ARMTargetMachine.cpp (revision 114214)
> +++ lib/Target/ARM/ARMTargetMachine.cpp (working copy)
> @@ -31,6 +31,27 @@
> }
> }
>
> +// TODO(jasonwkim): [2010/09/19 10:31:16 PDT (Sunday)]
> +// This is duplicated code. Refactor this.
> +static MCStreamer *createARMMCStreamer(const Target &T, const std::string &TT,
It's static, so it doesn't technically need the "ARM" bit in the name. Not really a big deal, but I'd prefer changing things here to be consistent with X86 for now, then adjusting both later if it's more appropriate to include the prefix. That is, add new bits in a manner consistent with what's already there, then do the cleanup, or vice versa, but not both in the same patch. Same basic idea as above in that it keeps the patches small and single purpose, which makes them easier to test and review.
> + MCContext &Ctx, TargetAsmBackend &TAB,
> + raw_ostream &_OS,
> + MCCodeEmitter *_Emitter,
> + bool RelaxAll) {
> + Triple TheTriple(TT);
> + switch (TheTriple.getOS()) {
> + case Triple::Darwin:
> + return createMachOStreamer(Ctx, TAB, _OS, _Emitter, RelaxAll);
> + case Triple::MinGW32:
> + case Triple::MinGW64:
> + case Triple::Cygwin:
> + case Triple::Win32:
> + return createWinCOFFStreamer(Ctx, TAB, *_Emitter, _OS, RelaxAll);
ARM doesn't currently have any intention of supporting any of the Windows targets, so these should be omitted here, or possibly trigger an assert.
> + default:
> + return createELFStreamer(Ctx, TAB, _OS, _Emitter, RelaxAll);
> + }
> +}
> +
> extern "C" void LLVMInitializeARMTarget() {
> // Register the target.
> RegisterTargetMachine<ARMTargetMachine> X(TheARMTarget);
> @@ -39,6 +60,19 @@
> // Register the target asm info.
> RegisterAsmInfoFn A(TheARMTarget, createMCAsmInfo);
> RegisterAsmInfoFn B(TheThumbTarget, createMCAsmInfo);
> +
> + // Register the MC Code Emitter
> + TargetRegistry::RegisterCodeEmitter(TheARMTarget,
> + createARMMCCodeEmitter);
> + TargetRegistry::RegisterCodeEmitter(TheThumbTarget,
> + createARMMCCodeEmitter);
> +
> + // Register the object streamer.
> + TargetRegistry::RegisterObjectStreamer(TheARMTarget,
> + createARMMCStreamer);
> + TargetRegistry::RegisterObjectStreamer(TheThumbTarget,
> + createARMMCStreamer);
> +
> }
>
> /// TargetMachine ctor - Create an ARM architecture model.
> @@ -46,23 +80,28 @@
> ARMBaseTargetMachine::ARMBaseTargetMachine(const Target &T,
> const std::string &TT,
> const std::string &FS,
> + StringRef TargetDescription,
> bool isThumb)
> : LLVMTargetMachine(T, TT),
> Subtarget(TT, FS, isThumb),
> FrameInfo(Subtarget),
> JITInfo(),
> - InstrItins(Subtarget.getInstrItineraryData()) {
> + InstrItins(Subtarget.getInstrItineraryData()),
> + DataLayout(TargetDescription),
The TargetData is accessible from the TargetMachine instance via the getTargetData() accessor function. Is that not sufficient for what you're looking to accomplish?
> + ELFWriterInfo(*this)
This creates an ELFWriter() even when the target doesn't need or want one. That doesn't seem right. I see X86 does the same thing, though. Doesn't seem right there, either, but perhaps I'm missing something and someone familiar with that area can comment.
> <snip>
> Index: lib/Target/ARM/ARMELFWriterInfo.h
> ===================================================================
> --- lib/Target/ARM/ARMELFWriterInfo.h (revision 0)
> +++ lib/Target/ARM/ARMELFWriterInfo.h (revision 0)
> @@ -0,0 +1,66 @@
> +//===-- ARMELFWriterInfo.h - ELF Writer Info for ARM ------------*- C++ -*-===//
> +//
> +// The LLVM Compiler Infrastructure
> +//
> +// This file is distributed under the University of Illinois Open Source
> +// License. See LICENSE.TXT for details.
> +//
> +//===----------------------------------------------------------------------===//
> +//
> +// This file implements ELF writer information for the ARM backend.
> +//
> +//===----------------------------------------------------------------------===//
> +
> +#ifndef ARM_ELF_WRITER_INFO_H
> +#define ARM_ELF_WRITER_INFO_H
> +
> +#include "llvm/Target/TargetELFWriterInfo.h"
> +
> +namespace llvm {
> +
> + class ARMELFWriterInfo : public TargetELFWriterInfo {
> +
> + // ELF Relocation types for ARM
> + // FIXME: TODO(jasonwkim): [2010/09/17 14:52:25 PDT (Friday)]
FIXME's don't need name/date info. "svn annotate" does the trick for that information.
> + // Come up with a better way to orgnize the 100+ ARM reloc types.
> +
> + enum ARMRelocationType {
> + };
> +
> + public:
> + ARMELFWriterInfo(TargetMachine &TM);
> + virtual ~ARMELFWriterInfo();
> +
> + /// getRelocationType - Returns the target specific ELF Relocation type.
> + /// 'MachineRelTy' contains the object code independent relocation type
> + virtual unsigned getRelocationType(unsigned MachineRelTy) const;
> +
> + /// hasRelocationAddend - True if the target uses an addend in the
> + /// ELF relocation entry.
> + virtual bool hasRelocationAddend() const { return is64Bit ? true : false; }
There's no 64-bit here. Should this always return 'false' for ARM? Probably shouldn't reference 64Bit in any case, I'd think.
> +
> + /// getDefaultAddendForRelTy - Gets the default addend value for a
> + /// relocation entry based on the target ELF relocation type.
> + virtual long int getDefaultAddendForRelTy(unsigned RelTy,
> + long int Modifier = 0) const;
> +
> + /// isPCRelativeRel - True if the relocation type is pc relative
> + virtual bool isPCRelativeRel(unsigned RelTy) const;
> +
> + /// getJumpTableRelocationTy - Returns the machine relocation type used
> + /// to reference a jumptable.
> + virtual unsigned getAbsoluteLabelMachineRelTy() const;
Advance warning: ARM does jump tables with custom handling. This may be an "interesting" bit.
> +
> + /// computeRelocation - Some relocatable fields could be relocated
> + /// directly, avoiding the relocation symbol emission, compute the
> + /// final relocation value for this symbol.
> + virtual long int computeRelocation(unsigned SymOffset, unsigned RelOffset,
> + unsigned RelTy) const;
> + };
> +
> +} // end llvm namespace
> +
> +#endif // ARM_ELF_WRITER_INFO_H
> Index: lib/Target/ARM/ARMELFWriterInfo.cpp
> ===================================================================
> --- lib/Target/ARM/ARMELFWriterInfo.cpp (revision 0)
> +++ lib/Target/ARM/ARMELFWriterInfo.cpp (revision 0)
> @@ -0,0 +1,65 @@
> +//===-- ARMELFWriterInfo.cpp - ELF Writer Info for the ARM backend --------===//
> +//
> +// The LLVM Compiler Infrastructure
> +//
> +// This file is distributed under the University of Illinois Open Source
> +// License. See LICENSE.TXT for details.
> +//
> +//===----------------------------------------------------------------------===//
> +//
> +// This file implements ELF writer information for the ARM backend.
> +//
> +//===----------------------------------------------------------------------===//
> +
> +#include "ARMELFWriterInfo.h"
> +#include "ARMRelocations.h"
> +#include "llvm/Function.h"
> +#include "llvm/Support/ErrorHandling.h"
> +#include "llvm/Target/TargetData.h"
> +#include "llvm/Target/TargetMachine.h"
> +
> +using namespace llvm;
> +
> +//===----------------------------------------------------------------------===//
> +// Implementation of the ARMELFWriterInfo class
> +//===----------------------------------------------------------------------===//
> +
> +ARMELFWriterInfo::ARMELFWriterInfo(TargetMachine &TM)
> + : TargetELFWriterInfo(TM) {
> + // silently OK construction
> +}
> +
> +ARMELFWriterInfo::~ARMELFWriterInfo() {}
> +
> +unsigned ARMELFWriterInfo::getRelocationType(unsigned MachineRelTy) const {
> + assert(0 && "ARMELFWriterInfo::getRelocationType() not implemented");
> + return 0;
> +}
> +
> +long int ARMELFWriterInfo::getDefaultAddendForRelTy(unsigned RelTy,
> + long int Modifier) const {
> + assert(0 && "ARMELFWriterInfo::getDefaultAddendForRelTy() not implemented");
> + return 0;
> +}
> +
> +unsigned ARMELFWriterInfo::getRelocationTySize(unsigned RelTy) const {
> + assert(0 && "ARMELFWriterInfo::getRelocationTySize() not implemented");
> + return 0;
> +}
> +
> +bool ARMELFWriterInfo::isPCRelativeRel(unsigned RelTy) const {
> + assert(0 && "ARMELFWriterInfo::isPCRelativeRel() not implemented");
> + return 1;
> +}
> +
> +unsigned ARMELFWriterInfo::getAbsoluteLabelMachineRelTy() const {
> + assert(0 && "ARMELFWriterInfo::getAbsoluteLabelMachineRelTy() not implemented");
> + return 0;
> +}
> +
> +long int ARMELFWriterInfo::computeRelocation(unsigned SymOffset,
> + unsigned RelOffset,
> + unsigned RelTy) const {
> + assert(0 && "ARMELFWriterInfo::getAbsoluteLabelMachineRelTy() not implemented");
> + return 0;
> +}
> Index: lib/Target/ARM/CMakeLists.txt
> ===================================================================
> --- lib/Target/ARM/CMakeLists.txt (revision 114214)
> +++ lib/Target/ARM/CMakeLists.txt (working copy)
> @@ -21,6 +21,7 @@
> ARMCodeEmitter.cpp
> ARMConstantIslandPass.cpp
> ARMConstantPoolValue.cpp
> + ARMELFWriterInfo.cpp
> ARMExpandPseudoInsts.cpp
> ARMFastISel.cpp
> ARMGlobalMerge.cpp
> Index: lib/Target/ARM/ARMTargetMachine.h
> ===================================================================
> --- lib/Target/ARM/ARMTargetMachine.h (revision 114214)
> +++ lib/Target/ARM/ARMTargetMachine.h (working copy)
> @@ -16,7 +16,9 @@
>
> #include "llvm/Target/TargetMachine.h"
> #include "llvm/Target/TargetData.h"
> +#include "llvm/MC/MCStreamer.h"
> #include "ARMInstrInfo.h"
> +#include "ARMELFWriterInfo.h"
> #include "ARMFrameInfo.h"
> #include "ARMJITInfo.h"
> #include "ARMSubtarget.h"
> @@ -38,10 +40,21 @@
> InstrItineraryData InstrItins;
> Reloc::Model DefRelocModel; // Reloc model before it's overridden.
>
> +protected:
> + const TargetData DataLayout; // Calculates type size & alignment
> + ARMELFWriterInfo ELFWriterInfo;
> +
> public:
> ARMBaseTargetMachine(const Target &T, const std::string &TT,
> - const std::string &FS, bool isThumb);
> + const std::string &FS,
> + StringRef Target,
> + bool isThumb);
>
> + virtual const TargetData *getTargetData() const { return &DataLayout; }
> + virtual const ARMELFWriterInfo *getELFWriterInfo() const {
> + return Subtarget.isTargetELF() ? &ELFWriterInfo : 0;
> + };
> +
> virtual const ARMFrameInfo *getFrameInfo() const { return &FrameInfo; }
> virtual ARMJITInfo *getJITInfo() { return &JITInfo; }
> virtual const ARMSubtarget *getSubtargetImpl() const { return &Subtarget; }
> @@ -63,7 +76,6 @@
> ///
> class ARMTargetMachine : public ARMBaseTargetMachine {
> ARMInstrInfo InstrInfo;
> - const TargetData DataLayout; // Calculates type size & alignment
> ARMTargetLowering TLInfo;
> ARMSelectionDAGInfo TSInfo;
> public:
> @@ -93,7 +105,6 @@
> class ThumbTargetMachine : public ARMBaseTargetMachine {
> // Either Thumb1InstrInfo or Thumb2InstrInfo.
> OwningPtr<ARMBaseInstrInfo> InstrInfo;
> - const TargetData DataLayout; // Calculates type size & alignment
> ARMTargetLowering TLInfo;
> ARMSelectionDAGInfo TSInfo;
> public:
> Index: lib/Target/TargetELFWriterInfo.cpp
> ===================================================================
> --- lib/Target/TargetELFWriterInfo.cpp (revision 114214)
> +++ lib/Target/TargetELFWriterInfo.cpp (working copy)
> @@ -18,6 +18,8 @@
> using namespace llvm;
>
> TargetELFWriterInfo::TargetELFWriterInfo(TargetMachine &tm) : TM(tm) {
> + // TODO(jasonwkim): [2010/09/18 09:57:44 PDT (Saturday)]
> + // Fixme: NaCl 64bit has 32bit pointers. Need a better test.
> is64Bit = TM.getTargetData()->getPointerSizeInBits() == 64;
> isLittleEndian = TM.getTargetData()->isLittleEndian();
> }
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20100922/84924e5d/attachment.html>
More information about the llvm-commits
mailing list