[PATCH] D121727: [NVPTX] Integrate ptxas to LIT tests

Thu Apr 28 07:11:48 PDT 2022

asavonic added a comment.

In D121727#3479832 <https://reviews.llvm.org/D121727#3479832>, @thakis wrote:

> It looks very weird to me to have LLVM's test suite support calling some proprietary tool. I can see how this is useful, but imho it doesn't belong into LLVM itself.
>
> This is both for practical and other reasons.
>
> From a practical point of view, if we rely on this tool for testing in LLVM, it means we can't run LLVM's tests on platforms that that tool doesn't run on.

It is weird and inconvenient to have a proprietary tool that is used for testing, I agree. This is why this feature is completely optional, and mainly intended to be used by buildbots or by people who contribute to NVPTX regularly.
If I understand correctly, PTX is only defined by the ISA manual, which is sometimes incomplete or unclear, so `ptxas` is used as a reference implementation. In fact, requests to test code generated by llc with ptxas (such as [1]) were frequent enough (and helpful as well) for me to justify making this patch set. Before this integration I had to dump llc output, run ptxas with correct options for each test, and do this for every test that I could break.

[1]: https://reviews.llvm.org/D114367#3147361

> Independent of practical considerations, LLVM tries to be an open-source, standalone toolchain development suite. Making it depend on proprietary programs, even just optionally and at the test level, works against both these goals.

I personally wouldn't call it a dependency. If there is a free software tool that can compile PTX assembly and verify it (I'm not aware of any) - it should be fairly easy to support it as well.
In any case, ptxas is just a tool used for development. It is the same story as with static analyzers: yes, many of them are proprietary, but if some people are willing to run them -  they can help to make free software better.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121727