[PATCH] D77231: [lld] Support -emit-asm with LTO

[Teresa Johnson via Phabricator via llvm-commits]

tejohnson added inline comments.


================
Comment at: lld/ELF/Driver.cpp:890
   config->emitLLVM = args.hasArg(OPT_plugin_opt_emit_llvm, false);
+  config->emitAsm = args.hasArg(OPT_lto_emit_asm, false);
   config->emitRelocs = args.hasArg(OPT_emit_relocs);
----------------
MaskRay wrote:
> hoyFB wrote:
> > hoyFB wrote:
> > > grimar wrote:
> > > > It looks inconsistent with another `OPT_lto*` options we have:
> > > > 
> > > > ```
> > > >   config->ltoAAPipeline = args.getLastArgValue(OPT_lto_aa_pipeline);
> > > >   config->ltoCSProfileGenerate = args.hasArg(OPT_lto_cs_profile_generate);
> > > >   config->ltoCSProfileFile = args.getLastArgValue(OPT_lto_cs_profile_file);
> > > > ...
> > > > ```
> > > > 
> > > > Should it be called `config->ltoEmitAsm` to be consistent?
> > > > 
> > > > Or, instead, it could be `args.hasArg(OPT_plugin_opt_emit_asm, false)`, what also would make sense it seems.
> > > > 
> > > > What do other reviewers think?
> > > Naming it ```config->ltoEmitAsm``` sounds good to me. If you look at the relationship between ```OPT_lto*``` and ```OPT_plugin_opt_*``` in lld, the later is always an alias of the former.
> > Well, on the second thought, I'm thinking maybe just removing the `--lto-emit-asm` switch and only having `--plugin-opt=emit-asm`, since it is not lld-exclusive. What do you think?
> `--plugin-opt=*` is for compatibility with gold. gold uses LLVMgold.so and the plugin interface does not allow insert arbitrary option on the fly. @pcc started to use `--lto-*` in D18667
> 
> Maybe he has some thoughts here.
For compatibility, please do add support for the plugin-opt version, even if it is just an alias for a new --lto-emit-asm.


================
Comment at: lld/ELF/LTO.cpp:312
+    for (unsigned i = 1; i != maxTasks; ++i)
+      saveBuffer(buf[i], config->outputFile + Twine(i));
+    return {};
----------------
hoyFB wrote:
> MaskRay wrote:
> > To be honest, the output file naming (--save-temps and the new emit-asm) here also looks strange to me. Why is 1 special cased? @pcc @tejohnson 
> My understanding is that output 0 has no suffix on the output file name, and suffix starts from output 1. Is that your question?
I think @pcc added that handling to gold-plugin, but I believe it is so there is no suffix when we are doing regular LTO where there is one task.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77231