Skip to content

Commit 9cc108d

Browse files
ShaharNavehShaharNaveh
committed
Unify BINARY_OP bytecodes
1 parent 8e0a86d commit 9cc108d

File tree

3 files changed

+162
-76
lines changed

3 files changed

+162
-76
lines changed

‎crates/codegen/src/compile.rs‎

Lines changed: 18 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -3301,7 +3301,7 @@ impl Compiler {
33013301
// Subtract to compute the correct index.
33023302
emit!(
33033303
self,
3304-
Instruction::BinaryOperation {
3304+
Instruction::BinaryOp {
33053305
op: BinaryOperator::Subtract
33063306
}
33073307
);
@@ -4339,26 +4339,24 @@ impl Compiler {
43394339
}
43404340

43414341
fn compile_op(&mut self, op: &Operator, inplace: bool) {
4342-
let op = match op {
4343-
Operator::Add => bytecode::BinaryOperator::Add,
4344-
Operator::Sub => bytecode::BinaryOperator::Subtract,
4345-
Operator::Mult => bytecode::BinaryOperator::Multiply,
4346-
Operator::MatMult => bytecode::BinaryOperator::MatrixMultiply,
4347-
Operator::Div => bytecode::BinaryOperator::Divide,
4348-
Operator::FloorDiv => bytecode::BinaryOperator::FloorDivide,
4349-
Operator::Mod => bytecode::BinaryOperator::Modulo,
4350-
Operator::Pow => bytecode::BinaryOperator::Power,
4351-
Operator::LShift => bytecode::BinaryOperator::Lshift,
4352-
Operator::RShift => bytecode::BinaryOperator::Rshift,
4353-
Operator::BitOr => bytecode::BinaryOperator::Or,
4354-
Operator::BitXor => bytecode::BinaryOperator::Xor,
4355-
Operator::BitAnd => bytecode::BinaryOperator::And,
4342+
let bin_op = match op {
4343+
Operator::Add => BinaryOperator::Add,
4344+
Operator::Sub => BinaryOperator::Subtract,
4345+
Operator::Mult => BinaryOperator::Multiply,
4346+
Operator::MatMult => BinaryOperator::MatrixMultiply,
4347+
Operator::Div => BinaryOperator::TrueDivide,
4348+
Operator::FloorDiv => BinaryOperator::FloorDivide,
4349+
Operator::Mod => BinaryOperator::Remainder,
4350+
Operator::Pow => BinaryOperator::Power,
4351+
Operator::LShift => BinaryOperator::Lshift,
4352+
Operator::RShift => BinaryOperator::Rshift,
4353+
Operator::BitOr => BinaryOperator::Or,
4354+
Operator::BitXor => BinaryOperator::Xor,
4355+
Operator::BitAnd => BinaryOperator::And,
43564356
};
4357-
if inplace {
4358-
emit!(self, Instruction::BinaryOperationInplace { op })
4359-
} else {
4360-
emit!(self, Instruction::BinaryOperation { op })
4361-
}
4357+
4358+
let op = if inplace { bin_op.as_inplace() } else { bin_op };
4359+
emit!(self, Instruction::BinaryOp { op })
43624360
}
43634361

43644362
/// Implement boolean short circuit evaluation logic.

‎crates/compiler-core/src/bytecode.rs‎

Lines changed: 126 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -594,10 +594,7 @@ pub enum Instruction {
594594
UnaryOperation {
595595
op: Arg<UnaryOperator>,
596596
},
597-
BinaryOperation {
598-
op: Arg<BinaryOperator>,
599-
},
600-
BinaryOperationInplace {
597+
BinaryOp {
601598
op: Arg<BinaryOperator>,
602599
},
603600
BinarySubscript,
@@ -1096,30 +1093,136 @@ op_arg_enum!(
10961093
/// The possible Binary operators
10971094
/// # Examples
10981095
///
1099-
/// ```ignore
1100-
/// use rustpython_compiler_core::Instruction::BinaryOperation;
1101-
/// use rustpython_compiler_core::BinaryOperator::Add;
1102-
/// let op = BinaryOperation {op: Add};
1096+
/// ```rust
1097+
/// use rustpython_compiler_core::Instruction::BinaryOp;
1098+
/// use rustpython_compiler_core::BinaryOperator::BinaryOperator;
1099+
/// let op = BinaryOp {op: BinaryOperator::Add};
11031100
/// ```
1104-
#[derive(Debug, Copy, Clone, PartialEq, Eq)]
1101+
///
1102+
/// See also:
1103+
/// - [_PyEval_BinaryOps](https://github.com/python/cpython/blob/8183fa5e3f78ca6ab862de7fb8b14f3d929421e0/Python/ceval.c#L316-L343)
11051104
#[repr(u8)]
1105+
#[derive(Clone, Copy, Debug, Eq, PartialEq)]
11061106
pub enum BinaryOperator {
1107-
Power = 0,
1108-
Multiply = 1,
1109-
MatrixMultiply = 2,
1110-
Divide = 3,
1111-
FloorDivide = 4,
1112-
Modulo = 5,
1113-
Add = 6,
1114-
Subtract = 7,
1115-
Lshift = 8,
1107+
/// `+`
1108+
Add = 0,
1109+
/// `&`
1110+
And = 1,
1111+
/// `//`
1112+
FloorDivide = 2,
1113+
/// `<<`
1114+
Lshift = 3,
1115+
/// `@`
1116+
MatrixMultiply = 4,
1117+
/// `*`
1118+
Multiply = 5,
1119+
/// `%`
1120+
Remainder = 6,
1121+
/// `|`
1122+
Or = 7,
1123+
/// `**`
1124+
Power = 8,
1125+
/// `>>`
11161126
Rshift = 9,
1117-
And = 10,
1118-
Xor = 11,
1119-
Or = 12,
1127+
/// `-`
1128+
Subtract = 10,
1129+
/// `/`
1130+
TrueDivide = 11,
1131+
/// `^`
1132+
Xor = 12,
1133+
/// `+=`
1134+
InplaceAdd = 13,
1135+
/// `&=`
1136+
InplaceAnd = 14,
1137+
/// `//=`
1138+
InplaceFloorDivide = 15,
1139+
/// `<<=`
1140+
InplaceLshift = 16,
1141+
/// `@=`
1142+
InplaceMatrixMultiply = 17,
1143+
/// `*=`
1144+
InplaceMultiply = 18,
1145+
/// `%=`
1146+
InplaceRemainder = 19,
1147+
/// `|=`
1148+
InplaceOr = 20,
1149+
/// `**=`
1150+
InplacePower = 21,
1151+
/// `>>=`
1152+
InplaceRshift = 22,
1153+
/// `-=`
1154+
InplaceSubtract = 23,
1155+
/// `/=`
1156+
InplaceTrueDivide = 24,
1157+
/// `^=`
1158+
InplaceXor = 25,
11201159
}
11211160
);
11221161

1162+
impl BinaryOperator {
1163+
/// Get the "inplace" version of the operator.
1164+
/// This has no effect if `self` is already an "inplace" operator.
1165+
///
1166+
/// # Example
1167+
/// ```rust
1168+
/// assert_eq!(BinaryOperator::Power.as_inplace(), BinaryOperator::InplacePower);
1169+
///
1170+
/// assert_eq!(BinaryOperator::InplaceSubtract.as_inplace(), BinaryOperator::InplaceSubtract);
1171+
/// ```
1172+
#[must_use]
1173+
pub const fn as_inplace(self) -> Self {
1174+
match self {
1175+
Self::Add => Self::InplaceAdd,
1176+
Self::FloorDivide => Self::InplaceFloorDivide,
1177+
Self::Lshift => Self::InplaceLshift,
1178+
Self::MatrixMultiply => Self::InplaceMatrixMultiply,
1179+
Self::Multiply => Self::InplaceMultiply,
1180+
Self::Remainder => Self::InplaceRemainder,
1181+
Self::Or => Self::InplaceOr,
1182+
Self::Power => Self::InplacePower,
1183+
Self::Rshift => Self::InplaceRshift,
1184+
Self::Subtract => Self::InplaceSubtract,
1185+
Self::TrueDivide => Self::InplaceTrueDivide,
1186+
Self::Xor => Self::InplaceXor,
1187+
_ => self,
1188+
}
1189+
}
1190+
}
1191+
1192+
impl fmt::Display for BinaryOperator {
1193+
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
1194+
let op = match self {
1195+
Self::Add => "+",
1196+
Self::And => "&",
1197+
Self::FloorDivide => "//",
1198+
Self::Lshift => "<<",
1199+
Self::MatrixMultiply => "@",
1200+
Self::Multiply => "*",
1201+
Self::Remainder => "%",
1202+
Self::Or => "|",
1203+
Self::Power => "**",
1204+
Self::Rshift => ">>",
1205+
Self::Subtract => "-",
1206+
Self::TrueDivide => "/",
1207+
Self::Xor => "^",
1208+
Self::InplaceAdd => "+=",
1209+
Self::InplaceAnd => "&=",
1210+
Self::InplaceFloorDivide => "//=",
1211+
Self::InplaceLshift => "<<=",
1212+
Self::InplaceMatrixMultiply => "@=",
1213+
Self::InplaceMultiply => "*=",
1214+
Self::InplaceRemainder => "%=",
1215+
Self::InplaceOr => "|=",
1216+
Self::InplacePower => "**=",
1217+
Self::InplaceRshift => ">>=",
1218+
Self::InplaceSubtract => "-=",
1219+
Self::InplaceTrueDivide => "/=",
1220+
Self::InplaceXor => "^=",
1221+
};
1222+
write!(f, "{op}")
1223+
}
1224+
}
1225+
11231226
op_arg_enum!(
11241227
/// The possible unary operators
11251228
#[derive(Debug, Copy, Clone, PartialEq, Eq)]
@@ -1514,7 +1617,7 @@ impl Instruction {
15141617
DeleteAttr { .. } => -1,
15151618
LoadConst { .. } => 1,
15161619
UnaryOperation { .. } => 0,
1517-
BinaryOperation { .. } | BinaryOperationInplace { .. } | CompareOperation { .. } => -1,
1620+
BinaryOp { .. } | CompareOperation { .. } => -1,
15181621
BinarySubscript => -1,
15191622
CopyItem { .. } => 1,
15201623
Pop => -1,
@@ -1719,8 +1822,7 @@ impl Instruction {
17191822
DeleteAttr { idx } => w!(DeleteAttr, name = idx),
17201823
LoadConst { idx } => fmt_const("LoadConst", arg, f, idx),
17211824
UnaryOperation { op } => w!(UnaryOperation, ?op),
1722-
BinaryOperation { op } => w!(BinaryOperation, ?op),
1723-
BinaryOperationInplace { op } => w!(BinaryOperationInplace, ?op),
1825+
BinaryOp { op } => write!(f, "BINARY_OP {}", op.get(arg)),
17241826
BinarySubscript => w!(BinarySubscript),
17251827
LoadAttr { idx } => w!(LoadAttr, name = idx),
17261828
CompareOperation { op } => w!(CompareOperation, ?op),

‎crates/vm/src/frame.rs‎

Lines changed: 18 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -868,10 +868,7 @@ impl ExecutingFrame<'_> {
868868
dict.set_item(&*key, value, vm)?;
869869
Ok(None)
870870
}
871-
bytecode::Instruction::BinaryOperation { op } => self.execute_bin_op(vm, op.get(arg)),
872-
bytecode::Instruction::BinaryOperationInplace { op } => {
873-
self.execute_bin_op_inplace(vm, op.get(arg))
874-
}
871+
bytecode::Instruction::BinaryOp { op } => self.execute_bin_op(vm, op.get(arg)),
875872
bytecode::Instruction::BinarySubscript => {
876873
let key = self.pop_value();
877874
let container = self.pop_value();
@@ -2126,40 +2123,29 @@ impl ExecutingFrame<'_> {
21262123
bytecode::BinaryOperator::Multiply => vm._mul(a_ref, b_ref),
21272124
bytecode::BinaryOperator::MatrixMultiply => vm._matmul(a_ref, b_ref),
21282125
bytecode::BinaryOperator::Power => vm._pow(a_ref, b_ref, vm.ctx.none.as_object()),
2129-
bytecode::BinaryOperator::Divide => vm._truediv(a_ref, b_ref),
2126+
bytecode::BinaryOperator::TrueDivide => vm._truediv(a_ref, b_ref),
21302127
bytecode::BinaryOperator::FloorDivide => vm._floordiv(a_ref, b_ref),
2131-
bytecode::BinaryOperator::Modulo => vm._mod(a_ref, b_ref),
2128+
bytecode::BinaryOperator::Remainder => vm._mod(a_ref, b_ref),
21322129
bytecode::BinaryOperator::Lshift => vm._lshift(a_ref, b_ref),
21332130
bytecode::BinaryOperator::Rshift => vm._rshift(a_ref, b_ref),
21342131
bytecode::BinaryOperator::Xor => vm._xor(a_ref, b_ref),
21352132
bytecode::BinaryOperator::Or => vm._or(a_ref, b_ref),
21362133
bytecode::BinaryOperator::And => vm._and(a_ref, b_ref),
2137-
}?;
2138-
2139-
self.push_value(value);
2140-
Ok(None)
2141-
}
2142-
fn execute_bin_op_inplace(
2143-
&mut self,
2144-
vm: &VirtualMachine,
2145-
op: bytecode::BinaryOperator,
2146-
) -> FrameResult {
2147-
let b_ref = &self.pop_value();
2148-
let a_ref = &self.pop_value();
2149-
let value = match op {
2150-
bytecode::BinaryOperator::Subtract => vm._isub(a_ref, b_ref),
2151-
bytecode::BinaryOperator::Add => vm._iadd(a_ref, b_ref),
2152-
bytecode::BinaryOperator::Multiply => vm._imul(a_ref, b_ref),
2153-
bytecode::BinaryOperator::MatrixMultiply => vm._imatmul(a_ref, b_ref),
2154-
bytecode::BinaryOperator::Power => vm._ipow(a_ref, b_ref, vm.ctx.none.as_object()),
2155-
bytecode::BinaryOperator::Divide => vm._itruediv(a_ref, b_ref),
2156-
bytecode::BinaryOperator::FloorDivide => vm._ifloordiv(a_ref, b_ref),
2157-
bytecode::BinaryOperator::Modulo => vm._imod(a_ref, b_ref),
2158-
bytecode::BinaryOperator::Lshift => vm._ilshift(a_ref, b_ref),
2159-
bytecode::BinaryOperator::Rshift => vm._irshift(a_ref, b_ref),
2160-
bytecode::BinaryOperator::Xor => vm._ixor(a_ref, b_ref),
2161-
bytecode::BinaryOperator::Or => vm._ior(a_ref, b_ref),
2162-
bytecode::BinaryOperator::And => vm._iand(a_ref, b_ref),
2134+
bytecode::BinaryOperator::InplaceSubtract => vm._isub(a_ref, b_ref),
2135+
bytecode::BinaryOperator::InplaceAdd => vm._iadd(a_ref, b_ref),
2136+
bytecode::BinaryOperator::InplaceMultiply => vm._imul(a_ref, b_ref),
2137+
bytecode::BinaryOperator::InplaceMatrixMultiply => vm._imatmul(a_ref, b_ref),
2138+
bytecode::BinaryOperator::InplacePower => {
2139+
vm._ipow(a_ref, b_ref, vm.ctx.none.as_object())
2140+
}
2141+
bytecode::BinaryOperator::InplaceTrueDivide => vm._itruediv(a_ref, b_ref),
2142+
bytecode::BinaryOperator::InplaceFloorDivide => vm._ifloordiv(a_ref, b_ref),
2143+
bytecode::BinaryOperator::InplaceRemainder => vm._imod(a_ref, b_ref),
2144+
bytecode::BinaryOperator::InplaceLshift => vm._ilshift(a_ref, b_ref),
2145+
bytecode::BinaryOperator::InplaceRshift => vm._irshift(a_ref, b_ref),
2146+
bytecode::BinaryOperator::InplaceXor => vm._ixor(a_ref, b_ref),
2147+
bytecode::BinaryOperator::InplaceOr => vm._ior(a_ref, b_ref),
2148+
bytecode::BinaryOperator::InplaceAnd => vm._iand(a_ref, b_ref),
21632149
}?;
21642150

21652151
self.push_value(value);

0 commit comments

Comments
 (0)