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

[George Rimar via Phabricator via llvm-commits]

grimar 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);
----------------
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?


================
Comment at: lld/ELF/Driver.cpp:1937
+  if (config->emitAsm)
+    return;
+
----------------
That the last 2 comments have a large common part. Also it seems that if `thinLTOIndexOnly`, `emitLLVM` and `emitAsm` were added at once and not separatelly, we would not split them. I think it might be better to rewrite these 3 comments into a single one and do:

```
// <A shorter and nicer comment here>
if (config->thinLTOIndexOnly || config->emitLLVM || config->emitAsm)
  return;
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77231