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

[Jin Xin Ng via Phabricator via llvm-commits]

Northbadge added a comment.

In D127778#3608725 <https://reviews.llvm.org/D127778#3608725>, @tejohnson wrote:

> In D127778#3603536 <https://reviews.llvm.org/D127778#3603536>, @MaskRay wrote:
>
>> From a layering perspective, the feature is better tested on the llvm/ project, not lld/. You may need to change llvm-lto2 to check the values. Having tests in lld/test/ELF is fine too as that reflects how people use the functionality, but they should not be replacement for llvm/ side tests.
>> (llvm/test/ThinLTO/X86 and llvm/test/LTO/X86)
>
> This is a good point, and for D127779 <https://reviews.llvm.org/D127779> as well.

I defer to the both of you on that point. However, this patch and D127779 <https://reviews.llvm.org/D127779> are more of an extension of `--save-temps` (in my opinion), and as such the tests are predicated on `--save-temps` being correct. I just assumed `--save-temps` was properly tested to begin with, but it seems to have only been tested within `ELF/lto` on a basic level. Would it be sufficient to write llvm side tests for `--save-temps` as a separate patch?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127778