Skip to content

Add minimal as/dis support for TOSA.001000.1 extended instruction set#6183

Merged
dneto0 merged 8 commits intoKhronosGroup:mainfrom
kpet:tosa-spirv
Jun 19, 2025
Merged

Add minimal as/dis support for TOSA.001000.1 extended instruction set#6183
dneto0 merged 8 commits intoKhronosGroup:mainfrom
kpet:tosa-spirv

Conversation

@kpet
Copy link
Copy Markdown
Contributor

@kpet kpet commented Jun 18, 2025

Change-Id: Ieefcd9c9a46dc3021c1600c02f9be96c49641aa9

Change-Id: Ieefcd9c9a46dc3021c1600c02f9be96c49641aa9
Signed-off-by: Kevin Petit <kevin.petit@arm.com>
@kpet
Copy link
Copy Markdown
Contributor Author

kpet commented Jun 18, 2025

Headers: KhronosGroup/SPIRV-Headers#526

kpet added 2 commits June 18, 2025 16:48
Change-Id: Ie0d1c5258367774451373f7d81e231e2fc674937
Change-Id: I34fbf54545ecb58a3bacdf1f711e22547e8b5666
@kpet
Copy link
Copy Markdown
Contributor Author

kpet commented Jun 18, 2025

So, it looks like SPIRV-Tools has its own logic to regenerate the SPIR-V headers when building with Bazel. I am not sure why but, assuming this will not change, it looks like I'm going to need Bazel definitions for the new instruction set added to the headers (KhronosGroup/SPIRV-Headers#527), a few changes to add the new instruction set to BUILD.bazel, and either changes to the utils/generate_language_headers.py script in this repo or its removal in favour of the more recent copy in SPIRV-Headers (see #6186).

What direction do maintainers prefer?

@dneto0
Copy link
Copy Markdown
Collaborator

dneto0 commented Jun 18, 2025

So, it looks like SPIRV-Tools has its own logic to regenerate the SPIR-V headers when building with Bazel. I am not sure why but, assuming this will not change, it looks like I'm going to need Bazel definitions for the new instruction set added to the headers (KhronosGroup/SPIRV-Headers#527), a few changes to add the new instruction set to BUILD.bazel, and either changes to the utils/generate_language_headers.py script in this repo or its removal in favour of the more recent copy in SPIRV-Headers (see #6186).

What direction do maintainers prefer?

Sorry for the confusion:

The error is that the bazel build can't find the TOSA header from spirv-headers:

test/ext_inst.tosa_test.cpp:19:10: fatal error: spirv/unified1/TOSA.001000.1.h: No such file or directory
   19 | #include "spirv/unified1/TOSA.001000.1.h"
      |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

The fix is to add an entry for include/spirv/unified1/TOSA.001000.1.h to the list here

That's one flow. It does not (directly) use the generate_language_headers.py script in either repo. It uses the checked-in .h file.

The Android.mk, the BUILD.gn flow (chromium) use utils/generate_language_headers.py script in SPIRV-Tools.

  • I see now that in the BUILD.gn case it looks redundant. Update the list here and I think it will work without needing to update the .py script.
  • For Android.mk I think it ended up working in bots because it does not use a hermetic build, and so it picked up the checked-in TOSA header without needing to generate it locally.

I think this implies I can do a bunch of cleanups.

To get this PR to pass, we only need the update to BUILD.bazel in SPIRV-Headers. I'll make a PR for that.

@dneto0
Copy link
Copy Markdown
Collaborator

dneto0 commented Jun 18, 2025

I posted KhronosGroup/SPIRV-Headers#529

With that, this PR builds on Bazel (tested on Linux).

kpet added 2 commits June 19, 2025 08:06
This reverts commit d4496b7.
Change-Id: I368598d93f94bc39ff71db56269bf1baeedf37c8
@kpet
Copy link
Copy Markdown
Contributor Author

kpet commented Jun 19, 2025

Thanks for the help with this PR David, much appreciated! I've approved and merged your SPIRV-Headers PR and picked it up here. This enabled progress but I'm still hitting a few issues. Going through the CI failures:

  • CI-ubuntu-gcc-release/CI-ubuntu-gcc-debug/CI-ubuntu-clang-ubsan: seems CMake wasn't able to find re2 or protobuf. CI hiccup?
  • CI-macos-clang-release-bazel/CI-ubuntu-clang-release-bazel: both fail with the following error
test/ext_inst.tosa_test.cpp:19:10: error: module //:base_ext_inst.tosa_test does not depend on a module exporting 'spirv/unified1/TOSA.001000.1.h'
#include "spirv/unified1/TOSA.001000.1.h"

Looks like I'm missing a dependency in BUILD.bazel. I should be able to figure that one out.

  • Github Actions Build and Test with Bazel: built correctly but the test I added is failing (and passing in other jobs):
 error: 2: 22: Invalid extended instruction name 'ARGMAX'.

It looks like the instruction set is not included in tables in Bazel builds.

kpet added 3 commits June 19, 2025 09:21
Change-Id: I265b9622ce86f3d86b68a523ccf849a0134b890b
Change-Id: Ifa2a94af00af09fbd5aa1ae48eeac0603aa5b5f0
Change-Id: I411737a0544187c0c01d9628df9a7b556490750e
@kpet
Copy link
Copy Markdown
Contributor Author

kpet commented Jun 19, 2025

So Bazel jobs are now passing but there are still a couple of failures that I don't think have anything to do with this PR:

  • CI-macos-clang-release Clang crashing on a file that I did not change :(
  • CI-ubuntu-gcc-debug CMake can't find abseil_cpp. Looks unrelated to this PR.

@dneto0
Copy link
Copy Markdown
Collaborator

dneto0 commented Jun 19, 2025

  • CI-ubuntu-gcc-debug: fatal: unable to access 'https://github.com/abseil/abseil-cpp.git/': Could not resolve host: github.com I should be able to trigger another attempt internally.
  • CI-macos-clang-release: clang: error: unable to execute command: Segmentation fault: 11 . Again likely transient. I'll trigger again.

@dneto0
Copy link
Copy Markdown
Collaborator

dneto0 commented Jun 19, 2025

The changes to BUILD.gn affect the GN build for Chromium. Unfortunately that build is not tested in this repo, but only when we (Chrome) tries to roll it into Chromium. I'll try to get this landed, then reapply the BUILD.gn changes soon after.

@dneto0 dneto0 enabled auto-merge (squash) June 19, 2025 14:03
@dneto0 dneto0 merged commit 3f76afc into KhronosGroup:main Jun 19, 2025
23 checks passed
@kpet kpet deleted the tosa-spirv branch June 19, 2025 17:36
@kpet
Copy link
Copy Markdown
Contributor Author

kpet commented Jun 19, 2025

Thank you!

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

Successfully merging this pull request may close these issues.

2 participants