Skip to content

Commit db3da9b

Browse files
committed
Fix builtin direction validation to check per-entry-point usage
The BuiltinStorageClassDirectionRule was validating builtin directions against ALL entry points in the module, but a builtin variable may only be used by specific entry points. For example, ClipDistance as Input is valid in Fragment shaders but not in Vertex shaders. If a module has both entry points but ClipDistance is only in the Fragment interface, validation should pass. This fix builds a map of which entry points actually use each variable (from the OpEntryPoint interface lists) and only validates builtin direction against those specific entry points. Added positive and negative tests for the fix.
1 parent 64684ea commit db3da9b

2 files changed

Lines changed: 96 additions & 3 deletions

File tree

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

Lines changed: 23 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1094,7 +1094,24 @@ impl ValidationRule for BuiltinStorageClassDirectionRule {
10941094
}
10951095

10961096
let module = ctx.module;
1097-
let entry_models = ctx.entry_models;
1097+
1098+
// Build map of variable ID -> set of execution models that use it
1099+
// by checking entry point interfaces
1100+
let mut var_to_models: HashMap<u32, HashSet<ExecutionModel>> = HashMap::new();
1101+
for ep in &module.entry_points {
1102+
if ep.class.opcode != Op::EntryPoint {
1103+
continue;
1104+
}
1105+
let Some(rspirv::dr::Operand::ExecutionModel(model)) = ep.operands.first() else {
1106+
continue;
1107+
};
1108+
// Interface variables start at operand index 3 (after ExecutionModel, Function, Name)
1109+
for op in ep.operands.iter().skip(3) {
1110+
if let rspirv::dr::Operand::IdRef(var_id) = op {
1111+
var_to_models.entry(*var_id).or_default().insert(*model);
1112+
}
1113+
}
1114+
}
10981115

10991116
// Collect built-in decorations with their storage classes
11001117
for inst in &module.annotations {
@@ -1141,8 +1158,11 @@ impl ValidationRule for BuiltinStorageClassDirectionRule {
11411158
continue;
11421159
};
11431160

1144-
// Validate direction per built-in per execution model
1145-
for model in entry_models {
1161+
// Only validate direction against entry points that actually use this variable
1162+
let Some(models) = var_to_models.get(target) else {
1163+
continue;
1164+
};
1165+
for model in models {
11461166
if let Some(error) = validate_builtin_direction(builtin, storage_class, *model) {
11471167
return Err(error.into());
11481168
}

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

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30413,3 +30413,76 @@ fn instance_id_builtin_accepted_in_vulkan_ray_tracing_shader() {
3041330413
.validate(TargetEnv::Vulkan1_2)
3041430414
.expect("InstanceId should be accepted in Vulkan ray tracing shader");
3041530415
}
30416+
30417+
#[test]
30418+
fn clip_distance_input_rejected_in_vertex_shader() {
30419+
// ClipDistance as Input is not allowed in Vertex shaders
30420+
let text = [
30421+
"OpCapability Shader",
30422+
"OpCapability ClipDistance",
30423+
"OpMemoryModel Logical GLSL450",
30424+
"OpEntryPoint Vertex %main \"main\" %clip",
30425+
"%void = OpTypeVoid",
30426+
"%float = OpTypeFloat 32",
30427+
"%arr = OpTypeArray %float %uint_1",
30428+
"%uint = OpTypeInt 32 0",
30429+
"%uint_1 = OpConstant %uint 1",
30430+
"%ptr_input_arr = OpTypePointer Input %arr",
30431+
"%clip = OpVariable %ptr_input_arr Input",
30432+
"OpDecorate %clip BuiltIn ClipDistance",
30433+
"%fn = OpTypeFunction %void",
30434+
"%main = OpFunction %void None %fn",
30435+
"%entry = OpLabel",
30436+
"OpReturn",
30437+
"OpFunctionEnd",
30438+
]
30439+
.join("\n");
30440+
let error = text
30441+
.as_str()
30442+
.validate(TargetEnv::Vulkan1_1)
30443+
.expect_err("ClipDistance Input should be rejected in Vertex shader");
30444+
assert!(
30445+
matches!(
30446+
error,
30447+
ValidationError::BuiltInWrongStorageClassForExecutionModel { .. }
30448+
),
30449+
"Expected BuiltInWrongStorageClassForExecutionModel, got: {error:?}"
30450+
);
30451+
}
30452+
30453+
#[test]
30454+
fn clip_distance_input_accepted_in_fragment_with_separate_vertex_entry() {
30455+
// ClipDistance as Input is allowed in Fragment shaders, even when
30456+
// the module also has a Vertex entry point (that doesn't use it)
30457+
let text = [
30458+
"OpCapability Shader",
30459+
"OpCapability ClipDistance",
30460+
"OpMemoryModel Logical GLSL450",
30461+
// Vertex entry point does NOT list %clip in its interface
30462+
"OpEntryPoint Vertex %vmain \"vmain\"",
30463+
// Fragment entry point DOES list %clip in its interface
30464+
"OpEntryPoint Fragment %fmain \"fmain\" %clip",
30465+
"OpExecutionMode %fmain OriginUpperLeft",
30466+
"%void = OpTypeVoid",
30467+
"%float = OpTypeFloat 32",
30468+
"%uint = OpTypeInt 32 0",
30469+
"%uint_1 = OpConstant %uint 1",
30470+
"%arr = OpTypeArray %float %uint_1",
30471+
"%ptr_input_arr = OpTypePointer Input %arr",
30472+
"%clip = OpVariable %ptr_input_arr Input",
30473+
"OpDecorate %clip BuiltIn ClipDistance",
30474+
"%fn = OpTypeFunction %void",
30475+
"%vmain = OpFunction %void None %fn",
30476+
"%ventry = OpLabel",
30477+
"OpReturn",
30478+
"OpFunctionEnd",
30479+
"%fmain = OpFunction %void None %fn",
30480+
"%fentry = OpLabel",
30481+
"OpReturn",
30482+
"OpFunctionEnd",
30483+
]
30484+
.join("\n");
30485+
text.as_str()
30486+
.validate(TargetEnv::Vulkan1_1)
30487+
.expect("ClipDistance Input should be accepted in Fragment shader");
30488+
}

0 commit comments

Comments
 (0)