[ELF] Support -plugin-opt=stats-file=

[arichardson]

Summary

This flag is added by clang::driver::tools::addLTOOptions() and was causing
errors for me when building the llvm-test-suite repository with LTO and
-DTEST_SUITE_COLLECT_STATS=ON. This replaces the --stats-file= option
added in 1c04b52b2594d403f739ed919ef420b1e47ae343 since the flag is only
used for LTO and should therefore be in the -plugin-opt= namespace.

Additionally, this commit fixes the REQUIRES: asserts that was added in
948d05324a150a5a24e93bad07c9090d5b8bd129: the feature was never defined in
the lld test suite so it effectively disabled the test.

[MaskRay]

If this is for LTO usage, I think we should remove --stats-file= recently added from D121809 (@MTC) and keep this -plugin-opt=stats-file=

[arichardson]

In D124105#3462404, @MaskRay wrote:

If this is for LTO usage, I think we should remove --stats-file= recently added from D121809 (@MTC) and keep this -plugin-opt=stats-file=

Yes this is purely for LTO usage. I tried building the LLVM test suite with LTO+statistics and hit the unsupported flag error.

[MaskRay]

(1) Fix the typo in the subject (2) remove --stats-file= then this patch is good to go:)

[arichardson]

Replace --stats-file and make the test actually run

[MTC]

Appreciate your fix, LGTM. I didn't notice the subtle differences between the option prefixes. Please wait for the last review from @MaskRay before landing it.

[MaskRay]

LGTM.

[dyung]

@MTC @arichardson The test stats-file-option.ll that is now enabled after this commit seems to fail if X86 is not one of the built targets. Can the test be rewritten to now require that, or an appropriate REQUIRES line added?

[arichardson]

In D124105#3501748, @dyung wrote:

@MTC @arichardson The test stats-file-option.ll that is now enabled after this commit seems to fail if X86 is not one of the built targets. Can the test be rewritten to now require that, or an appropriate REQUIRES line added?

This is a missing # REQUIRES: x86. The test wasn't being run before due to the asserts feature not being valid. I will commit the missing requires first thing tomorrow morning unless someone else beats me to it.

lld/ELF/Driver.cpp

static void readConfigs(opt::InputArgList &args) {
   config->mmapOutputFile =
       args.hasFlag(OPT_mmap_output_file, OPT_no_mmap_output_file, true);
   config->nmagic = args.hasFlag(OPT_nmagic, OPT_no_nmagic, false);
   config->noinhibitExec = args.hasArg(OPT_noinhibit_exec);
   config->nostdlib = args.hasArg(OPT_nostdlib);
   config->oFormatBinary = isOutputFormatBinary(args);
   config->omagic = args.hasFlag(OPT_omagic, OPT_no_omagic, false);
   config->optRemarksFilename = args.getLastArgValue(OPT_opt_remarks_filename);
-  config->optStatsFilename = args.getLastArgValue(OPT_opt_stats_filename);
+  config->optStatsFilename = args.getLastArgValue(OPT_plugin_opt_stats_file);
 
   // Parse remarks hotness threshold. Valid value is either integer or 'auto'.
   if (auto *arg = args.getLastArg(OPT_opt_remarks_hotness_threshold)) {
     auto resultOrErr = remarks::parseHotnessThresholdOption(arg->getValue());
     if (!resultOrErr)
       error(arg->getSpelling() + ": invalid argument '" + arg->getValue() +
             "', only integer or 'auto' is supported");
     else

lld/ELF/Options.td

defm opt_remarks_hotness_threshold: EEq<"opt-remarks-hotness-threshold",
   " Use 'auto' to apply the threshold from profile summary.">,
   MetaVarName<"<value>">;
 def opt_remarks_passes: Separate<["--"], "opt-remarks-passes">,
   HelpText<"Regex for the passes that need to be serialized to the output file">;
 def opt_remarks_with_hotness: FF<"opt-remarks-with-hotness">,
   HelpText<"Include hotness information in the optimization remarks file">;
 def opt_remarks_format: Separate<["--"], "opt-remarks-format">,
   HelpText<"The format used for serializing remarks (default: YAML)">;
-def opt_stats_filename: JJ<"stats-file=">,
-  HelpText<"Filename to write statistics to">;
 def save_temps: F<"save-temps">, HelpText<"Save intermediate LTO compilation results">;
 def lto_basic_block_sections: JJ<"lto-basic-block-sections=">,
   HelpText<"Enable basic block sections for LTO">;
 defm lto_unique_basic_block_section_names: BB<"lto-unique-basic-block-section-names",
     "Give unique names to every basic block section for LTO",
     "Do not give unique names to every basic block section for LTO (default)">;
 defm shuffle_sections: EEq<"shuffle-sections",
   "Shuffle matched sections using the given seed before mapping them to the output sections. "
def: F<"plugin-opt=opt-remarks-with-hotness">,
   Alias<opt_remarks_with_hotness>,
   HelpText<"Alias for --opt-remarks-with_hotness">;
 def: J<"plugin-opt=opt-remarks-hotness-threshold=">,
   Alias<opt_remarks_hotness_threshold>,
   HelpText<"Alias for --opt-remarks-hotness-threshold">;
 def: J<"plugin-opt=sample-profile=">,
   Alias<lto_sample_profile>, HelpText<"Alias for --lto-sample-profile">;
 def: F<"plugin-opt=save-temps">, Alias<save_temps>, HelpText<"Alias for --save-temps">;
+def plugin_opt_stats_file: J<"plugin-opt=stats-file=">,
+  HelpText<"Filename to write LTO statistics to">;
 def: F<"plugin-opt=thinlto-emit-imports-files">,
   Alias<thinlto_emit_imports_files>,
   HelpText<"Alias for --thinlto-emit-imports-files">;
 def: F<"plugin-opt=thinlto-index-only">,
   Alias<thinlto_index_only>,
   HelpText<"Alias for --thinlto-index-only">;
 def: J<"plugin-opt=thinlto-index-only=">,
   Alias<thinlto_index_only_eq>,

lld/test/ELF/lto/stats-file-option.ll

 ; REQUIRES: asserts
 
 ; RUN: llvm-as -o %t.bc %s
 
 ;; Try to save statistics to file.
-; RUN: ld.lld --stats-file=%t2.stats -m elf_x86_64 -r -o %t.o %t.bc
+; RUN: ld.lld --plugin-opt=stats-file=%t2.stats -m elf_x86_64 -r -o %t.o %t.bc
 ; RUN: FileCheck --input-file=%t2.stats %s
 
 ; CHECK: {
 ; CHECK: "asm-printer.EmittedInsts":
 ; CHECK: "inline.NumInlined":
 ; CHECK: "prologepilog.NumFuncSeen":
 ; CHECK: }
 

lld/test/lit.cfg.py

[('--targets-built', {'AArch64': 'aarch64',
                           'AVR': 'avr',
                           'Hexagon': 'hexagon',
                           'Mips': 'mips',
                           'MSP430': 'msp430',
                           'PowerPC': 'ppc',
                           'RISCV': 'riscv',
                           'Sparc': 'sparc',
                           'WebAssembly': 'wasm',
-                          'X86': 'x86'})
+                          'X86': 'x86'}),
+     ('--assertion-mode', {'ON': 'asserts'}),
      ])
 
 # Set a fake constant version so that we get consistent output.
 config.environment['LLD_VERSION'] = 'LLD 1.0'
 
 # LLD_IN_TEST determines how many times `main` is run inside each process, which
 # lets us test that it's cleaning up after itself and resetting global state
 # correctly (which is important for usage as a library).