[PATCH] [lld][ELF]: Minimal implementation for ARM static linking

Rui Ueyama ruiu at google.com
Fri Jan 16 06:23:26 PST 2015


Please upload your latest revision to Phabricator so that I can submit on
behalf of you.

On Thu, Jan 15, 2015 at 10:10 PM, Denis Protivensky <
dprotivensky at accesssoftek.com> wrote:

> I see everyone has accepted the revision.
> How should I merge it as I don't have write permissions in the repo? Can
> someone help me with this please?
>
>
> ================
> Comment at: lib/ReaderWriter/ELF/ARM/ARMExecutableWriter.h:9
> @@ +8,3 @@
>
> +//===----------------------------------------------------------------------===//
> +#ifndef ARM_EXECUTABLE_WRITER_H
> +#define ARM_EXECUTABLE_WRITER_H
> ----------------
> ruiu wrote:
> > ifdef guard should include the path name.
> >
> >   #ifndef LLD_READER_WRITER_ELF_ARM_ARM_EXECUTABLE_WRITER_H
> >
> Good.
>
> ================
> Comment at: lib/ReaderWriter/ELF/ARM/ARMExecutableWriter.h:39
> @@ +38,3 @@
> +  ARMLinkingContext &_context;
> +  ARMTargetLayout<ELFT> &_ARMLayout;
> +};
> ----------------
> ruiu wrote:
> > _ARM -> _arm
> Fixed.
>
> ================
> Comment at: lib/ReaderWriter/ELF/ARM/ARMRelocationHandler.h:34
> @@ +33,3 @@
> +private:
> +  ARMTargetLayout<ARMELFType> &_ARMTargetLayout;
> +};
> ----------------
> ruiu wrote:
> > A name beginning with '_' followed by an uppercase character is reserved
> to the implementation (just like __), so please don't use such name.
> s/_ARMTargetLayout/_armTargetLayout/.
> Good, didn't know about that.
>
> ================
> Comment at: lib/ReaderWriter/ELF/ARM/ARMTarget.h:1
> @@ +1,2 @@
> +//===--------- lib/ReaderWriter/ELF/ARM/ARMTarget.h
> -----------------------===//
> +//
> ----------------
> ruiu wrote:
> > Why do we need this almost-empty header? Can't we directly include
> ARMLinkingContext.h?
> That's how the structure of files is organized.
> Targets.h includes platform-specific *Target.h file, and it in turn
> includes its linking context.
>
> http://reviews.llvm.org/D6716
>
> EMAIL PREFERENCES
>   http://reviews.llvm.org/settings/panel/emailpreferences/
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150116/163128bd/attachment.html>


More information about the llvm-commits mailing list