Skip to content

Commit 56fe883

Browse files
committed
Add OpSwitch selector type validation
Add SwitchSelectorTypeRule that verifies the OpSwitch selector operand is an integer type (OpTypeInt), as required by the SPIR-V spec. A float or other non-integer selector is rejected with SwitchSelectorNotInteger.
1 parent 64635c5 commit 56fe883

3 files changed

Lines changed: 203 additions & 0 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
@@ -2813,6 +2813,14 @@ pub enum ValidationError {
28132813
/// The duplicate literal value.
28142814
literal: u64,
28152815
},
2816+
/// OpSwitch selector must be an integer type.
2817+
#[error("OpSwitch selector {selector_id} must be OpTypeInt, found {found_opcode:?}")]
2818+
SwitchSelectorNotInteger {
2819+
/// The selector operand ID.
2820+
selector_id: Id,
2821+
/// The opcode of the selector's type.
2822+
found_opcode: rspirv::spirv::Op,
2823+
},
28162824
/// A basic block is missing its required `OpLabel`.
28172825
#[error("function {function:?} contains a block without OpLabel at index {block_index}")]
28182826
MissingBlockLabel {

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

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1737,6 +1737,62 @@ impl ValidationRule for SwitchCaseUniquenessRule {
17371737
}
17381738
}
17391739

1740+
// ============================================================================
1741+
// Switch Selector Type Rule
1742+
// ============================================================================
1743+
1744+
/// Validates that OpSwitch selector operand is an integer type.
1745+
pub struct SwitchSelectorTypeRule;
1746+
1747+
impl ValidationRule for SwitchSelectorTypeRule {
1748+
fn name(&self) -> &'static str {
1749+
"switch-selector-type"
1750+
}
1751+
1752+
fn validate(&self, ctx: &ValidationContext<'_>) -> ValidationResult {
1753+
for function in &ctx.module.functions {
1754+
for block in &function.blocks {
1755+
for inst in &block.instructions {
1756+
if inst.class.opcode != Op::Switch {
1757+
continue;
1758+
}
1759+
// OpSwitch operands[0] = selector id
1760+
let Some(Operand::IdRef(selector_id)) = inst.operands.first() else {
1761+
continue;
1762+
};
1763+
let selector_rid = match ResultId::try_from(*selector_id) {
1764+
Ok(r) => r,
1765+
Err(_) => continue,
1766+
};
1767+
// Look up the instruction that defines the selector
1768+
let Some(selector_def) = ctx.definitions.get(&selector_rid) else {
1769+
continue;
1770+
};
1771+
// Get the selector's result type
1772+
let Some(type_id) = selector_def.result_type else {
1773+
continue;
1774+
};
1775+
let type_rid = match ResultId::try_from(type_id) {
1776+
Ok(r) => r,
1777+
Err(_) => continue,
1778+
};
1779+
let Some(type_inst) = ctx.definitions.get(&type_rid) else {
1780+
continue;
1781+
};
1782+
if type_inst.class.opcode != Op::TypeInt {
1783+
return Err(ValidationError::SwitchSelectorNotInteger {
1784+
selector_id: to_id(*selector_id),
1785+
found_opcode: type_inst.class.opcode,
1786+
}
1787+
.into());
1788+
}
1789+
}
1790+
}
1791+
}
1792+
Ok(())
1793+
}
1794+
}
1795+
17401796
/// Static rule instances
17411797
static BLOCK_STRUCTURE_RULE: BlockStructureRule = BlockStructureRule;
17421798
static MERGE_INSTRUCTION_RULE: MergeInstructionRule = MergeInstructionRule;
@@ -1752,6 +1808,7 @@ static MAXIMAL_RECONVERGENCE_PREDECESSORS_RULE: MaximalReconvergencePredecessors
17521808
MaximalReconvergencePredecessorsRule;
17531809
static LIFETIME_RULE: LifetimeRule = LifetimeRule;
17541810
static SWITCH_CASE_UNIQUENESS_RULE: SwitchCaseUniquenessRule = SwitchCaseUniquenessRule;
1811+
static SWITCH_SELECTOR_TYPE_RULE: SwitchSelectorTypeRule = SwitchSelectorTypeRule;
17551812

17561813
/// Returns all CFG validation rules.
17571814
pub fn all_cfg_rules() -> Vec<&'static dyn ValidationRule> {
@@ -1769,5 +1826,6 @@ pub fn all_cfg_rules() -> Vec<&'static dyn ValidationRule> {
17691826
&MAXIMAL_RECONVERGENCE_PREDECESSORS_RULE,
17701827
&LIFETIME_RULE,
17711828
&SWITCH_CASE_UNIQUENESS_RULE,
1829+
&SWITCH_SELECTOR_TYPE_RULE,
17721830
]
17731831
}

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

