Skip to content

Commit ff39274

Browse files
committed
fix(compiler): carry vector operands through inlining and CSE
Two passes walk an instruction's operands by hand and end the match with a catch-all, and the vector family fell into it. Both were live miscompiles, found writing a tensor kernel in ZynML. Inlining substitutes the id an instruction defines so a cloned body lands on fresh values, and its list of instructions that define something named none of the vector ops. An inlined `vector_splat` kept the id it had in the callee while every use of it moved to the fresh one, so `vstore` wrote a value nothing had computed. A store inside a called function silently did nothing and the kernel read back zero. CSE rewrites the uses of an instruction it eliminates, over thirteen kinds and no vector ones. Eliminating a redundant `extractvalue` left a `vload` pointing at an id that no longer existed, which Cranelift met as "arg not in value_map". `HirInstruction::replace_uses` covers all nine ops and is the one to reach for. These went unnoticed because vectorised code stays inside the function that produced it, so neither pass had met a vector op until one was written by hand.
1 parent 67a8bff commit ff39274

2 files changed

Lines changed: 39 additions & 1 deletion

File tree

‎crates/compiler/src/cse.rs‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -568,6 +568,32 @@ fn rewrite_inst_operands(inst: &mut HirInstruction, map: &impl Fn(&mut HirId) ->
568568
HirInstruction::Unary { operand, .. } => m!(operand),
569569
HirInstruction::Cast { operand, .. } => m!(operand),
570570
HirInstruction::Load { ptr, .. } => m!(ptr),
571+
// The vector family. Leaving these out let this pass delete an
572+
// instruction and rewrite every use of it except the ones a
573+
// vector op held, so a `vload` kept pointing at an id nothing
574+
// defined any more.
575+
HirInstruction::VectorSplat { scalar, .. } => m!(scalar),
576+
HirInstruction::VectorExtractLane { vector, .. } => m!(vector),
577+
HirInstruction::VectorInsertLane { vector, scalar, .. } => {
578+
m!(vector);
579+
m!(scalar);
580+
}
581+
HirInstruction::VectorHorizontalReduce { vector, .. } => m!(vector),
582+
HirInstruction::VectorLoad { ptr, .. } => m!(ptr),
583+
HirInstruction::VectorStore { value, ptr, .. } => {
584+
m!(value);
585+
m!(ptr);
586+
}
587+
HirInstruction::VectorUnaryOp { operand, .. } => m!(operand),
588+
HirInstruction::VectorMinMax { left, right, .. } => {
589+
m!(left);
590+
m!(right);
591+
}
592+
HirInstruction::VectorDot { acc, a, b, .. } => {
593+
m!(acc);
594+
m!(a);
595+
m!(b);
596+
}
571597
HirInstruction::Store { value, ptr, .. } => {
572598
m!(value);
573599
m!(ptr);

‎crates/compiler/src/inline.rs‎

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1341,7 +1341,19 @@ fn substitute_operands(inst: &mut HirInstruction, subs: &HashMap<HirId, HirId>)
13411341
| HirInstruction::InsertValue { result, .. }
13421342
| HirInstruction::Alloca { result, .. }
13431343
| HirInstruction::Select { result, .. }
1344-
| HirInstruction::Atomic { result, .. } => map_result(result),
1344+
| HirInstruction::Atomic { result, .. }
1345+
// Every vector instruction that defines a value. Leaving these
1346+
// out let the definition keep the id it had in the callee while
1347+
// its uses moved to the fresh one, so an inlined
1348+
// `vstore v, p` wrote a value nothing had computed.
1349+
| HirInstruction::VectorSplat { result, .. }
1350+
| HirInstruction::VectorExtractLane { result, .. }
1351+
| HirInstruction::VectorInsertLane { result, .. }
1352+
| HirInstruction::VectorHorizontalReduce { result, .. }
1353+
| HirInstruction::VectorLoad { result, .. }
1354+
| HirInstruction::VectorUnaryOp { result, .. }
1355+
| HirInstruction::VectorMinMax { result, .. }
1356+
| HirInstruction::VectorDot { result, .. } => map_result(result),
13451357
HirInstruction::Call { result, .. } | HirInstruction::IndirectCall { result, .. } => {
13461358
if let Some(r) = result {
13471359
map_result(r);

0 commit comments

Comments
 (0)