[PATCH] D24891: [ELF] Support -z max-page-size option

[Rui Ueyama via llvm-commits]

ruiu added inline comments.

================
Comment at: ELF/Config.h:141
@@ -139,2 +140,3 @@
+  uint64_t MaxPageSize;
   uint64_t ZStackSize = -1;
   unsigned LtoJobs;
----------------
Please remove `= -1` because the variable is always assigned now.

================
Comment at: ELF/Driver.cpp:277
@@ -274,1 +276,3 @@
+        error("invalid " + Key + ": " + Value);
+    }
   }
----------------
Can you return Result here and `return Default` at end?

================
Comment at: ELF/Driver.cpp:666
@@ +665,3 @@
+      getZOptionValue(Args, "max-page-size", Target->MaxPageSize);
+  if ((Config->MaxPageSize & (Config->MaxPageSize - 1)) != 0)
+    error("max-page-size: value isn't a power of 2");
----------------
We have `isPowerOf2_64` in MathExtras.h.

================
Comment at: ELF/Driver.cpp:672
@@ +671,3 @@
+    error("common-page-size: value isn't a power of 2");
+  Config->CommonPageSize = std::min(CommonPageSize, Config->MaxPageSize);
+
----------------
If there's no clear consensus how common-page-size should be interpreted, I'd vote for the simplest rule: just -z common-page-size's value to CommonPageSize.


https://reviews.llvm.org/D24891