[PATCH] D127778: [LTO][ELF] Add selective --save-temps= option

Fangrui Song via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Jun 22 17:16:36 PDT 2022


MaskRay added inline comments.


================
Comment at: lld/ELF/Driver.cpp:1163
+
+  // --save-temps takes precedence over --save-temps=
+  if (!config->saveTemps) {
----------------



================
Comment at: lld/ELF/Driver.cpp:1164
+  // --save-temps takes precedence over --save-temps=
+  if (!config->saveTemps) {
+    // parse --save-temps=
----------------
Since there are only very few allowed values, SmallVector<StringRef, 0> may work better.
You may deduplicate the values in lld/ELF/Driver.cpp

How about this: remove `bool config->saveTemps`. If `--save-temps` is specified, populate the array with all allowed values.


================
Comment at: lld/ELF/Driver.cpp:1165
+  if (!config->saveTemps) {
+    // parse --save-temps=
+    for (auto &s : args.getAllArgValues(OPT_save_temps_eq))
----------------
The comment is unnecessary.


================
Comment at: lld/test/ELF/lto/save-temps-eq.ll:9
+;; with the output from individualized save-temps later
+; RUN: rm -rf %t.all
+; RUN: mkdir %t.all
----------------
With `mkdir %t && cd %t`, all other `%t` references can be removed.
If you want to test a subdirectory behavior, create a directory without the `%t` prefix.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127778



More information about the llvm-commits mailing list