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

[Artem Belevich via Phabricator via llvm-commits]

tra added a subscriber: echristo.
tra added a comment.

In D121727#3386172 <https://reviews.llvm.org/D121727#3386172>, @asavonic wrote:

> The advantage here is that we can add these checks to all existing LIT tests and make sure that they indeed produce valid PTX code.

Without ptxas available those tests would not do anything useful, would they? Why not just use `REQUIRES: ptxas` and avoid adding this special-case substitution of ptxas with `true`.
I really don't see it buying us anything. There are no existing ptxas tests and adding `REQUIRES: ptxas` to the new ones sounds like the right thing to do.

> For tests that require `ptxas` to function - `REQUIRES: ptxas` is definitely needed.



> If we want to distinguish these two cases, then we can have two substitutions: `ptxas-verify` that expands to `ptxas -c -o /dev/null` (or to `true` if `ptxas` is not available), and just `ptxas` that requires the corresponding feature.

I'm not convinced we need those two cases. I would prefer to keep things simple. If there's no ptxas available -> don't run ptxas tests.
As a user I would be unpoleasantly surprised to see a test supposedly launching ptxas passing in my sandbox (because it actually happened ran 'true', without my knowledge), yet LLVM producing invalid PTX.
I would much rather know upfront that PTX has not been tested with ptxas, rather than risking a false positive if ptxas was not enabled.

@echristo - WDYT? Do we pretend to run any other external tools that we use in tests if they are not available?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121727