Skip to content

rocmPackages.hipblaslt: stop inlining war and peace in asm comments#449985

Merged
JohnRTitor merged 1 commit intoNixOS:masterfrom
LunNova:lunnova/hipblaslt-fix-maybe
Oct 11, 2025
Merged

rocmPackages.hipblaslt: stop inlining war and peace in asm comments#449985
JohnRTitor merged 1 commit intoNixOS:masterfrom
LunNova:lunnova/hipblaslt-fix-maybe

Conversation

@LunNova
Copy link
Member

@LunNova LunNova commented Oct 8, 2025 •
edited
Loading

Fixes #449880 by reducing peak build dir space usage to 150GB

hipblaslt was filling the build tmpfs on hydra, peaking at 240GB in build/Tensile/build_tmp/TENSILE

with this bodge we stop it from writing very verbose comments in the generated assembly kernels which were contributing a significant portion of the total build directory size

example excessive whitespace/comment content:
s_nop 0 // 1 wait state required when next inst writes vgprs held by previous dwordx4 store inst

This is not an optimal fix. Ideally TensileCreateLibrary should be taught to throw away .S files as soon as possible by turning them into code objects, rather than its current approach that materializes all assembly files at the same time in one huge directory.
I will aim to create a better upstreamable fix; we need a short term fix so here we are. 🙃

Things done

  • Built on platform:
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • Tested, as applicable:
  • Ran nixpkgs-review on this PR. See nixpkgs-review usage.
  • Tested basic functionality of all binary files, usually in ./result/bin/.
  • Nixpkgs Release Notes
    • Package update: when the change is major or breaking.
  • NixOS Release Notes
    • Module addition: when adding a new NixOS module.
    • Module update: when the change is significant.
  • Fits CONTRIBUTING.md, pkgs/README.md, maintainers/README.md and other READMEs.

Add a 👍 reaction to pull requests you find important.

@LunNova
rocmPackages.hipblaslt: stop inlining war and peace in asm comments
f263078 
Reduces peak build dir usage to 150G

hipblaslt was filling the build tmpfs, peaking at 240G in the
build/Tensile/build_tmp/TENSILEdir

with this bodge we stop it from writing very verbose comments in
the generated assembly kernels

example excessive whitespace/comment content:
s_nop 0                                            // 1 wait state required when next inst writes vgprs held by previous dwordx4 store inst
@LunNova
Copy link
Member Author

LunNova commented Oct 8, 2025

nixpkgs-review result

Generated using nixpkgs-review.

Command: nixpkgs-review pr 449985 --package rocmPackages.gfx908.hipblaslt --package rocmPackages.hipblaslt
Commit: f263078e24f2e66264a1913ff4d71393a82d76b8

x86_64-linux

✅ 4 packages built:
  • rocmPackages.gfx908.hipblaslt
  • rocmPackages.gfx908.hipblaslt.benchmark (rocmPackages.gfx908.hipblaslt.benchmark.benchmark)
  • rocmPackages.hipblaslt
  • rocmPackages.hipblaslt.benchmark (rocmPackages.hipblaslt.benchmark.benchmark)

@nixpkgs-ci nixpkgs-ci bot added 10.rebuild-linux: 11-100 This PR causes between 11 and 100 packages to rebuild on Linux. 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 11.by: package-maintainer This PR was created by a maintainer of all the package it changes. 6.topic: rocm ROCm is an Advanced Micro Devices software stack for graphics processing unit programming. labels Oct 8, 2025
@LunNova LunNova requested a review from 06kellyjac October 8, 2025 16:32
Flakebi
Flakebi approved these changes Oct 8, 2025
View reviewed changes
Copy link
Member

@Flakebi Flakebi left a comment

Choose a reason for hiding this comment

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

Impressive 😂
LGTM

GZGavinZhao
GZGavinZhao approved these changes Oct 8, 2025
View reviewed changes
Copy link
Contributor

@GZGavinZhao GZGavinZhao left a comment

Choose a reason for hiding this comment

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

WUT

@nixpkgs-ci nixpkgs-ci bot added 12.approvals: 2 This PR was reviewed and approved by two persons. 12.approved-by: package-maintainer This PR was reviewed and approved by a maintainer listed in any of the changed packages. labels Oct 8, 2025
@06kellyjac
Copy link
Member

06kellyjac commented Oct 8, 2025

Running a review, 30 mins in, 60G so far

06kellyjac
06kellyjac approved these changes Oct 8, 2025
View reviewed changes
Copy link
Member

@06kellyjac 06kellyjac left a comment

