Opinions on conditional dependencies on proprietary tools

So on the one hand, I favor a “live and let live” approach on stuff that is isolated to individual targets.

That said, having looked at the review in question (https://reviews.llvm.org/D121727), it seems to me a not particularly useful change in the first place. As was pointed out, the point of test cases is to verify that the compiler produces the expected output. Piping the output to an assembler and then throwing away the output of that assembler (except for a single bit of information) is almost entirely pointless. Building in support for a propietary and external tool just for that sets a bad precedent IMHO.

If the goal is to improve test coverage of the NVPTX backend, it would be better to change the test cases so that they actually test the compiler’s output. You don’t need ptxas for that. A better approach would be to run the test cases through update_llc_test_checks.py.

In this case we also want to verify that the produced output is valid, which is not always the same as ‘expected’.

The problem is that developer’s idea of the expected output does not always match the reality and the point of this seemingly pointless test is to find such cases.

We do test compiler output. The problem is that we’re testing vs our expectations of the correct output, not the reality as defined by ptxas.

If/when ptxas catches a problem it indicates that there’s both a problem in LLVM and in the tests that either didn’t check for something, or incorrectly considered such produced output valid. Increased coverage is a nice bonus, but the primary purpose is to detect cases where our idea of ‘correct’ is wrong. It’s not a hypothetical problem. This mismatch between the expectations and the reality has happened on multiple occasions.

Off the top of my head, we ran into this problem fairly regularly:

  • on multiple occasions produced symbols with characters PTX does not accept
  • produced debug info in a way ptxas considered invalid (wrong label/section location)
  • ran into ptxas bugs when it could not parse DWARF generated by LLVM
  • generated instructions with types not accepted by ptxas
  • generated address labels in the wrong location.
  • produced assembly directives not accepted by ptxas.
  • generated invalid instruction variants.

Some of the problems were not checked for by regular tests.
Some were checked, but incompletely (nobody expected issues to pop up where they did)
Others were checked and were correct, as far as the developer was concerned, yet produced code was not useful in practice because ptxas disagreed, due to bugs or disagreement with the documentation used to implement new instructions.

In each of these cases, why wasn’t a FileCheck test added as a regular test? That would be the expected and common way of handling this.

I agree that we should strive to extend test coverage with FileCheck tests usable by everyone. We did add appropriate tests after we became aware of the problem and knew what to check for.

As I said:

In other words – it’s hard to pattern-match something we don’t know yet.

Running ptxas helps us gain the missing knowledge about LLVM generating something it should not have and about the tests which erroneously allowed something invalid to pass.