Shader packs: read struct and buffer-block members that atomic functions use

naga's GLSL frontend read `s.member` on the right-hand side out of a loaded
copy of the whole struct. Once an atomic function marks that member atomic,
the copy's member is an atomic value no operator, conversion, call or store
takes ("Incompatible operands: Atomic(...) as Uint"). The member is now
loaded through a pointer to it, as array elements already were; constant
expressions keep the value form.

Tests: every read the #9081 probe makes (assignment, operators,
constructors, min, subgroupBroadcastFirst) on a shared struct member, a
buffer-block member, a member of a shared struct array and a nested member;
constants' members in constant expressions; a member beside a runtime-sized
array; and, on a host GPU, the values those reads return.

Fixes #9081

Co-Authored-By: Claude Opus 5.5 <[email protected]>
This commit is contained in:
Arslan ArmanulyandClaude Opus 5.5 committed 2026-11-04 15:44:19 +05:00
1 parent 4601a3ba9f
commit b873ea2dd6
4 files changed
+305 -1

No files matched your search

+2
View File
@@ -16,6 +16,8 @@ are in that folder (`LICENSE.MIT`, `LICENSE.APACHE`).
The GLSL frontend also resets the common type for each comma-separated declarator so one
variable's array specifier does not change later variables.
It also reads a struct member outside a constant expression through a pointer to the member,
not out of a loaded copy of the whole struct, so a member an atomic function uses can be read.
## libmimalloc-sys and mimalloc
@@ -0,0 +1,162 @@
//! An ordinary read of a struct or buffer-block member that an atomic function also uses gets
//! the member's current value (#9081), on a host GPU through the translated program.
use horizon_shaders::{Macros, Pack, preprocess, translate_compute};
/// Runs compute program `source` once (one work group), with `probe_buffer` (set 4, binding 2)
/// starting at `start`, and returns `probe_result.result` (set 4, binding 1).
fn run(device: &wgpu::Device, queue: &wgpu::Queue, source: &str, start: [u32; 2]) -> u32 {
use std::io::Write;
use wgpu::util::DeviceExt;
let mut zip = zip::ZipWriter::new(std::io::Cursor::new(Vec::new()));
zip.start_file(
"shaders/deferred.csh",
zip::write::SimpleFileOptions::default(),
)
.unwrap();
zip.write_all(source.as_bytes()).unwrap();
let pack =
Pack::from_zip_bytes("atomic-values.zip", zip.finish().unwrap().into_inner()).unwrap();
let program = translate_compute(&preprocess(&pack, "deferred.csh", &Macros::standard()))
.unwrap_or_else(|e| panic!("{e:?}"));
let module = device.create_shader_module(wgpu::ShaderModuleDescriptor {
label: None,
source: wgpu::ShaderSource::Naga(std::borrow::Cow::Owned(program.module)),
});
let pipeline = device.create_compute_pipeline(&wgpu::ComputePipelineDescriptor {
label: None,
layout: None,
module: &module,
entry_point: Some("main"),
compilation_options: Default::default(),
cache: None,
});
let storage = wgpu::BufferUsages::STORAGE | wgpu::BufferUsages::COPY_SRC;
let uniforms = device.create_buffer(&wgpu::BufferDescriptor {
label: None,
size: u64::from(program.interface.uniform_size.max(48)),
usage: wgpu::BufferUsages::UNIFORM,
mapped_at_creation: false,
});
let result = device.create_buffer_init(&wgpu::util::BufferInitDescriptor {
label: None,
contents: &[0; 4],
usage: storage,
});
let probe: Vec<u8> = start.iter().flat_map(|v| v.to_le_bytes()).collect();
let probe = device.create_buffer_init(&wgpu::util::BufferInitDescriptor {
label: None,
contents: &probe,
usage: storage,
});
let read = device.create_buffer(&wgpu::BufferDescriptor {
label: None,
size: 4,
usage: wgpu::BufferUsages::COPY_DST | wgpu::BufferUsages::MAP_READ,
mapped_at_creation: false,
});
let groups: Vec<wgpu::BindGroup> = (0..5)
.map(|set| {
let entries: Vec<wgpu::BindGroupEntry> = match set {
0 => vec![wgpu::BindGroupEntry {
binding: 0,
resource: uniforms.as_entire_binding(),
}],
4 if source.contains("probe_buffer") => vec![
wgpu::BindGroupEntry {
binding: 1,
resource: result.as_entire_binding(),
},
wgpu::BindGroupEntry {
binding: 2,
resource: probe.as_entire_binding(),
},
],
4 => vec![wgpu::BindGroupEntry {
binding: 1,
resource: result.as_entire_binding(),
}],
_ => Vec::new(),
};
device.create_bind_group(&wgpu::BindGroupDescriptor {
label: None,
layout: &pipeline.get_bind_group_layout(set),
entries: &entries,
})
})
.collect();
let mut encoder = device.create_command_encoder(&Default::default());
{
let mut pass = encoder.begin_compute_pass(&Default::default());
pass.set_pipeline(&pipeline);
for (set, group) in groups.iter().enumerate() {
pass.set_bind_group(set as u32, group, &[]);
}
pass.dispatch_workgroups(1, 1, 1);
}
encoder.copy_buffer_to_buffer(&result, 0, &read, 0, 4);
queue.submit([encoder.finish()]);
read.slice(..)
.map_async(wgpu::MapMode::Read, |r| r.unwrap());
device.poll(wgpu::PollType::wait_indefinitely()).unwrap();
let bytes = read.slice(..).get_mapped_range().unwrap();
u32::from_le_bytes(bytes[..4].try_into().unwrap())
}
#[test]
#[ignore = "requires a host GPU; run with --ignored --nocapture"]
fn reads_of_atomic_members_see_the_atomic_updates() {
let instance = wgpu::Instance::new(wgpu::InstanceDescriptor::new_without_display_handle());
let adapter = pollster::block_on(instance.request_adapter(&Default::default())).unwrap();
eprintln!("Atomic member GPU: {}", adapter.get_info().name);
// Pack buffers sit in bind group 4, as in the game.
let (device, queue) = pollster::block_on(adapter.request_device(&wgpu::DeviceDescriptor {
required_limits: wgpu::Limits {
max_bind_groups: 5,
..Default::default()
},
..Default::default()
}))
.unwrap();
let result = "layout(std430, binding = 1) buffer probe_result { uint result; };\n";
// Eight invocations each add 1 to a zeroed shared member, then read it: 8.
for (host, member) in [
("shared struct { uint other; uint count; } sh;", "sh.count"),
(
"struct Counter { uint count; uint other; };\nshared Counter counters[2];",
"counters[1].count",
),
] {
for (read, expected) in [
("result = M * 10u + 1u;", 81),
("uint r = M; result = r;", 8),
("result = uint(float(M) * 0.5);", 4),
("result = min(M, 5u);", 5),
] {
let source = format!(
"#version 460 core\nlayout(local_size_x = 8) in;\n{host}\n{result}\
void main() {{\n {member} = 0u;\n barrier();\n \
atomicAdd({member}, 1u);\n barrier();\n {}\n}}\n",
read.replace('M', member)
);
assert_eq!(
run(&device, &queue, &source, [0, 0]),
expected,
"{member}: {read}"
);
}
}
// One invocation adds 2 to a buffer member that starts at 5, then reads it: 7.
for (read, expected) in [
("result = pc.count * 10u + 1u;", 71),
("uint r = pc.count; result = r;", 7),
("result = uint(int(pc.count) - 10 + 100);", 97),
] {
let source = format!(
"#version 460 core\nlayout(local_size_x = 1) in;\n\
layout(std430, binding = 2) buffer probe_buffer {{ uint count; uint other; }} pc;\n\
{result}void main() {{\n atomicAdd(pc.count, 2u);\n {read}\n}}\n"
);
assert_eq!(run(&device, &queue, &source, [5, 0]), expected, "{read}");
}
}
@@ -0,0 +1,130 @@
//! A struct member or buffer-block member that an atomic function uses is read like any other
//! `uint` (GLSL 4.60, 8.11, as #9081 quotes it: atomic memory functions work on "an individual
//! signed or unsigned integer stored in buffer object or shared variable storage"). The shapes
//! come from #9081's probe, which mirrors RenderPearl v2.7.2 (MIT, see the pull request).
use horizon_shaders::{Macros, Pack, preprocess, translate_compute};
/// The ordinary reads of a member `M` the probe makes: an assignment, an operator, constructors,
/// built-in calls and a subgroup operation.
const READS: [&str; 7] = [
"uint r = M; result = r;",
"result = M + 1u;",
"result = uint(M);",
"result = uint(int(M) + 1);",
"result = uint(float(M));",
"result = min(M, 4u);",
"result = subgroupBroadcastFirst(M);",
];
/// Where the member lives, and how the program names it.
const HOSTS: [(&str, &str); 4] = [
("shared struct { uint count; uint other; } sh;", "sh.count"),
(
"layout(std430, binding = 2) buffer probe_buffer { uint count; uint other; } pc;",
"pc.count",
),
(
"struct Counter { uint count; uint other; };\nshared Counter counters[2];",
"counters[1].count",
),
(
"struct Inner { uint count; };\nshared struct { uint first; Inner inner; } nested;",
"nested.inner.count",
),
];
fn program(host: &str, member: &str, read: &str) -> String {
format!(
"#version 460 core\n#extension GL_KHR_shader_subgroup_ballot : require\n\
layout(local_size_x = 8) in;\n{host}\n\
layout(std430, binding = 1) buffer probe_result {{ uint result; }};\n\
void main() {{\n {member} = 0u;\n barrier();\n atomicAdd({member}, 1u);\n \
barrier();\n {}\n}}\n",
read.replace('M', member)
)
}
/// Translates `source`, then checks that it validates and that every backend wgpu uses writes it.
fn translate(source: &str) -> Result<(), String> {
let zip = {
use std::io::Write;
let mut zip = zip::ZipWriter::new(std::io::Cursor::new(Vec::new()));
zip.start_file(
"shaders/deferred.csh",
zip::write::SimpleFileOptions::default(),
)
.unwrap();
zip.write_all(source.as_bytes()).unwrap();
zip.finish().unwrap().into_inner()
};
let pack = Pack::from_zip_bytes("atomic-members.zip", zip).unwrap();
let program = translate_compute(&preprocess(&pack, "deferred.csh", &Macros::standard()))
.map_err(|e| format!("{e:?}"))?;
let info = naga::valid::Validator::new(
naga::valid::ValidationFlags::all(),
naga::valid::Capabilities::all(),
)
.validate(&program.module)
.map_err(|e| format!("{e:?}"))?;
naga::back::spv::write_vec(&program.module, &info, &Default::default(), None)
.map_err(|e| format!("spv: {e:?}"))?;
let msl = naga::back::msl::Options {
lang_version: (2, 4),
..Default::default()
};
naga::back::msl::write_string(&program.module, &info, &msl, &Default::default())
.map_err(|e| format!("msl: {e:?}"))?;
Ok(())
}
#[test]
fn every_read_of_an_atomic_member_translates() {
let mut failed = Vec::new();
for (host, member) in HOSTS {
for read in READS {
if let Err(e) = translate(&program(host, member, read)) {
failed.push(format!("{member}: `{read}`: {e}"));
}
}
}
assert!(failed.is_empty(), "{failed:#?}");
}
#[test]
fn a_read_member_next_to_an_atomic_one_keeps_its_value_type() {
// `other` is never atomic: it is stored and read as before, beside the atomic `count`.
let source = "#version 460 core\nlayout(local_size_x = 8) in;\n\
struct Pair { uint count; uint other; };\nshared Pair sh;\n\
layout(std430, binding = 1) buffer probe_result { uint result; };\n\
void main() {\n sh.other = 3u;\n sh.count = 0u;\n barrier();\n \
atomicAdd(sh.count, 1u);\n barrier();\n \
result = sh.other * 2u + sh.count;\n}\n";
translate(source).unwrap();
}
#[test]
fn members_of_constants_stay_constant_expressions() {
// Constant expressions read a constant's component or member as a value: global `const`s
// built from them, and an array size.
let source = "#version 460 core\nlayout(local_size_x = 8) in;\n\
struct Size { ivec2 xy; int z; };\n\
const Size size = Size(ivec2(4, 2), 3);\n\
const vec3 scale = vec3(0.5, 0.25, 2.0);\n\
const float second = scale.y;\n\
const int rows = size.xy.y * size.z;\n\
layout(std430, binding = 1) buffer probe_result { uint result; };\n\
void main() {\n float values[rows];\n values[0] = second;\n \
result = uint(values[0] * 8.0) + uint(scale.z);\n}\n";
translate(source).unwrap();
}
#[test]
fn a_member_beside_a_runtime_sized_array_is_read_on_its_own() {
// The same member read, with no atomic function: a buffer block ending in an array without
// a size can't be loaded whole, only member by member.
let source = "#version 460 core\nlayout(local_size_x = 1) in;\n\
layout(std430, binding = 2) buffer probe_buffer { uint count; uint data[]; } pc;\n\
void main() {\n pc.data[0] = pc.count + 1u;\n}\n";
translate(source).unwrap();
}
+11 -1
View File
@@ -594,7 +594,17 @@ impl<'a> Context<'a> {
pointer
}
HirExprKind::Select { base, ref field } => {
let base = self.lower_expect_inner(stmt, frontend, base, pos)?.0;
// A member read loads the member through a pointer, as an array element does,
// instead of taking it out of a loaded copy of the whole value: a member an
// atomic function uses becomes atomic (`atomic_upgrade`), and only a load of it
// gives its value. A constant expression keeps its value form: it has no memory.
let base_pos = match pos {
ExprPos::Rhs if !self.is_const => ExprPos::AccessBase {
constant_index: true,
},
other => other,
};
let base = self.lower_expect_inner(stmt, frontend, base, base_pos)?.0;
frontend.field_selection(self, pos, base, field, meta)?
}