[PATCH] D19663: [ELF] - Basic support of linkerscript commands: DATA_SEGMENT_ALIGN, DATA_SEGMENT_END, CONSTANT

[Rui Ueyama via llvm-commits]

ruiu accepted this revision.
ruiu added a comment.
This revision is now accepted and ready to land.

LGTM


================
Comment at: ELF/LinkerScript.cpp:104
@@ -103,1 +103,3 @@
 
+uint64_t getConstantValue(StringRef C) {
+  // Two different page sizes can be used to perform optimization.
----------------
static?

================
Comment at: ELF/LinkerScript.cpp:105-109
@@ +104,7 @@
+uint64_t getConstantValue(StringRef C) {
+  // Two different page sizes can be used to perform optimization.
+  // Binary can be enlarged by up to one page bytes, but one page
+  // of virtual memory will be saved. DATA_SEGMENT_ALIGN() command implements
+  // this in other linkers. We currently - don't, so page values are the
+  // same for both constants.
+  if (C == "COMMONPAGESIZE" || C == "MAXPAGESIZE")
----------------
Are you describing the same thing in two places in the same commit? Maybe you want to leave one of them and remove the other?

================
Comment at: ELF/LinkerScript.cpp:137
@@ +136,3 @@
+    expect("(");
+    uint64_t C = getConstantValue(next());
+    expect(")");
----------------
Can you consistently use `V` as a return value in this function just like other `if` clauses do?


https://reviews.llvm.org/D19663