[PATCH] D24830: [ELF] - Linkerscript: implemented BYTE/SHORT/LONG/QUAD commands.

[George Rimar via llvm-commits]

grimar added inline comments.

================
Comment at: ELF/LinkerScript.cpp:424
@@ -417,1 +423,3 @@
   }
+  // It handles the bytes data command, which places some byte value into output
+  // section and moves the location counter in according to value data size.
----------------
ruiu wrote:
> Add a blank line before this.
> 
> Maybe just
> 
>   // Handle BYTE(), SHORT(), LONG(), or QUAD().
Done.

================
Comment at: ELF/LinkerScript.cpp:717
@@ +716,3 @@
+  int I = getSectionIndex(Name);
+  OutputSectionCommand *Cmd = dyn_cast_or_null<OutputSectionCommand>(
+      (I == INT_MAX) ? nullptr : Opt.Commands[I].get());
----------------
ruiu wrote:
> This looks odd. You can return early.
> 
>   if (I == INT_MAX)
>     return;
>   
Done.

================
Comment at: ELF/LinkerScript.cpp:1519
@@ +1518,3 @@
+BytesDataCommand *ScriptParser::readBytesDataCommand(StringRef Tok) {
+  unsigned Size = StringSwitch<unsigned>(Tok)
+                      .Case("BYTE", 1)
----------------
ruiu wrote:
> Use int instead of unsigned so to remove the cast to unsigned below.
Done.

================
Comment at: test/ELF/linkerscript/data-commands.s:2
@@ +1,3 @@
+# REQUIRES: x86
+# RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux %s -o %t.o
+
----------------
rafael wrote:
> Add a big endian test.
Done.

================
Comment at: test/ELF/linkerscript/data-commands.s:21
@@ +20,3 @@
+# CHECK:      Contents of section .foo:
+# CHECK-NEXT:  00e8 ff11ff22 11ff4433 2211ff88 77665544
+# CHECK-NEXT:  00f8 332211
----------------
rafael wrote:
> Don't check the address.
Done.


https://reviews.llvm.org/D24830