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

Fangrui Song via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Jun 30 11:45:52 PDT 2022


MaskRay accepted this revision.
MaskRay added a comment.
This revision is now accepted and ready to land.

`\opt` caused a Windows failure: https://buildkite.com/llvm-project/premerge-checks/builds/100428#0181b59c-9093-413f-bee2-39f2a6364181

Use `--save-temps=opt`

POSIX.1-2008 says:

> A <backslash> that is not quoted shall preserve the literal value of the following character, with the exception of a <newline>.

therefore `\opt` is interpreted as `opt`. But Windows doesn't seem to do this.

Please fix them.



================
Comment at: lld/ELF/Driver.cpp:1167
+    // --save-temps implies saving all temps.
+    for (auto *s : saveTempsValues)
+      config->saveTempsArgs.insert(s);
----------------
See https://llvm.org/docs/CodingStandards.html#use-auto-type-deduction-to-make-code-more-readable


================
Comment at: lld/test/ELF/lto/save-temps-eq.ll:18
+;; Check precedence if both --save-temps and --save-temps= are present
+; RUN: ld.lld main.o thin1.o --save-temps=preopt --save-temps --save-temps=\opt -o %t/all2/a.out
+; RUN: cmp %t/all2/a.out %t/all/a.out
----------------
`\opt` caused a Windows failure: https://buildkite.com/llvm-project/premerge-checks/builds/100428#0181b59c-9093-413f-bee2-39f2a6364181

`--save-temps=opt`

POSIX.1-2008 says:

`A <backslash> that is not quoted shall preserve the literal value of the following character, with the exception of a <newline>.`

therefore `\opt` is interpreted as `opt`. But Windows doesn't seem to do this.


================
Comment at: lld/test/ELF/lto/save-temps-eq.ll:60
+;; Check opt
+; RUN: ld.lld main.o thin1.o --save-temps=\opt
+; RUN: cmp %t/all/a.out a.out && rm -f a.out
----------------



================
Comment at: llvm/test/ThinLTO/X86/selective-save-temps.ll:1
+;; This test is similar to lld/test/ELF/lto/save-temps-eq.ll
+
----------------
This comment may be inverse (due to layering). The lld/ELF test can say that it is similar to this file.


================
Comment at: llvm/tools/llvm-lto2/llvm-lto2.cpp:277
+    return 1;
+  } else if (SaveTemps || !SelectSaveTemps.empty()) {
+    DenseSet<StringRef> SaveTempsArgs;
----------------
remove `else` since the early return pattern is used.


================
Comment at: llvm/tools/llvm-lto2/llvm-lto2.cpp:283
+      else {
+        llvm::errs() << "invalid -select-save-temps argument: " + S << '\n';
+        return 1;
----------------



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