Skip to content

Commit 8a18ddd

Browse files
maleadtclaude
andauthored
Metal: fix byte-GEP coalescing crash on cross-block chains (#863)
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent df59387 commit 8a18ddd

2 files changed

Lines changed: 49 additions & 10 deletions

File tree

src/metal.jl

Lines changed: 17 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -407,18 +407,25 @@ function merge_byte_gep_chains!(mod::LLVM.Module)
407407
v isa LLVM.GetElementPtrInst && length(operands(v)) == 2 &&
408408
LLVM.LLVMType(LLVM.API.LLVMGetGEPSourceElementType(v)) == i8
409409

410-
for f in functions(mod)
411-
isdeclaration(f) && continue
412-
worklist = LLVM.Instruction[]
410+
# the first `gep i8, (gep i8, p, A), B` in `f`, or `nothing`
411+
function next_chain(f)
413412
for bb in blocks(f), inst in instructions(bb)
414-
is_byte_gep(inst) && is_byte_gep(operands(inst)[1]) && push!(worklist, inst)
413+
is_byte_gep(inst) && is_byte_gep(operands(inst)[1]) && return inst
415414
end
416-
# process defs-before-uses (instruction order) so longer chains collapse fully:
417-
# an inner gep is rewritten before the outer one that consumes it.
418-
for gep in worklist
419-
src = operands(gep)[1]
420-
(is_byte_gep(gep) && is_byte_gep(src)) || continue # may already be merged
421-
base = operands(src)[1]
415+
return nothing
416+
end
417+
418+
for f in functions(mod)
419+
isdeclaration(f) && continue
420+
# Rescan after each merge instead of keeping a worklist: a merge can erase a *different*
421+
# chained GEP (a now-dead inner GEP whose only use was the one we just folded), and a
422+
# block layout that doesn't follow dominance can place that inner GEP after the outer one,
423+
# so stale worklist entries would be use-after-free. Rescanning only ever inspects live
424+
# instructions; each merge replaces a chained GEP with a shallower one (and shortens its
425+
# users), so the total chain depth strictly decreases and this terminates.
426+
while (gep = next_chain(f)) !== nothing
427+
src = operands(gep)[1]
428+
base = operands(src)[1]
422429
inbounds = LLVM.API.LLVMIsInBounds(gep) != 0 && LLVM.API.LLVMIsInBounds(src) != 0
423430
@dispose builder=IRBuilder() begin
424431
position!(builder, gep)

test/metal.jl

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -159,6 +159,38 @@ end
159159
@test count("getelementptr", air) == 1
160160
end
161161

162+
@testset "byte GEP coalescing across blocks" begin
163+
# Regression: when the outer GEP's block is laid out before its (dominating) inner base's
164+
# block, the inner GEP can be erased before it is reached. A stale worklist would then
165+
# dereference a freed instruction (`BoundsError` on a 0-element operand set), so the pass
166+
# must rescan live instructions instead. It must also still fully coalesce the 3-level chain.
167+
p = opaque_ptrs ? "ptr addrspace(1)" : "i8 addrspace(1)*"
168+
ir = """
169+
define void @k($p %p, i64 %i, i64 %a, i64 %b) {
170+
entry:
171+
br label %bbB
172+
bbA:
173+
%g3 = getelementptr i8, $p %g2, i64 %b
174+
store i8 0, $p %g3, align 1
175+
br label %end
176+
bbB:
177+
%g1 = getelementptr i8, $p %p, i64 %i
178+
%g2 = getelementptr i8, $p %g1, i64 %a
179+
br label %bbA
180+
end:
181+
ret void
182+
}
183+
"""
184+
@dispose ctx=Context() begin
185+
mod = parse(LLVM.Module, ir)
186+
GPUCompiler.merge_byte_gep_chains!(mod)
187+
f = functions(mod)["k"]
188+
ngep = sum(bb -> count(i -> i isa LLVM.GetElementPtrInst, collect(instructions(bb))),
189+
collect(blocks(f)))
190+
@test ngep == 1
191+
end
192+
end
193+
162194
@testset "GC runtime input arguments" begin
163195
mod = @eval module $(gensym())
164196
mutable struct PleaseAllocate

0 commit comments

Comments
 (0)