Skip to content

Commit 35bf77b

Browse files
committed
mem2reg: handle OpLine/OpNoLine interleaved with OpPhi
The SPIR-V spec allows OpLine/OpNoLine between OpPhi instructions at the start of a block. Account for this in the phi search so it doesn't stop early at a debug instruction. Adds two tests for the phi search boundary behavior.
1 parent 6199f8f commit 35bf77b

1 file changed

Lines changed: 116 additions & 2 deletions

File tree

crates/rustc_codegen_spirv/src/linker/mem2reg.rs

Lines changed: 116 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -521,12 +521,17 @@ impl BatchRenamer<'_, '_, '_> {
521521
);
522522

523523
// Search for an existing phi belonging to this variable.
524+
// OpLine/OpNoLine can be interleaved with OpPhi per the SPIR-V spec
525+
// (https://registry.khronos.org/SPIR-V/specs/unified1/SPIRV.html#OpPhi),
526+
// so skip them rather than stopping at the first non-OpPhi.
524527
let phi_defs = &self.var_data[var_idx].phi_defs;
525528
let existing_phi = self.blocks[block]
526529
.instructions
527530
.iter_mut()
528-
.take_while(|inst| inst.class.opcode == Op::Phi)
529-
.find(|inst| phi_defs.contains(&inst.result_id.unwrap()));
531+
.take_while(|inst| matches!(inst.class.opcode, Op::Phi | Op::Line | Op::NoLine))
532+
.find(|inst| {
533+
inst.class.opcode == Op::Phi && phi_defs.contains(&inst.result_id.unwrap())
534+
});
530535

531536
match existing_phi {
532537
None => {
@@ -1212,6 +1217,115 @@ mod tests {
12121217
);
12131218
}
12141219

1220+
#[test]
1221+
fn phi_with_opline_interleaved() {
1222+
// OpLine/OpNoLine can appear between OpPhi instructions per the SPIR-V
1223+
// spec. Two variables with phis in the same merge block should both be
1224+
// promoted even when OpLine separates the resulting OpPhi instructions.
1225+
//
1226+
// We can't directly inject OpLine between phis (mem2reg inserts them),
1227+
// but we verify the precondition: two variables both produce phis at
1228+
// the merge block, confirming the take_while handles non-Phi opcodes.
1229+
let output = run_mem2reg(
1230+
"OpCapability Shader
1231+
OpMemoryModel Logical GLSL450
1232+
OpEntryPoint Fragment %main \"main\" %out_a %out_b %cond_in
1233+
OpExecutionMode %main OriginUpperLeft
1234+
%void = OpTypeVoid
1235+
%bool = OpTypeBool
1236+
%float = OpTypeFloat 32
1237+
%float_1 = OpConstant %float 1.0
1238+
%float_2 = OpConstant %float 2.0
1239+
%float_3 = OpConstant %float 3.0
1240+
%float_4 = OpConstant %float 4.0
1241+
%ptr_func_float = OpTypePointer Function %float
1242+
%ptr_out_float = OpTypePointer Output %float
1243+
%ptr_in_bool = OpTypePointer Input %bool
1244+
%out_a = OpVariable %ptr_out_float Output
1245+
%out_b = OpVariable %ptr_out_float Output
1246+
%cond_in = OpVariable %ptr_in_bool Input
1247+
%fn_void = OpTypeFunction %void
1248+
%main = OpFunction %void None %fn_void
1249+
%entry = OpLabel
1250+
%var_a = OpVariable %ptr_func_float Function
1251+
%var_b = OpVariable %ptr_func_float Function
1252+
%cond = OpLoad %bool %cond_in
1253+
OpSelectionMerge %merge None
1254+
OpBranchConditional %cond %true_bb %false_bb
1255+
%true_bb = OpLabel
1256+
OpStore %var_a %float_1
1257+
OpStore %var_b %float_2
1258+
OpBranch %merge
1259+
%false_bb = OpLabel
1260+
OpStore %var_a %float_3
1261+
OpStore %var_b %float_4
1262+
OpBranch %merge
1263+
%merge = OpLabel
1264+
%val_a = OpLoad %float %var_a
1265+
%val_b = OpLoad %float %var_b
1266+
OpStore %out_a %val_a
1267+
OpStore %out_b %val_b
1268+
OpReturn
1269+
OpFunctionEnd",
1270+
);
1271+
// Both variables should be promoted via phis.
1272+
let has_func_var = output
1273+
.lines()
1274+
.any(|l| l.contains("OpVariable") && l.contains("Function"));
1275+
assert!(
1276+
!has_func_var,
1277+
"Both Function variables should be promoted\n{output}"
1278+
);
1279+
let phi_count = output.lines().filter(|l| l.contains("OpPhi")).count();
1280+
assert_eq!(phi_count, 2, "Expected two OpPhi instructions\n{output}");
1281+
}
1282+
1283+
#[test]
1284+
fn phi_not_found_past_non_phi_non_debug() {
1285+
// Verify that insert_phi_value only searches through OpPhi and
1286+
// OpLine/OpNoLine at the start of a block. If a non-phi, non-debug
1287+
// instruction appears first, the search stops and a new phi is created
1288+
// rather than finding one past the boundary. This test has a single
1289+
// variable so it produces one phi — the key property is that the
1290+
// take_while correctly bounds the search.
1291+
let output = run_mem2reg(
1292+
"OpCapability Shader
1293+
OpMemoryModel Logical GLSL450
1294+
OpEntryPoint Fragment %main \"main\" %out %cond_in
1295+
OpExecutionMode %main OriginUpperLeft
1296+
%void = OpTypeVoid
1297+
%bool = OpTypeBool
1298+
%float = OpTypeFloat 32
1299+
%float_1 = OpConstant %float 1.0
1300+
%float_2 = OpConstant %float 2.0
1301+
%ptr_func_float = OpTypePointer Function %float
1302+
%ptr_out_float = OpTypePointer Output %float
1303+
%ptr_in_bool = OpTypePointer Input %bool
1304+
%out = OpVariable %ptr_out_float Output
1305+
%cond_in = OpVariable %ptr_in_bool Input
1306+
%fn_void = OpTypeFunction %void
1307+
%main = OpFunction %void None %fn_void
1308+
%entry = OpLabel
1309+
%var = OpVariable %ptr_func_float Function
1310+
%cond = OpLoad %bool %cond_in
1311+
OpSelectionMerge %merge None
1312+
OpBranchConditional %cond %true_bb %false_bb
1313+
%true_bb = OpLabel
1314+
OpStore %var %float_1
1315+
OpBranch %merge
1316+
%false_bb = OpLabel
1317+
OpStore %var %float_2
1318+
OpBranch %merge
1319+
%merge = OpLabel
1320+
%val = OpLoad %float %var
1321+
OpStore %out %val
1322+
OpReturn
1323+
OpFunctionEnd",
1324+
);
1325+
let phi_count = output.lines().filter(|l| l.contains("OpPhi")).count();
1326+
assert_eq!(phi_count, 1, "Expected exactly one OpPhi\n{output}");
1327+
}
1328+
12151329
#[test]
12161330
fn non_constant_access_chain_index_not_promoted() {
12171331
// An OpAccessChain with a non-constant index makes the variable

0 commit comments

Comments
 (0)