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

Wed Mar 16 07:58:13 PDT 2022

asavonic added a comment.

In D121727#3383955 <https://reviews.llvm.org/D121727#3383955>, @tra wrote:

> In D121727#3383523 <https://reviews.llvm.org/D121727#3383523>, @asavonic wrote:
>
>> We should probably have both: substitution to `true` works for *all* existing tests where can use ptxas as a sanity check, and a feature allows us to write tests for machine code.
>
> Wouldn't it be redundant? If `REQUIRES: ptxas` is satisfied, there's no need for using `true` as a substitute. If it's not satisfied, then we would not run such test and therefore `true` would not be useful either.
>
> Using `ptxas` in a test w/o `REQUIRES: ptxas` would be a user error, IMO as the test would be relying on something that's not expected to be available by default and would not do anything useful.
> I would actually make this assertion even stronger -- allowing a test to run and return a success when we in fact didn't test anything would be a wrong thing to do. We should correctly report such test as not executed.

`ptxas` in a test without `REQUIRES: ptxas` can still work as an optional verification step, similar to `opt -verify` or `llc -verify-machineinstrs`. 
Yes, if `ptxas` is not available then these checks will do nothing, but it should be fine as long as there is a way to enable them. We can also setup a buildbot to run with `ptxas`.
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.

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.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121727