[CI][DO NOT MERGE] Probe: LLVM pin bump on top of #1071 - #1072
Draft
Phil-amd wants to merge 3 commits into
Draft
Conversation
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>
Throwaway, for PR #1072 only. Never merge this commit. The baseline entry #1072 needs is keyed on main's LLVM inputs, and nothing has saved it: caches a PR creates are scoped to that PR's merge ref, so #1071 cannot warm it for another PR. Only a push to main can. Rather than block the positive control on that merge, build the base pin here and save it under the base key at this PR's own scope, which the PR can read back: - First run: baseline restore misses, the scaffolding builds the base commit's LLVM and saves it. Everything downstream then runs against a real base MLIR - the base wheel compiles where it cannot on main today, the BASELINE_LLVM marker is written, and the vs-main table prints. - Re-run: the restore hits, exercising the real cache path end to end. The second llvm-project source and build tree are dropped as soon as the tarball is packaged, so the wheel build does not run with both on disk.
Member
Author
|
Added probe scaffolding ( The entry this PR wants is keyed on main's LLVM inputs and nothing has saved it — PR caches are scoped to their own merge ref, so #1071 cannot warm it for another PR. But Run 1 (this push) — baseline restore misses, scaffolding builds LLVM
Run 2 (re-run all jobs) — the restore hits and the same chain runs off the cache rather than a fresh build. That is the real path. Scaffolding is three steps plus a cache save, all Still not covered by any of this: that a push to main saves an entry the next PR can read. That is GitHub's own cache-scope behaviour, not something this change introduces. |
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
1 participant
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.
Throwaway probe for #1071. Do not merge, do not review as a change — it exists only to drive CI down the pin-bump path. #1051 is untouched; its commit is cherry-picked here.
Contents: #1071 (the baseline-MLIR change) plus @Phil-amd's commit from #1051 (
33d9959, LLVMe2a39f50->941a04e6and thegpu.launch_funcadaptation).What this run can prove
Resolve base LLVM pinreportspin_changed=true(verified locally against this exact tree).vs latest tagstill runs, and the PR stays green.What it cannot prove yet, and why
The baseline cache will miss, and that is expected — not a bug.
The entry it looks for is keyed
...4087a417(main's three LLVM inputs under the new content-derived formula). Nothing has saved that yet: caches created by a PR are scoped to that PR's merge ref, so #1071's own run cannot warm it for anyone else. Only a push to main saves an entry other PRs can read.So the positive control — cache hit, baseline wheel built against the old MLIR, a printed
current vs maintable — needs this order:Until step 2, a skip here is the correct behaviour, and today it is also what #1051 gets on main.
Expected on the first run
Resolve base LLVM pinpin_changed=true,base_llvm_hash=e2a39f50…Restore baseline MLIR cacheUnpack and verify baseline MLIRe2a39f50…, vs-main skippedBASELINE_LLVMmarkermain@<sha>with no(llvm …)suffixvs latest tagAlso note
prepare-mlircold-builds LLVM here: the key formula changed, so the old cache does not apply. That cost is stated in #1071.Close this once #1071 lands and the positive control has been observed.