[llvm-commits] Initial cut of ARM MC ELF emitter (PATCH)
Jason Kim
jasonwkim at google.com
Wed Sep 22 20:55:24 PDT 2010
Hi Jim,
Just makes debugging a little easier at times, is all (explicit
names), makes it easier to realize what function I am in.
Do you want me to revert the name change?
BTW, I *just* sent an update to this patch, it fixes a bug I introduced in it .
(sorry!) and Thanks
-jason
On Wed, Sep 22, 2010 at 6:48 PM, Jim Grosbach <grosbach at apple.com> wrote:
>
> 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();
> }
>
More information about the llvm-commits
mailing list