Skip to content

Commit 7d2cd3d

Browse files
committed
Add OpenGL binding checks for uniform/storage block variables
Implements ARB_gl_spirv validation: Uniform and shader storage block variables must be decorated with a Binding in OpenGL environments. - Add is_opengl_env() helper - Add OpenGlBufferBindingRule in new opengl.rs module - Make vulkan.rs helper functions pub(super) for reuse - Add OpenGlBufferMissingBindingDecoration error variant
1 parent ebd6b81 commit 7d2cd3d

7 files changed

Lines changed: 423 additions & 3 deletions

File tree

rust/spirv-tools-core/src/validation/error.rs

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -297,6 +297,14 @@ pub enum ValidationError {
297297
/// The struct type ID.
298298
struct_id: u32,
299299
},
300+
/// An OpenGL uniform/storage block variable is missing a Binding decoration (ARB_gl_spirv).
301+
#[error("{storage_class:?} id {variable_id} is missing Binding decoration. From ARB_gl_spirv extension: Uniform and shader storage block variables must also be decorated with a Binding.")]
302+
OpenGlBufferMissingBindingDecoration {
303+
/// The storage class of the variable.
304+
storage_class: rspirv::spirv::StorageClass,
305+
/// The variable ID.
306+
variable_id: u32,
307+
},
300308
/// A Block/BufferBlock struct is missing ArrayStride decoration on an array member.
301309
#[error("Structure id {struct_id} decorated as {decoration_type} must be explicitly laid out with ArrayStride decorations")]
302310
BlockMissingArrayStride {

rust/spirv-tools-core/src/validation/helpers.rs

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -566,6 +566,18 @@ pub fn is_vulkan_env(env: TargetEnv) -> bool {
566566
)
567567
}
568568

569+
/// Returns true if the target environment is an OpenGL environment.
570+
pub fn is_opengl_env(env: TargetEnv) -> bool {
571+
matches!(
572+
env,
573+
TargetEnv::OpenGl4_0
574+
| TargetEnv::OpenGl4_1
575+
| TargetEnv::OpenGl4_2
576+
| TargetEnv::OpenGl4_3
577+
| TargetEnv::OpenGl4_5
578+
)
579+
}
580+
569581
// ============================================================================
570582
// Constant evaluation helpers
571583
// ============================================================================

