[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