[PATCH] D49366: [LLD][COFF] Add support for /FUNCTIONPADMIN command-line option

[Rui Ueyama via Phabricator via llvm-commits]

ruiu accepted this revision.
ruiu added a comment.

LGTM



================
Comment at: COFF/Driver.cpp:1387-1390
+  // Handle /functionpadmin
+  for (auto *Arg : Args.filtered(OPT_functionpadmin))
+    parseFunctionPadMin(Arg->getValue(), Config->Machine);
+
----------------
aganea wrote:
> ruiu wrote:
> > This place does not seem like a right place to add this code. We parse command line arguments much earlier.
> We need `Config->Machine` to be set before parsing `/FUNCTIONPADMIN`. In some cases this is extracted from inputs, `run()` a few lines above.
Ah, you are right. Thank you for clarifying.


================
Comment at: COFF/Options.td:35
 def failifmismatch : P<"failifmismatch", "">;
+def functionpadmin : Joined<["/", "-", "-?"], "functionpadmin">, HelpText<"Prepares an image for hotpatching">;
 def guard   : P<"guard", "Control flow guard">;
----------------
ruiu wrote:
> Newline before `HelpText` just like other definitions.
You can use `P` just like above, can't you?


Repository:
  rLLD LLVM Linker

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D49366/new/

https://reviews.llvm.org/D49366