migrate backends/vulkan/tools/gpuinfo (TARGETS+BUCK with similar rules) (#21432) - #21432
Conversation
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/21432
Note: Links to docs will display an error until the docs builds have been completed. ✅ You can merge normally! (1 Unrelated Failure)As of commit 3ebf385 with merge base d8c6be2 ( FLAKY - The following job failed but was likely due to flakiness present on trunk:
This comment was automatically generated by Dr. CI and updates every 15 minutes. |
|
@bigfootjon has exported this pull request. If you are a Meta employee, you can view the originating Diff in D109082051. |
This PR needs a
|
There was a problem hiding this comment.
Pull request overview
This PR continues the internal TARGETS→BUCK migration pattern by moving target definitions into targets.bzl (via define_common_targets(is_fbcode=...)) and wiring new/updated BUCK files to call into those shared definitions, with conditional branching to preserve fbcode vs xplat differences (notably for backends/vulkan/tools/gpuinfo).
Changes:
- Introduce/extend
targets.bzlentrypoints that define targets conditionally based onis_fbcode. - Replace several fbcode-only
TARGETSfiles withBUCKfiles that calldefine_common_targets(is_fbcode = is_fbcode()). - Wrap the MediaTek runtime library in
non_fbcode_target(...)inbackends/mediatek/runtime/BUCK.
Reviewed changes
Copilot reviewed 31 out of 32 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| extension/pytree/test/targets.bzl | New shared target definitions for pytree tests (fbcode-gated). |
| extension/pytree/test/TARGETS | Removed legacy TARGETS definitions (migrated). |
| extension/pytree/test/BUCK | New BUCK entrypoint calling define_common_targets(is_fbcode=...). |
| examples/models/llama/config/targets.bzl | Gate existing test target definitions on is_fbcode to preserve prior TARGETS-only behavior. |
| examples/models/llama/config/TARGETS | Removed legacy TARGETS wrapper (migrated). |
| examples/models/llama/config/BUCK | New BUCK entrypoint calling define_common_targets(is_fbcode=...). |
| backends/vulkan/tools/gpuinfo/targets.bzl | Consolidates fbcode/xplat gpuinfo build rules with an is_fbcode branch. |
| backends/vulkan/tools/gpuinfo/TARGETS | Removed legacy TARGETS definitions (migrated). |
| backends/vulkan/tools/gpuinfo/BUCK | Reduced to BUCK shim that calls into targets.bzl. |
| backends/vulkan/test/targets.bzl | Consolidates Vulkan test targets; fbcode branch defines Python tests, xplat branch defines shader filegroup + compute API tests. |
| backends/vulkan/test/TARGETS | Removed legacy TARGETS definitions (migrated). |
| backends/vulkan/test/BUCK | Reduced to BUCK shim that calls into targets.bzl. |
| backends/mediatek/runtime/BUCK | Wraps the runtime cxx_library with non_fbcode_target for migration compatibility. |
| backends/cuda/tests/targets.bzl | New fbcode-gated CUDA test target definitions. |
| backends/cuda/tests/TARGETS | Removed legacy TARGETS definitions (migrated). |
| backends/cuda/tests/BUCK | New BUCK entrypoint calling define_common_targets(is_fbcode=...). |
| backends/cuda/runtime/targets.bzl | New fbcode-gated CUDA runtime target definitions (libraries + C++ unit tests). |
| backends/cuda/runtime/TARGETS | Removed legacy TARGETS definitions (migrated). |
| backends/cuda/runtime/BUCK | New BUCK entrypoint calling define_common_targets(is_fbcode=...). |
| backends/cuda/runtime/shims/tests/targets.bzl | Updates shared shim test definitions to accept is_fbcode and early-return outside fbcode. |
| backends/cuda/runtime/shims/tests/TARGETS | Removed legacy TARGETS definitions (migrated). |
| backends/cuda/runtime/shims/tests/BUCK | New BUCK entrypoint calling define_common_targets(is_fbcode=...). |
| backends/apple/coreml/targets.bzl | Consolidates CoreML fbcode Python targets and xplat Apple C++ targets under one define_common_targets with branching. |
| backends/apple/coreml/TARGETS | Removed legacy TARGETS definitions (migrated). |
| backends/apple/coreml/BUCK | Reduced to BUCK shim that calls into targets.bzl. |
| backends/aoti/tests/targets.bzl | New fbcode-gated AOTI test target definitions. |
| backends/aoti/tests/TARGETS | Removed legacy TARGETS definitions (migrated). |
| backends/aoti/tests/BUCK | New BUCK entrypoint calling define_common_targets(is_fbcode=...). |
| backends/aoti/slim/factory/test/targets.bzl | Updates SlimTensor factory tests to accept is_fbcode and early-return outside fbcode. |
| backends/aoti/slim/factory/test/TARGETS | Removed legacy TARGETS definitions (migrated). |
| backends/aoti/slim/factory/test/BUCK | New BUCK entrypoint calling define_common_targets(is_fbcode=...). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # Any targets that should be shared between fbcode and xplat must be defined in | ||
| # targets.bzl. | ||
|
|
ndmitchell
left a comment
There was a problem hiding this comment.
Review automatically exported from Phabricator review in Meta.
Summary: TARGETS contained fbcode-only Python rules (placeholder backend imports); BUCK contained xplat-only Apple platform cxx_library targets. Created a new targets.bzl with define_common_targets(is_fbcode = False) whose body splits via if/else: fbcode branch holds the Python rules, xplat branch holds the Apple cxx targets. Replaced BUCK with thin caller, deleted TARGETS. Differential Revision: D109082057
Summary: The TARGETS file was 0 bytes; BUCK defines the actual neuron_backend cxx_library targeting Android. fbcode now falls through to BUCK with no behavior change. Differential Revision: D109082047
Summary: TARGETS had fbcode-only python_unittest rules; BUCK had xplat fb_native filegroup + define_compute_api_test_targets() call. Combined into a new targets.bzl with if/else split on is_fbcode. Differential Revision: D109082043
…s) (pytorch#21432) Summary: Both files defined the same target names with cell-specific differences (TARGETS: is_fbcode=True for vulkan_spv_shader_lib, no compiler_flags; BUCK: extra arvr opencl + folly:json deps, compiler_flags select). Combined into a new targets.bzl that branches on is_fbcode for the fb_xplat_cxx_binary, with the buck_filegroup and vulkan_spv_shader_lib shared. Reviewed By: ndmitchell Differential Revision: D109082051
…s) (pytorch#21432) Summary: Both files defined the same target names with cell-specific differences (TARGETS: is_fbcode=True for vulkan_spv_shader_lib, no compiler_flags; BUCK: extra arvr opencl + folly:json deps, compiler_flags select). Combined into a new targets.bzl that branches on is_fbcode for the fb_xplat_cxx_binary, with the buck_filegroup and vulkan_spv_shader_lib shared. Reviewed By: ndmitchell Differential Revision: D109082051
787286b to
e29e2c3
Compare
…s) (pytorch#21432) Summary: Pull Request resolved: pytorch#21432 Both files defined the same target names with cell-specific differences (TARGETS: is_fbcode=True for vulkan_spv_shader_lib, no compiler_flags; BUCK: extra arvr opencl + folly:json deps, compiler_flags select). Combined into a new targets.bzl that branches on is_fbcode for the fb_xplat_cxx_binary, with the buck_filegroup and vulkan_spv_shader_lib shared. Reviewed By: ndmitchell Differential Revision: D109082051
e29e2c3 to
3ebf385
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 11 changed files in this pull request and generated no new comments.
Suppressed comments (4)
backends/vulkan/tools/gpuinfo/BUCK:2
- The PR title/description focuses on migrating
backends/vulkan/tools/gpuinfo, but this change set also migratesbackends/vulkan/testandbackends/apple/coreml, and updatesbackends/mediatek/runtime/BUCK. Please update the PR description to mention the additional directories so reviewers can assess scope and risk appropriately.
# Any targets that should be shared between fbcode and xplat must be defined in
# targets.bzl.
backends/apple/coreml/targets.bzl:17
- The fbcode branch header comment is now inside targets.bzl itself and is a bit misleading (it reads like this file is fbcode-only) and leaves multiple consecutive blank lines. This can confuse future migrations and makes the file harder to scan.
# Any targets that should be shared between fbcode and xplat must be defined in
# targets.bzl. This file can contain fbcode-only targets.
backends/vulkan/tools/gpuinfo/targets.bzl:30
fb_xplat_cxx_binary(name = "vulkan_gpuinfo", ...)is duplicated across the fbcode and xplat branches with onlycompiler_flagsanddepsdiffering. In this consolidated file, consider building a sharedkwargsdict (or shared srcs/headers variables) and applying conditional additions, to reduce the chance of the two branches diverging over time.
if is_fbcode:
fb_xplat_cxx_binary(
name = "vulkan_gpuinfo",
srcs = native.glob([
"**/*.cpp",
backends/apple/coreml/targets.bzl:151
coreml_backend_optionsandcoreml_backend_options_testare defined in both the fbcode and xplat branches with (apparently) identical rule args. Keeping duplicate definitions in a single file increases the chance they drift; consider defining these once outside theis_fbcodeconditional so both environments share the same definition.
# Header-only library for CoreML backend options
runtime.cxx_library(
name = "coreml_backend_options",
exported_headers = [
"runtime/include/coreml_backend/coreml_backend_options.h",
Summary:
Both files defined the same target names with cell-specific differences
(TARGETS: is_fbcode=True for vulkan_spv_shader_lib, no compiler_flags;
BUCK: extra arvr opencl + folly:json deps, compiler_flags select).
Combined into a new targets.bzl that branches on is_fbcode for the
fb_xplat_cxx_binary, with the buck_filegroup and vulkan_spv_shader_lib
shared.
Reviewed By: ndmitchell
Differential Revision: D109082051