Skip to content

Bundle specific and generic arch build targets together. #780

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.

Sign up for GitHub

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

Open
tsrw2048 wants to merge 1 commit into
base: rocm-jaxlib-v0.10.0
Choose a base branch
from
Open

Bundle specific and generic arch build targets together. #780

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion build/rocm/rocm.bazelrc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ common:rocm --action_env=CLANG_COMPILER_PATH="/usr/lib/llvm-18/bin/clang"
common:rocm --action_env=HIPCC_COMPILE_FLAGS_APPEND=--offload-compress
common:rocm --copt=-Wno-gnu-offsetof-extensions
common:rocm --copt=-Qunused-arguments
common:rocm --repo_env=TF_ROCM_AMDGPU_TARGETS="gfx908,gfx90a,gfx942,gfx950,gfx1030,gfx1100,gfx1101,gfx1200,gfx1201"
common:rocm --repo_env=TF_ROCM_AMDGPU_TARGETS="gfx908,gfx90a,gfx942,gfx950,gfx1030,gfx1100,gfx1101,gfx1200,gfx1201,gfx9-4-generic,gfx10-3-generic,gfx11-generic,gfx12-generic"

@gulsumgudukbay gulsumgudukbay May 20, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure why we need all of these. Doesn't for example gfx9-4-generic cover gfx950 already?

@magaonka-amd magaonka-amd May 20, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IIUC this most lilkely fix sort kernel issues , with max_k unit tests???

@gulsumgudukbay gulsumgudukbay May 20, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IIUC this most lilkely fix sort kernel issues , with max_k unit tests???

if the sort kernel issues are there, why do we add generic targets? why don't we keep specific targets until those issues are fixed?

@tsrw2048 tsrw2048 May 20, 2026 •
edited
Loading

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@gulsumgudukbay We can drop generic arch build targets if needed. I was under the impression that we wanted to add them in and test with them so we would have a smoother transition to pure generic build targets in the future. As mentioned earlier, the main issue with generic build targets right now is the max_k tests that use the sort kernel.

@gulsumgudukbay gulsumgudukbay May 20, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@gulsumgudukbay We can drop generic arch build targets if needed. I was under the impression that we wanted to add them in and test with them so it would we would have a smoother transition to pure generic build targets in the future. As mentioned earlier, the main issue with generic build targets right now is the max_k tests that use the sort kernel.

maybe it is better to keep specific targets until the issue is fixed. did we have any doc to track theRock issue @magaonka-amd?
if there's a bug we shouldn't use the generic targets. originally I proposed the generic targets to rocm-jax to reduce wheel size. but if they are buggy with TheRock and since this is the downstream repo (changes like these are candidates to upstream) we should be careful (and wait) in what we upstream.

Or if our goal is to test/track if the bug is fixed in downstream CI, we can keep only generic targets. Right now it will bloat the wheel size because we have duplicate targets

@tsrw2048 tsrw2048 May 20, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@gulsumgudukbay That makes sense. I will leave these as specific targets for now and close the PR then.


# Used for @xla//build_tools/rocm:parallel_gpu_execute
common:rocm --legacy_external_runfiles=true
Expand Down