[PATCH] D61711: [ELF][Driver] Fix precedence of symbol ordering file and CGProfile

Thu May 16 20:03:55 PDT 2019

tcwang marked an inline comment as done.
tcwang added inline comments.

================
Comment at: lld/ELF/Driver.cpp:968-970
+    if (Args.hasArg(OPT_call_graph_ordering_file))
+      error("--symbol-ordering-file should NOT be used together"
+            " with --call-graph-order-file");
----------------
ruiu wrote:
> tcwang wrote:
> > ruiu wrote:
> > > Check for invalid combination of flags are usually done in `checkOptions()`. Can you write a check for this combination there?
> > checkOptions() function doesn't take any arguments in but I need Args to check if OPT_call_graph_ordering_file is present or not. It's because the flag is read very late in the stage (just before writeResult<ELFT>() ) and not saved to any Config field, I have to use Args to directly check its presence. Whereas in checkOptions(), all other checkings are done by checking Config fields. Also, if I do check in checkOptions(), which happens after readConfigs(), it needs to read in the file even if an error will be reported later?
> OK, but please make the wording consistent with other error messages. If you look at `checkOptions`, you'd notice that we use the same wording for reporting incompatible option combinations.
Yeah, that makes sense. Thanks for pointing out.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61711