Skip to content

codegen: mark write barrier field load as volatile #59559

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Sep 16, 2025
Merged

codegen: mark write barrier field load as volatile #59559

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions Compiler/test/codegen.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -134,14 +134,14 @@ if !is_debug_build && opt_level > 0
# Array
test_loads_no_call(strip_debug_calls(get_llvm(sizeof, Tuple{Vector{Int}})), [Iptr])
# As long as the eltype is known we don't need to load the elsize, but do need to check isvector
@test_skip test_loads_no_call(strip_debug_calls(get_llvm(sizeof, Tuple{Array{Any}})), ["atomic $Iptr", "ptr", "ptr", Iptr, Iptr, "ptr", Iptr])
@test_skip test_loads_no_call(strip_debug_calls(get_llvm(sizeof, Tuple{Array{Any}})), ["atomic volatile $Iptr", "ptr", "ptr", Iptr, Iptr, "ptr", Iptr])
# Memory
test_loads_no_call(strip_debug_calls(get_llvm(core_sizeof, Tuple{Memory{Int}})), [Iptr])
# As long as the eltype is known we don't need to load the elsize
test_loads_no_call(strip_debug_calls(get_llvm(core_sizeof, Tuple{Memory{Any}})), [Iptr])
# Check that we load the elsize and isunion from the typeof layout
test_loads_no_call(strip_debug_calls(get_llvm(core_sizeof, Tuple{Memory})), [Iptr, "atomic $Iptr", "ptr", "i32", "i16"])
test_loads_no_call(strip_debug_calls(get_llvm(core_sizeof, Tuple{Memory})), [Iptr, "atomic $Iptr", "ptr", "i32", "i16"])
test_loads_no_call(strip_debug_calls(get_llvm(core_sizeof, Tuple{Memory})), [Iptr, "atomic volatile $Iptr", "ptr", "i32", "i16"])
test_loads_no_call(strip_debug_calls(get_llvm(core_sizeof, Tuple{Memory})), [Iptr, "atomic volatile $Iptr", "ptr", "i32", "i16"])
# Primitive Type size should be folded to a constant
test_loads_no_call(strip_debug_calls(get_llvm(core_sizeof, Tuple{Ptr})), String[])

Expand Down
3 changes: 3 additions & 0 deletions src/llvm-late-gc-lowering.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1913,6 +1913,9 @@ Value *LateLowerGCFrame::EmitLoadTag(IRBuilder<> &builder, Type *T_size, Value *
auto &M = *builder.GetInsertBlock()->getModule();
LoadInst *load = builder.CreateAlignedLoad(T_size, addr, M.getDataLayout().getPointerABIAlignment(0), V->getName() + ".tag");
load->setOrdering(AtomicOrdering::Unordered);
// Mark as volatile to prevent optimizers from treating GC tag loads as constants
// since GC mark bits can change during runtime (issue #59547)
load->setVolatile(true);
load->setMetadata(LLVMContext::MD_tbaa, tbaa_tag);
MDBuilder MDB(load->getContext());
auto *NullInt = ConstantInt::get(T_size, 0);
Expand Down
41 changes: 41 additions & 0 deletions test/llvmpasses/gc-writebarrier-volatile.ll
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
; This file is a part of Julia. License is MIT: https://julialang.org/license

; RUN: opt --load-pass-plugin=libjulia-codegen%shlibext -passes='function(LateLowerGCFrame,FinalLowerGC,gvn)' -S %s | FileCheck %s

; Test for issue #59547: Ensure write barrier GC tag loads are volatile
; This test verifies that the LateLowerGCFrame pass marks GC tag loads as volatile
; to prevent GVN from incorrectly constant-folding them, which would eliminate
; necessary write barrier checks.

@tag = external addrspace(10) global {}, align 16

declare void @julia.write_barrier({} addrspace(10)*, {} addrspace(10)*)
declare {}*** @julia.get_pgcstack()
declare {} addrspace(10)* @julia.gc_alloc_obj({}**, i64, {} addrspace(10)*)

; Test that write barrier expansion produces volatile GC tag loads
; CHECK-LABEL: @test_writebarrier_volatile_tags
define {} addrspace(10)* @test_writebarrier_volatile_tags() {
top:
%pgcstack = call {}*** @julia.get_pgcstack()
%current_task = bitcast {}*** %pgcstack to {}**
%parent = call {} addrspace(10)* @julia.gc_alloc_obj({}** %current_task, i64 8, {} addrspace(10)* @tag)
%child = call {} addrspace(10)* @julia.gc_alloc_obj({}** %current_task, i64 8, {} addrspace(10)* @tag)
call void @julia.write_barrier({} addrspace(10)* %parent, {} addrspace(10)* %child)
ret {} addrspace(10)* %parent

; The critical test: GC tag loads must be volatile to prevent constant folding
; CHECK: load atomic volatile i64, ptr {{.*}} unordered, align 8, {{.*}}!tbaa
; CHECK: and i64 {{.*}}, 3
; CHECK: icmp eq i64 {{.*}}, 3
; CHECK: br i1 {{.*}}, label %may_trigger_wb, label

; CHECK: may_trigger_wb:
; CHECK: load atomic volatile i64, ptr {{.*}} unordered, align 8, {{.*}}!tbaa
; CHECK: and i64 {{.*}}, 1
; CHECK: icmp eq i64 {{.*}}, 0
; CHECK: br i1 {{.*}}, label %trigger_wb, label

; CHECK: trigger_wb:
; CHECK: call void @ijl_gc_queue_root(ptr {{.*}})
}