Choose a reason for hiding this comment

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

nixpkgs-review result

Generated using nixpkgs-review.

Command: nixpkgs-review pr 449985 --package rocmPackages.gfx908.hipblaslt --package rocmPackages.hipblaslt
Commit: f263078e24f2e66264a1913ff4d71393a82d76b8

x86_64-linux

✅ 4 packages built:
  • rocmPackages.gfx908.hipblaslt
  • rocmPackages.gfx908.hipblaslt.benchmark (rocmPackages.gfx908.hipblaslt.benchmark.benchmark)
  • rocmPackages.hipblaslt
  • rocmPackages.hipblaslt.benchmark (rocmPackages.hipblaslt.benchmark.benchmark)

151GB peak, very cool improvement 🚀

@nixpkgs-ci nixpkgs-ci bot added 12.approvals: 3+ This PR was reviewed and approved by three or more persons. and removed 12.approvals: 2 This PR was reviewed and approved by two persons. labels Oct 8, 2025
@nixos-discourse
Copy link

nixos-discourse commented Oct 9, 2025

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/prs-already-reviewed/2617/2581

@pshirshov
Copy link
Contributor

pshirshov commented Oct 10, 2025

Even with this patch the build failed for me with the same out of space failure.

@LunNova
Copy link
Member Author

LunNova commented Oct 10, 2025

It still needs way too much space with the patch, but it reduces it enough that it should pass on hydra. Talked with @mweinelt

If you're trying to build this locally make sure you have 160GB free wherever your nix build temp dirs end up.

@pshirshov
Copy link
Contributor

pshirshov commented Oct 10, 2025

My /tmp is on tmpfs so it's a bit limited... Ok, I'll try to switch to regular disk

@mweinelt
Copy link
Member

mweinelt commented Oct 10, 2025

It still needs way too much space with the patch, but it reduces it enough that it should pass on hydra. Talked with @mweinelt

The current tmpfs on x86 _64-linux is still 160 GB. I still need to reinstall the builders to make that larger. We might get lucky until then.

@JohnRTitor
Copy link
Member

JohnRTitor commented Oct 11, 2025 •
edited
Loading

My man.... That's why my build was taking too long and way too much space.

@JohnRTitor JohnRTitor added this pull request to the merge queue Oct 11, 2025
@JohnRTitor
Copy link
Member

JohnRTitor commented Oct 11, 2025 •
edited
Loading

Can anyone provide a cache for this :)

Merged via the queue into NixOS:master with commit 2d841db Oct 11, 2025
42 of 44 checks passed
@LunNova LunNova deleted the lunnova/hipblaslt-fix-maybe branch October 11, 2025 05:05
@LunNova
Copy link
Member Author

LunNova commented Oct 11, 2025

… way too much space.

we upgraded from 'way too much' to 'too much'

nukdokplex added a commit to nukdokplex/ncaa that referenced this pull request Oct 11, 2025
@nukdokplex
disable ollama to fix sleipnir build
ca2142f 
it's caused by hipblaslt build failure, alread fixed in master, refer to
NixOS/nixpkgs#449985
nukdokplex added a commit to nukdokplex/ncaa that referenced this pull request Oct 11, 2025
@nukdokplex
disable ollama to fix sleipnir build
ca9e3d6 
it's caused by hipblaslt build failure, alread fixed in master, refer to
NixOS/nixpkgs#449985
This was referenced Oct 12, 2025
Merged
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@06kellyjac 06kellyjac 06kellyjac approved these changes

@mschwaig mschwaig Awaiting requested review from mschwaig

@SuperSandro2000 SuperSandro2000 Awaiting requested review from SuperSandro2000

+4 more reviewers

@Flakebi Flakebi Flakebi approved these changes

@categoricalcat categoricalcat categoricalcat approved these changes

@qweered qweered qweered approved these changes

@GZGavinZhao GZGavinZhao GZGavinZhao approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

6.topic: rocm ROCm is an Advanced Micro Devices software stack for graphics processing unit programming. 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 11-100 This PR causes between 11 and 100 packages to rebuild on Linux. 11.by: package-maintainer This PR was created by a maintainer of all the package it changes. 12.approvals: 3+ This PR was reviewed and approved by three or more persons. 12.approved-by: package-maintainer This PR was reviewed and approved by a maintainer listed in any of the changed packages.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Build failure: rocmPackages.hipblaslt

10 participants

@LunNova @06kellyjac @nixos-discourse @pshirshov @mweinelt @JohnRTitor @Flakebi @categoricalcat @qweered @GZGavinZhao