rust/spirv-tools-core/src/validation/mod.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -326,6 +326,7 @@ pub(crate) fn validate_words_internal(
326326
&rules::block_layout::all_block_layout_rules(),
327327
)?;
328328
run_rules(&validation_ctx, &rules::vulkan::all_vulkan_rules())?;
329+
run_rules(&validation_ctx, &rules::opengl::all_opengl_rules())?;
329330
run_rules(&validation_ctx, &rules::pointers::all_pointer_rules())?;
330331
run_rules(&validation_ctx, &rules::decorations::all_decoration_rules())?;
331332
run_rules(

rust/spirv-tools-core/src/validation/rules/mod.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@
3939
//! - [`memory`]: Memory model validation rules
4040
//! - [`pointers`]: Pointer and store validation rules
4141
//! - [`vulkan`]: Vulkan-specific validation rules
42+
//! - [`opengl`]: OpenGL-specific validation rules (ARB_gl_spirv)
4243
//! - [`entry_points`]: Entry point interface validation rules
4344
//! - [`execution_limitations`]: Execution model limitations validation (callgraph-based)
4445
//! - [`execution_modes`]: Execution mode validation rules
@@ -110,6 +111,7 @@ pub mod mesh_shading;
110111
pub mod misc;
111112
pub mod mode_setting;
112113
pub mod non_uniform;
114+
pub mod opengl;
113115
pub mod pointers;
114116
pub mod primitives;
115117
pub mod ray_tracing;
Lines changed: 141 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,141 @@
1+
//! OpenGL-specific validation rules.
2+
//!
3+
//! This module validates OpenGL-specific SPIR-V requirements including:
4+
//!
5+
//! - ARB_gl_spirv binding requirements for uniform/storage block variables
6+
7+
use rspirv::spirv::{Decoration, Op, StorageClass};
8+
9+
use crate::validation::context::{ValidationContext, ValidationRule};
10+
use crate::validation::error::ValidationError;
11+
use crate::validation::helpers::is_opengl_env;
12+
use crate::validation::rules::vulkan::{
13+
get_array_element_struct, get_variable_pointee_type, has_decoration,
14+
};
15+
use crate::validation::types::ResultId;
16+
use crate::validation::ValidationResult;
17+
18+
// ============================================================================
19+
// OpenGL Buffer Binding Rule
20+
// ============================================================================
21+
22+
/// Validates that uniform and shader storage block variables have Binding decorations in OpenGL.
23+
///
24+
/// From ARB_gl_spirv extension:
25+
/// Uniform and shader storage block variables must also be decorated with a *Binding*.
26+
pub struct OpenGlBufferBindingRule;
27+
28+
impl ValidationRule for OpenGlBufferBindingRule {
29+
fn name(&self) -> &'static str {
30+
"opengl-buffer-binding"
31+
}
32+
33+
fn should_skip(&self, ctx: &ValidationContext<'_>) -> bool {
34+
!is_opengl_env(ctx.env)
35+
}
36+
37+
fn validate(&self, ctx: &ValidationContext<'_>) -> ValidationResult {
38+
// Build a set of variable IDs referenced by any entry point
39+
let mut entry_point_vars: std::collections::HashSet<u32> =
40+
std::collections::HashSet::new();
41+
for ep in &ctx.module.entry_points {
42+
let mut operands = ep.operands.iter();
43+
// Skip ExecutionModel
44+
let _ = operands.next();
45+
// Skip entry point function ID
46+
let _ = operands.next();
47+
// Skip name (LiteralString)
48+
let _ = operands.next();
49+
// Remaining operands are interface variable IDs
50+
for operand in operands {
51+
if let rspirv::dr::Operand::IdRef(id) = operand {
52+
entry_point_vars.insert(*id);
53+
}
54+
}
55+
}
56+
57+
for inst in &ctx.module.types_global_values {
58+
if inst.class.opcode != Op::Variable && inst.class.opcode != Op::UntypedVariableKHR {
59+
continue;
60+
}
61+
62+
let storage_class = match inst.operands.first() {
63+
Some(rspirv::dr::Operand::StorageClass(sc)) => *sc,
64+
_ => continue,
65+
};
66+
67+
let var_id = match inst.result_id {
68+
Some(id) => id,
69+
None => continue,
70+
};
71+
72+
let uniform = storage_class == StorageClass::Uniform
73+
|| storage_class == StorageClass::UniformConstant;
74+
let storage_buffer = storage_class == StorageClass::StorageBuffer;
75+
76+
if !uniform && !storage_buffer {
77+
continue;
78+
}
79+
80+
// Get the pointee struct type
81+
let pointee_type_id = get_variable_pointee_type(inst, ctx);
82+
let Some(struct_id) = pointee_type_id else {
83+
continue;
84+
};
85+
86+
// Check if pointee is a struct (possibly through arrays)
87+
let pointee_opcode = ResultId::try_from(struct_id)
88+
.ok()
89+
.and_then(|rid| ctx.opcodes.get(&rid))
90+
.copied();
91+
92+
let final_struct_id = if matches!(
93+
pointee_opcode,
94+
Some(Op::TypeArray) | Some(Op::TypeRuntimeArray)
95+
) {
96+
get_array_element_struct(struct_id, ctx)
97+
} else if pointee_opcode == Some(Op::TypeStruct) {
98+
Some(struct_id)
99+
} else {
100+
None
101+
};
102+
103+
let Some(struct_id) = final_struct_id else {
104+
continue;
105+
};
106+
107+
let has_block = has_decoration(ctx, struct_id, Decoration::Block);
108+
let has_buffer_block = has_decoration(ctx, struct_id, Decoration::BufferBlock);
109+
110+
// OpenGL requires Binding on:
111+
// - Uniform variables with Block or BufferBlock decoration
112+
// - StorageBuffer variables with Block decoration
113+
if (uniform && (has_block || has_buffer_block))
114+
|| (storage_buffer && has_block)
115+
{
116+
// Only check variables referenced by entry points
117+
if !entry_point_vars.is_empty()
118+
&& entry_point_vars.contains(&var_id)
119+
&& !has_decoration(ctx, var_id, Decoration::Binding)
120+
{
121+
return Err(ValidationError::OpenGlBufferMissingBindingDecoration {
122+
storage_class,
123+
variable_id: var_id,
124+
}
125+
.into());
126+
}
127+
}
128+
}
129+
130+
Ok(())
131+
}
132+
}
133+
134+
// ============================================================================
135+
// All OpenGL rules
136+
// ============================================================================
137+
138+
/// Returns all OpenGL-specific validation rules.
139+
pub fn all_opengl_rules() -> Vec<&'static dyn ValidationRule> {
140+
vec![&OpenGlBufferBindingRule]
141+
}

rust/spirv-tools-core/src/validation/rules/vulkan.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -375,7 +375,7 @@ fn int_bit_width(
375375
}
376376
}
377377

378-
fn has_decoration(ctx: &ValidationContext<'_>, target: u32, decoration: Decoration) -> bool {
378+
pub(super) fn has_decoration(ctx: &ValidationContext<'_>, target: u32, decoration: Decoration) -> bool {
379379
ctx.module.annotations.iter().any(|inst| {
380380
inst.class.opcode == Op::Decorate
381381
&& matches!(
@@ -710,7 +710,7 @@ impl ValidationRule for VulkanBufferBlockDecorationsRule {
710710
}
711711

712712
/// Gets the pointee type ID from a variable instruction.
713-
fn get_variable_pointee_type(
713+
pub(super) fn get_variable_pointee_type(
714714
inst: &rspirv::dr::Instruction,
715715
ctx: &ValidationContext<'_>,
716716
) -> Option<u32> {
@@ -745,7 +745,7 @@ fn get_variable_pointee_type(
745745
}
746746

747747
/// Gets the struct element type from an array type.
748-
fn get_array_element_struct(array_type_id: u32, ctx: &ValidationContext<'_>) -> Option<u32> {
748+
pub(super) fn get_array_element_struct(array_type_id: u32, ctx: &ValidationContext<'_>) -> Option<u32> {
749749
let array_inst = ResultId::try_from(array_type_id)
750750
.ok()
751751
.and_then(|rid| ctx.definitions.get(&rid))?;

0 commit comments

Comments
 (0)