[PATCH] D153559: [libLTO] Add support for -save-temps

[Teresa Johnson via Phabricator via llvm-commits]

tejohnson added a comment.

It would be better to work with @fhahn on the old patch (you can commandeer it if he is ok with that), so that comment history is preserved. But I have some comments on this version that I will add here.  Looks like a test is still needed (this patch affects legacy regular LTO, the prior llvm-lto tests would have only used the existing option for ThinLTO - see thinlto-save-temps flag in llvm-lto.cpp). I'll leave a comment on the original version, I think it makes sense to common up the options used for this between thin and regular LTO in the legacy LTO API and its tool, llvm-lto.



================
Comment at: llvm/tools/llvm-lto2/llvm-lto2.cpp:67
+namespace llvm {
+extern cl::opt<bool> SaveTemps;
+}
----------------
llvm-lto2.cpp tests the new LTO API, so it is a bit odd for it to use a flag from the legacy LTO API.

I'm not sure it makes sense to common the flag between these 2 different interfaces and their corresponding tools.


================
Comment at: llvm/tools/lto/lto.cpp:36
+namespace llvm {
+extern cl::opt<bool> SaveTemps;
+}
----------------
Unused in this file


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153559