Lines changed: 137 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1654,3 +1654,140 @@ fn switch_duplicate_case_literal_fails() {
16541654
"expected SwitchDuplicateCaseLiteral with literal 42, got {err:?}"
16551655
);
16561656
}
1657+
1658+
// ============================================================================
1659+
// Switch Selector Type Validation
1660+
// ============================================================================
1661+
1662+
#[test]
1663+
fn switch_with_integer_selector_passes() {
1664+
let text = r#"
1665+
OpCapability Shader
1666+
OpMemoryModel Logical GLSL450
1667+
OpEntryPoint GLCompute %main "main"
1668+
OpExecutionMode %main LocalSize 1 1 1
1669+
%void = OpTypeVoid
1670+
%fn = OpTypeFunction %void
1671+
%u32 = OpTypeInt 32 0
1672+
%u32_0 = OpConstant %u32 0
1673+
%main = OpFunction %void None %fn
1674+
%entry = OpLabel
1675+
OpSelectionMerge %merge None
1676+
OpSwitch %u32_0 %merge 1 %case1 2 %case2
1677+
%case1 = OpLabel
1678+
OpBranch %merge
1679+
%case2 = OpLabel
1680+
OpBranch %merge
1681+
%merge = OpLabel
1682+
OpReturn
1683+
OpFunctionEnd
1684+
"#;
1685+
assemble_and_validate(text).expect("OpSwitch with integer selector should pass");
1686+
}
1687+
1688+
#[test]
1689+
fn switch_with_float_selector_fails() {
1690+
use rspirv::binary::Assemble;
1691+
use rspirv::dr::{Instruction, Module, Operand};
1692+
1693+
let mut module = Module::new();
1694+
module.header = Some(rspirv::dr::ModuleHeader {
1695+
magic_number: rspirv::spirv::MAGIC_NUMBER,
1696+
version: (1 << 16) | (5 << 8),
1697+
generator: 0,
1698+
bound: 20,
1699+
reserved_word: 0,
1700+
});
1701+
1702+
module.capabilities.push(Instruction::new(
1703+
Op::Capability,
1704+
None,
1705+
None,
1706+
vec![Operand::Capability(rspirv::spirv::Capability::Shader)],
1707+
));
1708+
module.memory_model = Some(Instruction::new(
1709+
Op::MemoryModel,
1710+
None,
1711+
None,
1712+
vec![
1713+
Operand::AddressingModel(rspirv::spirv::AddressingModel::Logical),
1714+
Operand::MemoryModel(rspirv::spirv::MemoryModel::GLSL450),
1715+
],
1716+
));
1717+
module.entry_points.push(Instruction::new(
1718+
Op::EntryPoint,
1719+
None,
1720+
None,
1721+
vec![
1722+
Operand::ExecutionModel(rspirv::spirv::ExecutionModel::GLCompute),
1723+
Operand::IdRef(10),
1724+
Operand::LiteralString("main".to_string()),
1725+
],
1726+
));
1727+
module.execution_modes.push(Instruction::new(
1728+
Op::ExecutionMode,
1729+
None,
1730+
None,
1731+
vec![
1732+
Operand::IdRef(10),
1733+
Operand::ExecutionMode(rspirv::spirv::ExecutionMode::LocalSize),
1734+
Operand::LiteralBit32(1),
1735+
Operand::LiteralBit32(1),
1736+
Operand::LiteralBit32(1),
1737+
],
1738+
));
1739+
1740+
// %2 = OpTypeVoid
1741+
module.types_global_values.push(Instruction::new(Op::TypeVoid, None, Some(2), vec![]));
1742+
// %3 = OpTypeFunction %void
1743+
module.types_global_values.push(Instruction::new(
1744+
Op::TypeFunction, None, Some(3), vec![Operand::IdRef(2)],
1745+
));
1746+
// %4 = OpTypeFloat 32
1747+
module.types_global_values.push(Instruction::new(
1748+
Op::TypeFloat, None, Some(4), vec![Operand::LiteralBit32(32)],
1749+
));
1750+
// %5 = OpConstant %4 0.0
1751+
module.types_global_values.push(Instruction::new(
1752+
Op::Constant, Some(4), Some(5), vec![Operand::LiteralBit32(0)],
1753+
));
1754+
1755+
let mut func = rspirv::dr::Function::new();
1756+
func.def = Some(Instruction::new(
1757+
Op::Function, Some(2), Some(10),
1758+
vec![
1759+
Operand::FunctionControl(rspirv::spirv::FunctionControl::NONE),
1760+
Operand::IdRef(3),
1761+
],
1762+
));
1763+
1764+
// Entry block with Switch on float selector
1765+
let mut entry = rspirv::dr::Block::new();
1766+
entry.label = Some(Instruction::new(Op::Label, None, Some(11), vec![]));
1767+
entry.instructions.push(Instruction::new(
1768+
Op::SelectionMerge, None, None,
1769+
vec![Operand::IdRef(14), Operand::SelectionControl(rspirv::spirv::SelectionControl::NONE)],
1770+
));
1771+
// OpSwitch %5(float) %14(merge) -- this is invalid
1772+
entry.instructions.push(Instruction::new(
1773+
Op::Switch, None, None,
1774+
vec![Operand::IdRef(5), Operand::IdRef(14)],
1775+
));
1776+
func.blocks.push(entry);
1777+
1778+
let mut merge = rspirv::dr::Block::new();
1779+
merge.label = Some(Instruction::new(Op::Label, None, Some(14), vec![]));
1780+
merge.instructions.push(Instruction::new(Op::Return, None, None, vec![]));
1781+
func.blocks.push(merge);
1782+
1783+
func.end = Some(Instruction::new(Op::FunctionEnd, None, None, vec![]));
1784+
module.functions.push(func);
1785+
1786+
let binary = module.assemble();
1787+
let err = validate_module(&binary, TargetEnv::Universal1_6)
1788+
.expect_err("OpSwitch with float selector should fail");
1789+
assert!(
1790+
matches!(err, ValidationError::SwitchSelectorNotInteger { .. }),
1791+
"expected SwitchSelectorNotInteger, got {err:?}"
1792+
);
1793+
}

0 commit comments

Comments
 (0)