[PATCH] [lld][ELF]: Minimal implementation for ARM static linking
Denis Protivensky
dprotivensky at accesssoftek.com
Thu Jan 15 22:10:37 PST 2015
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/
More information about the llvm-commits
mailing list