Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add SPIR-V backend - SPIR-V Translator cross validation #2569

Open
MrSidims opened this issue May 17, 2024 · 4 comments
Open

Add SPIR-V backend - SPIR-V Translator cross validation #2569

MrSidims opened this issue May 17, 2024 · 4 comments

Comments

@MrSidims
Copy link
Contributor

MrSidims commented May 17, 2024

In this repository we have .cl tests which are being compiled for spir target emitting LLVM IR, that will be translated to SPIR-V and backwards. The suggestion is to compile them for spirv target as well using SPIR-V backend and then translate it back to LLVM IR by the translator. It might be not in the spirit of unit tests for a single project, but we already have an external dependency on clang here and rely on its output, modifying our expectations from time to time depending on the changes there. Meanwhile such cross validation would add sanity checks improving quality of the both projects. WDYT?

@MrSidims
Copy link
Contributor Author

@VyacheslavLevytskyy @svenvh @asudarsa please express your opinion about that

@VyacheslavLevytskyy
Copy link
Contributor

I like both this way of thinking in general, and this suggestion in particular. SPIR-V Backend is a client of SPIRV-LLVM-Translator in a sense of reverse translation from SPIR-V to LLVM IR, so such step would help to define some guarantees.

I'd expect a caveat at least in that SPIR-V Backend is an experimental backend at the moment, but this can be addressed with additional checks of the config in a style of ; RUN: %if spirv-backend %{ llc ... }, and anyway current plans is to make SPIR-V Backend non-experimental sooner than later.

@asudarsa
Copy link
Contributor

Thanks @MrSidims. I am fully in support of this idea. There are two ways in which we can accomplish this. (1) Run llc/clang explicitly in the tests. or (2) Modify translator code to call llc (under an option) instead of spir-v writer logic. We are already looking at option 2, so we can complete that. I am ok with either approach though.
Adding to caveats, one thing is we will need to start building LLVM with SPIR-V backend. This is not a big overhead though.

@svenvh
Copy link
Member

svenvh commented May 20, 2024

I support the idea, great to see the SPIR-V backend progress!

Some thoughts on how we go about implementing it:

  • We should be careful not to skip the .cl tests when the SPIR-V backend isn't available. Adding a REQUIRES: spirv-backend would cause the entire test to be skipped for example, which is not good as we'd still want to run the llvm-spirv parts of the test. Using the %if construct that @VyacheslavLevytskyy mentioned above should work. But that leads to my next point:
  • Ideally we avoid adding lots of new logic in the test RUN lines which then needs to be repeated across all test files. Maybe we could add a script so the test files only need an extra line like ; RUN: %if spirv-backend %{ run-spirv-backend-and-validate.sh %s }, and we standardize the tests on FileCheck prefixes etc. (in hindsight, we should probably have used a similar approach for e.g. the transcoding tests, but that's beyond the scope of this task)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants