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

[Teresa Johnson via Phabricator via llvm-commits]

tejohnson added inline comments.


================
Comment at: lld/test/ELF/lto/save-temps-eq.ll:19
+; RUN: ls %t.all2/a.out | count 1
+; RUN: mv %t/*.o.* %t.all2
+; RUN: for i in %t.all2/*; do diff "$i" %t.all/`basename "$i"` || exit 1; done
----------------
Probably want to test to ensure which temp files were emitted by each of these? Edit: I do see now how you are removing them one by one and check for an empty %t.all at the end, but I think it would be clearer to check which file is emitted more directly.

Also, can you test that the options are composable? I.e. that you can specify multiple and get the union of the requested temps files?


================
Comment at: lld/test/ELF/lto/save-temps-eq.ll:98
+
+; Check multi-stage
+; RUN: rm -rf %t.9
----------------
It's really hard to follow what the checking below is doing, with the various for loops and diffs. Can the output file checks be made explicit, similar to what I suggested above?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127778