[PATCH] D33525: [ThinLTO] Migrate ThinLTOBitcodeWriter to the new PM.

Chandler Carruth via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri May 26 13:59:26 PDT 2017


chandlerc added a comment.

In https://reviews.llvm.org/D33525#766050, @timshen wrote:

> In https://reviews.llvm.org/D33525#764251, @chandlerc wrote:
>
> > (focusing on the LLVM side of this review for now)
> >
> > Can you add an LLVM-based test? Can you add this to `lib/Passes/PassRegistry.def`?
>
>
> Talked offline. Given the fact that BitcodeWriter (and possibly assembly writer?) is not registered either, it seems to be a larger issue that's out of the scope of this patch.


While *generic* testing is out of scope, I think you need at least *some* testing of this in this patch. It can be an option directly in the `llvm-lto` tool or a direct option in the `opt` tool itself, but we shouldn't add a bunch of code to LLVM that can only ever be exercised by using some other tool.


https://reviews.llvm.org/D33525





More information about the llvm-commits mailing list