[FlyToROCDL] Bump LLVM to 941a04e6 and adapt gpu.launch_func lowering - #1051
Open
Phil-amd wants to merge 1 commit into
Open
[FlyToROCDL] Bump LLVM to 941a04e6 and adapt gpu.launch_func lowering#1051Phil-amd wants to merge 1 commit into
Phil-amd wants to merge 1 commit into
Conversation
Phil-amd
force-pushed
the
phil/bump-llvm-941a04e6
branch
2 times, most recently
from
August 21, 2026 06:27
04e3147 to
b6eadae
Compare
Phil-amd
changed the title
Member
Author
|
@jli-melchior |
Collaborator
|
LGTM |
Phil-amd
force-pushed
the
phil/bump-llvm-941a04e6
branch
2 times, most recently
from
August 26, 2026 09:47
b6eadae to
33d9959
Compare
Phil-amd
added a commit
that referenced
this pull request
Aug 27, 2026
prepare-mlir builds one shared MLIR install from the PR's pin and uses it for both wheels. An LLVM pin bump also carries the source adaptation that the new pin requires, so the base commit cannot compile against it and the baseline wheel is never produced. The run ends at "No usable main benchmark baseline found" (run 32954898547, PR #1051), losing the vs-main comparison for exactly the PRs whose performance impact is least predictable. When the LLVM inputs differ from the base commit's, restore the MLIR install belonging to the base pin and build the baseline wheel against that. - ci_mlir_cache_key.sh derives the cache key from file contents, making it computable for the base commit. It replaces hashFiles() and serves both call sites, so the two keys cannot drift. - The baseline entry is restored before the shared one and under the same path. actions/cache derives its version from the path list, so a renamed restore would miss whatever the key said. - The unpacked install is checked against its VCSRevision.h. A wrong-pin baseline is worse than none: it yields a plausible number nobody queries. - prepare-mlir marks the wheel with the LLVM it was really built against, and the table is labelled from that marker. Deriving it from the pinned hashes would mislabel a bump that needed no source adaptation. A cache miss means no baseline. The wanted entry is the one every non-bump PR restores on every run, so a miss is the exception. Changing the key formula invalidates the cache, so every PR cold-builds LLVM until main's next push saves an entry under the new one.
This was referenced Aug 27, 2026
coderfeli
pushed a commit
that referenced
this pull request
Aug 27, 2026
#1071) prepare-mlir builds one shared MLIR install from the PR's pin and uses it for both wheels. An LLVM pin bump also carries the source adaptation that the new pin requires, so the base commit cannot compile against it and the baseline wheel is never produced. The run ends at "No usable main benchmark baseline found" (run 32954898547, PR #1051), losing the vs-main comparison for exactly the PRs whose performance impact is least predictable. When the LLVM inputs differ from the base commit's, restore the MLIR install belonging to the base pin and build the baseline wheel against that. - ci_mlir_cache_key.sh derives the cache key from file contents, making it computable for the base commit. It replaces hashFiles() and serves both call sites, so the two keys cannot drift. - The baseline entry is restored before the shared one and under the same path. actions/cache derives its version from the path list, so a renamed restore would miss whatever the key said. - The unpacked install is checked against its VCSRevision.h. A wrong-pin baseline is worse than none: it yields a plausible number nobody queries. - prepare-mlir marks the wheel with the LLVM it was really built against, and the table is labelled from that marker. Deriving it from the pinned hashes would mislabel a bump that needed no source adaptation. A cache miss means no baseline. The wanted entry is the one every non-bump PR restores on every run, so a miss is the exception. Changing the key formula invalidates the cache, so every PR cold-builds LLVM until main's next push saves an entry under the new one.
…wering Move the upstream LLVM pin from e2a39f504fee to 941a04e69ee8. Upstream merged the two gpu::LaunchFuncOp builders taking a SymbolRefAttr kernel into one, inserting `Value asyncObject` before `clusterSize`. LaunchFuncOpLowering chose between the two old signatures, so neither call matched any more; FlyToROCDL.cpp was the only translation unit that failed to build. Collapse both branches into one call against the merged builder. This also stops an op carrying both an asyncObject and an async token from losing the token: the old asyncObject builder never pushed an AsyncTokenType onto the result types. Signed-off-by: Phil Li <haicli@amd.com>
Phil-amd
force-pushed
the
phil/bump-llvm-941a04e6
branch
from
August 27, 2026 21:19
3f40950 to
0afb7f8
Compare
coderfeli
pushed a commit
that referenced
this pull request
Aug 31, 2026
The baseline install is ~1.4 GB and these runners sit in different
regions, so the transfer is slow enough to be interrupted mid-flight:
Received 1010827264 of 1396566653 (72.4%), 1.6 MBs/sec
##[warning]Failed to restore: Server failed to authenticate the
request ... Authorization header ... signature.
Cache not found for input keys: mlir-install-...-4087a417...
The entry existed and matched - 1396566653 bytes is its exact size - but
the blob SAS auth failed partway through, and actions/cache reports that
as a plain miss. The run then built the baseline wheel against the PR's
own MLIR, where it cannot compile, and lost the vs-main comparison to
what was really a network fault. Seen on PR #1051, run 33117631657; the
same signature is on record for these runners from an earlier
investigation.
Retry the restore once. It costs nothing when the first attempt works,
and both attempts carry a timeout so a hung transfer cannot eat the job
budget instead.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Bump the upstream LLVM pin from
e2a39f504fee(2026-07-23) to941a04e69ee8(2026-08-09), and adapt
FlyToROCDLto the one upstream API change that brokethe build.
Motivation
Picks up ~2.5 weeks of upstream LLVM, including
llvm/llvm-project#214885 "[AMDGPU] Keep TDM counter low",
which makes the hazard recognizer insert
s_wait_tensorcnt 10before a TDM opthat would otherwise follow another one with no intervening drain
(gfx1250/gfx1251, new
needs-tdm-drainsubtarget feature).Changes
thirdparty/llvm-build-info.json:upstream.llvm_hash→941a04e69ee8.lib/Conversion/FlyToROCDL/FlyToROCDL.cpp: upstream merged the twogpu::LaunchFuncOpbuilders taking aSymbolRefAttrkernel into one,inserting
Value asyncObjectbetweenasyncDependenciesandclusterSize.LaunchFuncOpLoweringchose between the two old signatures, so neither callmatched any more. Both branches collapse into a single call against the merged
builder. This was the only translation unit that failed to build.
Side effect: the old
asyncObjectbuilder never pushed anAsyncTokenTypeonto the result types, so an op carrying both an
asyncObjectand an asynctoken lost the token. The merged builder now gets the token type in both
cases. The guard rejecting
asyncObject+asyncDependenciesis unchanged.Breaking Changes
None for FlyDSL users. Consumers building their own LLVM must move to
941a04e69ee8:FlyToROCDL.cppwill not compile against the old pin, where themerged
gpu::LaunchFuncOpbuilder does not exist.