Repository navigation
GCN: fold cast-null checks of stack objects; return datalayout strings everywhere - #1001
Merged
Merged
Conversation
Since JuliaLang/julia#58837 (Julia 1.11.8 and 1.12), the null checks guarding loads of immutables kept in memory compare against an addrspacecast of the generic null, rather than the private null. AMDGPU lowers that to -1, so the check is correct, but LLVM doesn't fold it, and the stack slot stays alive. Fold that form too, and check that the test kernel doesn't need a stack slot, which also covers these Julia versions.
Targets returned either a string or a DataLayout the caller had to dispose, and the GCN target did both depending on the back-end, which broke callers that disposed the result. Following LLVM.jl 10, return the text everywhere, and leave creating a DataLayout object to the callers that need one.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1001 +/- ##
==========================================
- Coverage 89.46% 89.46% -0.01%
==========================================
Files 31 31
Lines 6883 6881 -2
==========================================
- Hits 6158 6156 -2
Misses 725 725 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
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
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.
Two follow-ups to #1000.
Null checks on Julia 1.11.8+. The null checks #1000 folds come from Julia's codegen (
null_pointer_cmp), which compared against the private null, i.e. address 0. On AMDGPU, that's the address of the first stack object. JuliaLang/julia#58837 changed the comparison to use the generic null cast to the private address space, which AMDGPU lowers to -1. That fix is in Julia 1.11.8+ and 1.12, but not in 1.10 (where it is labeled for backport but was never backported). The checks it emits give correct results, but LLVM doesn't fold them, so the stack slot stays alive: the kernel from #1000's test needs 24 bytes of scratch on Julia 1.11.9, and none with this PR. The fold now also matches this form. The test now checks that noallocais left, which also covers the newer Julia versions; the old@check_notpattern passed on them without checking anything.llvm_datalayout. Depending on the target,llvm_datalayoutreturned either a string or aDataLayoutthe caller had to dispose. For GCN, the type even depended on the back-end, which is why #1000 had to adjust the atomic validation test. It now returns a string (ornothing) everywhere, following LLVM.jl 10's convention: text where text is wanted,DataLayout(...)for an object you query. The overrides I could find (Enzyme, WASMCompiler.jl, Whisk.jl) already return strings or forward the call.