Skip to content

Introduce dedicated LeftDiv/RightDiv instructions and matrix-division handling #218

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
nallana wants to merge 3 commits into from
Closed

Introduce dedicated LeftDiv/RightDiv instructions and matrix-division handling #218

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
4aff9cb
Fix matrix division operators in ignition
nallana Mar 23, 2026
2620483
Remove orphan PrimitiveOp::Div and switch fusion to RightDiv
cursoragent Mar 23, 2026
b411ad4
Merge pull request #219 from runmat-org/cursor/orphaned-primitive-op-…
nallana Mar 24, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 6 additions & 6 deletions crates/runmat-accelerate/src/fusion.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -228,7 +228,7 @@ pub fn detect_fusion_groups(graph: &AccelGraph) -> Vec<FusionGroup> {
| AccelNodeLabel::Primitive(PrimitiveOp::Sub)
| AccelNodeLabel::Primitive(PrimitiveOp::Mul)
| AccelNodeLabel::Primitive(PrimitiveOp::ElemMul)
| AccelNodeLabel::Primitive(PrimitiveOp::Div)
| AccelNodeLabel::Primitive(PrimitiveOp::RightDiv)
Copy link
Copy Markdown

@cursor cursor bot Mar 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

RightDiv unreachable in elementwise fusion allowed list

Low Severity

PrimitiveOp::RightDiv is listed in the allowed elementwise ops for fusion chain folding, but it can never be reached. The is_elementwise() check on line 221 returns false for RightDiv nodes (since primitive_category maps it to AccelOpCategory::Other), so the loop always breaks before evaluating the allowed match. This dead entry was introduced by renaming Div → RightDiv while also changing its category from Elementwise to Other, without removing it from the allowed list.

Fix in Cursor Fix in Web

| AccelNodeLabel::Primitive(PrimitiveOp::ElemDiv)
);
if !allowed {
Expand Down Expand Up @@ -1089,7 +1089,7 @@ impl FusionGroupPlan {
match &node.label {
AccelNodeLabel::Primitive(PrimitiveOp::Mul)
| AccelNodeLabel::Primitive(PrimitiveOp::ElemMul)
| AccelNodeLabel::Primitive(PrimitiveOp::Div)
| AccelNodeLabel::Primitive(PrimitiveOp::RightDiv)
| AccelNodeLabel::Primitive(PrimitiveOp::ElemDiv)
| AccelNodeLabel::Primitive(PrimitiveOp::ElemLeftDiv)
| AccelNodeLabel::Primitive(PrimitiveOp::Add)
Expand Down Expand Up @@ -1819,7 +1819,7 @@ fn detect_centered_gram(
AccelNodeLabel::Primitive(op) => op,
_ => continue,
};
if div_op != PrimitiveOp::Div && div_op != PrimitiveOp::ElemDiv {
if div_op != PrimitiveOp::RightDiv && div_op != PrimitiveOp::ElemDiv {
continue;
}
if div_node.inputs.len() != 2 {
Expand Down Expand Up @@ -2094,7 +2094,7 @@ fn detect_power_step_normalize(
AccelNodeLabel::Primitive(op) => op,
_ => continue,
};
if div_op != PrimitiveOp::Div && div_op != PrimitiveOp::ElemDiv {
if div_op != PrimitiveOp::RightDiv && div_op != PrimitiveOp::ElemDiv {
continue;
}
if div_node.inputs.len() != 2 {
Expand Down Expand Up @@ -2599,7 +2599,7 @@ fn primitive_expr(
let (lhs, rhs) = binary(exprs)?;
Some(format!("({lhs} * {rhs})"))
}
PrimitiveOp::Div | PrimitiveOp::ElemDiv | PrimitiveOp::ElemLeftDiv => {
PrimitiveOp::RightDiv | PrimitiveOp::ElemDiv | PrimitiveOp::ElemLeftDiv => {
let (lhs, rhs) = binary(exprs)?;
Some(format!("({lhs} / {rhs})"))
}
Expand Down Expand Up @@ -3003,7 +3003,7 @@ fn analyze_image_normalize(
}
match div_node.label {
AccelNodeLabel::Primitive(PrimitiveOp::ElemDiv)
| AccelNodeLabel::Primitive(PrimitiveOp::Div) => {}
| AccelNodeLabel::Primitive(PrimitiveOp::RightDiv) => {}
_ => img_norm_fail!("not div primitive"),
}
if div_node.inputs.len() != 2 {
Expand Down
2 changes: 1 addition & 1 deletion crates/runmat-accelerate/src/fusion_exec.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -956,7 +956,7 @@ pub async fn execute_matmul_epilogue(request: FusionExecutionRequest<'_>) -> Res
}
}
}
crate::graph::PrimitiveOp::Div | crate::graph::PrimitiveOp::ElemDiv => {
crate::graph::PrimitiveOp::RightDiv | crate::graph::PrimitiveOp::ElemDiv => {
if let Some(val) = const_f64 {
if val != 0.0 {
alpha *= 1.0 / val;
Expand Down
6 changes: 4 additions & 2 deletions crates/runmat-accelerate/src/graph.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,8 @@ pub enum PrimitiveOp {
Add,
Sub,
Mul,
Div,
RightDiv,
LeftDiv,
Pow,
Neg,
UPlus,
Expand All @@ -112,7 +113,8 @@ impl fmt::Display for PrimitiveOp {
PrimitiveOp::Add => "Add",
PrimitiveOp::Sub => "Sub",
PrimitiveOp::Mul => "Mul",
PrimitiveOp::Div => "Div",
PrimitiveOp::RightDiv => "RightDiv",
PrimitiveOp::LeftDiv => "LeftDiv",
PrimitiveOp::Pow => "Pow",
PrimitiveOp::Neg => "Neg",
PrimitiveOp::UPlus => "UPlus",
Expand Down
4 changes: 2 additions & 2 deletions crates/runmat-accelerate/tests/fusion_patterns.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ fn stats_centered_gram_pattern() {
assert!(count_primitives(&graph, PrimitiveOp::Sub) >= 1);
assert!(count_primitives(&graph, PrimitiveOp::Transpose) >= 1);
assert!(count_primitives(&graph, PrimitiveOp::Mul) >= 1);
assert!(count_primitives(&graph, PrimitiveOp::Div) >= 1);
assert!(count_primitives(&graph, PrimitiveOp::RightDiv) >= 1);
assert!(has_builtin(&graph, "diag"));
}

Expand Down Expand Up @@ -168,7 +168,7 @@ fn monte_carlo_factor_risk_pattern() {
assert!(count_primitives(&graph, PrimitiveOp::Sub) >= 1);
assert!(count_primitives(&graph, PrimitiveOp::Transpose) >= 1);
assert!(count_primitives(&graph, PrimitiveOp::Mul) >= 2);
assert!(count_primitives(&graph, PrimitiveOp::Div) >= 1);
assert!(count_primitives(&graph, PrimitiveOp::RightDiv) >= 1);
assert!(has_builtin(&graph, "diag"));
}

Expand Down
2 changes: 1 addition & 1 deletion crates/runmat-ignition/INSTR_SET.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ Notation:
- UPlus: [+x] → [x] (object overload `uplus` if available)
- Neg: [x] → [-x] (object overload `uminus`, else numeric elementwise)
- Transpose: [V] → [V'] (calls builtin transpose)
- Add/Sub/Mul/Div/Pow: binary numeric; object overloads (`plus`, `minus`, `mtimes`, `mrdivide`, `power`) attempted first
- Add/Sub/Mul/RightDiv/LeftDiv/Pow: binary numeric or matrix ops; object overloads (`plus`, `minus`, `mtimes`, `mrdivide`, `mldivide`, `power`) attempted first
- ElemMul/ElemDiv/ElemLeftDiv/ElemPow: elementwise ops with object overloads (`times`, `rdivide`, `ldivide`, `power`)
- Equal/NotEqual/Less/LessEqual/Greater/GreaterEqual: numeric and array comparisons; object overloads (`eq`, `ne`, `lt`, `le`, `gt`, `ge`) with fallbacks; handle objects compare by identity via runtime
- AndAnd(target), OrOr(target): short-circuit variants (currently unused by the compiler; `JumpIfFalse` lowering is used instead)
Expand Down
2 changes: 1 addition & 1 deletion crates/runmat-ignition/OOP_SEMANTICS.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ Calls are performed through the `call_method` builtin with the receiver as the f
Unary and binary operators prefer object-specific methods exposed as builtins:

- Unary: `uplus`, `uminus`
- Binary arithmetic/matrix: `plus`, `minus`, `mtimes`, `mrdivide`, `power`
- Binary arithmetic/matrix: `plus`, `minus`, `mtimes`, `mrdivide`, `mldivide`, `power`
- Elementwise: `times`, `rdivide`, `ldivide`, `power`
- Relational: `eq`, `ne`, `lt`, `le`, `gt`, `ge`

Expand Down
58 changes: 52 additions & 6 deletions crates/runmat-ignition/src/accel_graph.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,8 @@ impl<'a> GraphBuilder<'a> {
Instr::Add => self.handle_binary_primitive(pc, PrimitiveOp::Add),
Instr::Sub => self.handle_binary_primitive(pc, PrimitiveOp::Sub),
Instr::Mul => self.handle_binary_primitive(pc, PrimitiveOp::Mul),
Instr::Div => self.handle_binary_primitive(pc, PrimitiveOp::Div),
Instr::RightDiv => self.handle_binary_primitive(pc, PrimitiveOp::RightDiv),
Instr::LeftDiv => self.handle_binary_primitive(pc, PrimitiveOp::LeftDiv),
Instr::Pow => self.handle_binary_primitive(pc, PrimitiveOp::Pow),
Instr::ElemMul => self.handle_binary_primitive(pc, PrimitiveOp::ElemMul),
Instr::ElemDiv => self.handle_binary_primitive(pc, PrimitiveOp::ElemDiv),
Expand Down Expand Up @@ -471,7 +472,7 @@ impl<'a> GraphBuilder<'a> {
_ => Type::Unknown,
}
}
PrimitiveOp::Div => {
PrimitiveOp::RightDiv => {
let lhs_type = self.values.get(lhs as usize).map(|v| &v.ty);
let rhs_type = self.values.get(rhs as usize).map(|v| &v.ty);
match (lhs_type, rhs_type) {
Expand All @@ -481,6 +482,16 @@ impl<'a> GraphBuilder<'a> {
_ => Type::Unknown,
}
}
PrimitiveOp::LeftDiv => {
let lhs_type = self.values.get(lhs as usize).map(|v| &v.ty);
let rhs_type = self.values.get(rhs as usize).map(|v| &v.ty);
match (lhs_type, rhs_type) {
(Some(left), Some(right)) => {
runmat_builtins::shape_rules::left_divide_output_type(left, right)
}
_ => Type::Unknown,
}
}
Comment thread
cursor[bot] marked this conversation as resolved.
_ => {
let shape = self.infer_elementwise_shape(&inputs);
if matches!(shape, ShapeInfo::Unknown) {
Expand Down Expand Up @@ -889,6 +900,7 @@ fn categorize_builtin(tags: &[AccelGraphTag]) -> AccelOpCategory {
fn primitive_category(op: PrimitiveOp) -> AccelOpCategory {
match op {
PrimitiveOp::Transpose => AccelOpCategory::Transpose,
PrimitiveOp::RightDiv | PrimitiveOp::LeftDiv => AccelOpCategory::Other,
PrimitiveOp::Less
| PrimitiveOp::LessEqual
| PrimitiveOp::Greater
Expand All @@ -898,7 +910,6 @@ fn primitive_category(op: PrimitiveOp) -> AccelOpCategory {
| PrimitiveOp::Add
| PrimitiveOp::Sub
| PrimitiveOp::Mul
| PrimitiveOp::Div
| PrimitiveOp::Pow
| PrimitiveOp::Neg
| PrimitiveOp::UPlus
Expand All @@ -912,6 +923,7 @@ fn primitive_category(op: PrimitiveOp) -> AccelOpCategory {
fn primitive_tags(op: PrimitiveOp) -> Vec<AccelGraphTag> {
match op {
PrimitiveOp::Transpose => vec![AccelGraphTag::Transpose],
PrimitiveOp::RightDiv | PrimitiveOp::LeftDiv => vec![],
PrimitiveOp::Neg | PrimitiveOp::UPlus => {
vec![AccelGraphTag::Unary, AccelGraphTag::Elementwise]
}
Expand Down Expand Up @@ -959,11 +971,11 @@ mod tests {
}

#[test]
fn accel_graph_div_uses_right_divide_shape() {
fn accel_graph_right_divide_uses_matrix_divide_shape() {
let instructions = vec![
Instr::LoadVar(0),
Instr::LoadVar(1),
Instr::Div,
Instr::RightDiv,
Instr::StoreVar(2),
];
let var_types = vec![
Expand All @@ -978,7 +990,7 @@ mod tests {
let graph = build_accel_graph(&instructions, &var_types);
let mut out_type = None;
for node in &graph.nodes {
if let AccelNodeLabel::Primitive(PrimitiveOp::Div) = node.label {
if let AccelNodeLabel::Primitive(PrimitiveOp::RightDiv) = node.label {
let out_id = node.outputs.first().copied().expect("output");
let value = graph.value(out_id).expect("value");
out_type = Some(value.ty.clone());
Expand All @@ -991,4 +1003,38 @@ mod tests {
})
);
}

#[test]
fn accel_graph_left_divide_uses_matrix_divide_shape() {
let instructions = vec![
Instr::LoadVar(0),
Instr::LoadVar(1),
Instr::LeftDiv,
Instr::StoreVar(2),
];
let var_types = vec![
Type::Tensor {
shape: Some(vec![Some(2), Some(3)]),
},
Type::Tensor {
shape: Some(vec![Some(2), Some(4)]),
},
Type::Unknown,
];
let graph = build_accel_graph(&instructions, &var_types);
let mut out_type = None;
for node in &graph.nodes {
if let AccelNodeLabel::Primitive(PrimitiveOp::LeftDiv) = node.label {
let out_id = node.outputs.first().copied().expect("output");
let value = graph.value(out_id).expect("value");
out_type = Some(value.ty.clone());
}
}
assert_eq!(
out_type,
Some(Type::Tensor {
shape: Some(vec![Some(3), Some(4)])
})
);
}
}
12 changes: 9 additions & 3 deletions crates/runmat-ignition/src/compiler.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2004,18 +2004,24 @@ impl Compiler {
BinOp::Mul => {
self.emit(Instr::Mul);
}
BinOp::Div | BinOp::LeftDiv => {
self.emit(Instr::Div);
BinOp::Div => {
self.emit(Instr::RightDiv);
}
BinOp::LeftDiv => {
self.emit(Instr::LeftDiv);
}
BinOp::Pow => {
self.emit(Instr::Pow);
}
BinOp::ElemMul => {
self.emit(Instr::ElemMul);
}
BinOp::ElemDiv | BinOp::ElemLeftDiv => {
BinOp::ElemDiv => {
self.emit(Instr::ElemDiv);
}
BinOp::ElemLeftDiv => {
self.emit(Instr::ElemLeftDiv);
}
BinOp::ElemPow => {
self.emit(Instr::ElemPow);
}
Expand Down
6 changes: 4 additions & 2 deletions crates/runmat-ignition/src/instr.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,8 @@ pub enum Instr {
Add,
Sub,
Mul,
Div,
RightDiv,
LeftDiv,
Copy link
Copy Markdown

@cursor cursor bot Mar 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LeftDiv/RightDiv not handled in turbine compiler

High Severity

Renaming Instr::Div to Instr::RightDiv and adding Instr::LeftDiv in the Instr enum was not propagated to crates/runmat-turbine/src/compiler.rs and crates/runmat-turbine/src/lib.rs, which still reference the now-nonexistent Instr::Div variant and have no handling for either RightDiv or LeftDiv. The turbine JIT compiler will fail to compile any bytecode containing these division instructions.

Fix in Cursor Fix in Web

Pow,
Neg,
UPlus,
Expand Down Expand Up @@ -217,7 +218,8 @@ impl Instr {
Instr::Add
| Instr::Sub
| Instr::Mul
| Instr::Div
| Instr::RightDiv
| Instr::LeftDiv
| Instr::Pow
| Instr::ElemMul
| Instr::ElemDiv
Expand Down
62 changes: 55 additions & 7 deletions crates/runmat-ignition/src/vm.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2467,7 +2467,7 @@ async fn run_interpreter_inner(
}
}
}
Instr::Div => {
Instr::RightDiv => {
let b = stack
.pop()
.ok_or(mex("StackUnderflow", "stack underflow"))?;
Expand All @@ -2485,8 +2485,8 @@ async fn run_interpreter_inner(
Ok(v) => stack.push(v),
Err(_) => {
let (a_acc, b_acc) =
accel_promote_binary(AutoBinaryOp::Elementwise, &a, &b).await?;
let v = call_builtin_vm!("rdivide", &[a_acc, b_acc])?;
accel_promote_binary(AutoBinaryOp::MatMul, &a, &b).await?;
let v = call_builtin_vm!("mrdivide", &[a_acc, b_acc])?;
stack.push(v)
}
}
Expand All @@ -2501,16 +2501,64 @@ async fn run_interpreter_inner(
Ok(v) => stack.push(v),
Err(_) => {
let (a_acc, b_acc) =
accel_promote_binary(AutoBinaryOp::Elementwise, &a, &b).await?;
let v = call_builtin_vm!("rdivide", &[a_acc, b_acc])?;
accel_promote_binary(AutoBinaryOp::MatMul, &a, &b).await?;
let v = call_builtin_vm!("mrdivide", &[a_acc, b_acc])?;
stack.push(v)
}
}
}
_ => {
let (a_acc, b_acc) =
accel_promote_binary(AutoBinaryOp::Elementwise, &a, &b).await?;
let v = call_builtin_vm!("rdivide", &[a_acc, b_acc])?;
accel_promote_binary(AutoBinaryOp::MatMul, &a, &b).await?;
let v = call_builtin_vm!("mrdivide", &[a_acc, b_acc])?;
stack.push(v)
}
}
}
Instr::LeftDiv => {
let b = stack
.pop()
.ok_or(mex("StackUnderflow", "stack underflow"))?;
let a = stack
.pop()
.ok_or(mex("StackUnderflow", "stack underflow"))?;
match (&a, &b) {
(Value::Object(obj), _) => {
let args = vec![
Value::Object(obj.clone()),
Value::String("mldivide".to_string()),
b.clone(),
];
match call_builtin_vm!("call_method", &args) {
Ok(v) => stack.push(v),
Err(_) => {
let (a_acc, b_acc) =
accel_promote_binary(AutoBinaryOp::MatMul, &a, &b).await?;
let v = call_builtin_vm!("mldivide", &[a_acc, b_acc])?;
stack.push(v)
}
}
}
(_, Value::Object(obj)) => {
let args = vec![
Value::Object(obj.clone()),
Value::String("mldivide".to_string()),
a.clone(),
];
match call_builtin_vm!("call_method", &args) {
Ok(v) => stack.push(v),
Err(_) => {
let (a_acc, b_acc) =
accel_promote_binary(AutoBinaryOp::MatMul, &a, &b).await?;
let v = call_builtin_vm!("mldivide", &[a_acc, b_acc])?;
stack.push(v)
}
}
}
_ => {
let (a_acc, b_acc) =
accel_promote_binary(AutoBinaryOp::MatMul, &a, &b).await?;
let v = call_builtin_vm!("mldivide", &[a_acc, b_acc])?;
stack.push(v)
}
}
Expand Down
Loading
Loading