Skip to content

Commit 4880080

Browse files
committed
Converting "c >= r" or "r <= c" into "r < c + 1" if possible.
1 parent 7939672 commit 4880080

File tree

2 files changed

+71
-15
lines changed

2 files changed

+71
-15
lines changed

integration/src/const_collapse.rs

Lines changed: 47 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -47,17 +47,19 @@ pub fn collapse_const_if_possible(op: &Operator, inputs: &[MaybeConstant]) {
4747
{
4848
// Right operand is constant and can be immediate
4949
must_collapse.replace(true);
50-
} else if is_commutative(op)
51-
&& let [
50+
} else if let [
5251
MaybeConstant::ReferenceConstant {
5352
value,
5453
must_collapse,
5554
},
5655
MaybeConstant::NonConstant,
5756
] = inputs
58-
&& can_be_i16(value)
57+
&& ((is_commutative(op)
58+
&& can_be_i16(value)) || can_turn_to_lt(op, value).is_some())
5959
{
60-
// Left operand is constant and can be immediate (commutative operations only)
60+
// Left operand is constant and can be immediate
61+
// (either the operation is commutative, or it's
62+
// a GE that can be transformed to LT)
6163
must_collapse.replace(true);
6264
}
6365
}
@@ -98,6 +100,18 @@ pub fn collapse_const_if_possible(op: &Operator, inputs: &[MaybeConstant]) {
98100
{
99101
// Left operand is constant and can be immediate
100102
must_collapse.replace(true);
103+
} else if let [
104+
_,
105+
MaybeConstant::ReferenceConstant {
106+
value,
107+
must_collapse,
108+
},
109+
] = inputs
110+
&& can_turn_to_lt(op, value).is_some()
111+
{
112+
// The constant is on the right, but we can turn the LE into an LT,
113+
// so we can use the left operand as immediate.
114+
must_collapse.replace(true);
101115
}
102116
}
103117

@@ -136,6 +150,35 @@ fn is_commutative(op: &Operator) -> bool {
136150
)
137151
}
138152

153+
/// If op is a GE or LE, can we turn "c >= x" or "x <= c" into "x < c + 1", where c + 1 fits in i16?
154+
pub fn can_turn_to_lt(op: &Operator, value: &WasmValue) -> Option<i16> {
155+
match op {
156+
// Signed case:
157+
Operator::I32GeS | Operator::I64GeS | Operator::I32LeS | Operator::I64LeS => match value {
158+
WasmValue::I32(v) => v.checked_add(1).and_then(|v| i16::try_from(v).ok()),
159+
WasmValue::I64(v) => v.checked_add(1).and_then(|v| i16::try_from(v).ok()),
160+
_ => None,
161+
},
162+
// Unsigned case:
163+
// This is weird, because we need to represent an unsigned value as a sign-extended i16.
164+
// So, we can represent (c+1) if it falls in the ranges:
165+
// [0..0x7FFF] or [0xFFFF8000..0xFFFFFFFF], for u32
166+
// [0..0x7FFF] or [0xFFFFFFFFFFFF8000..0xFFFFFFFFFFFFFFFF], for u64
167+
Operator::I32GeU | Operator::I64GeU | Operator::I32LeU | Operator::I64LeU => match value {
168+
WasmValue::I32(v) => {
169+
let uv = *v as u32;
170+
uv.checked_add(1).and_then(|v| i16::try_from(v as i32).ok())
171+
}
172+
WasmValue::I64(v) => {
173+
let uv = *v as u64;
174+
uv.checked_add(1).and_then(|v| i16::try_from(v as i64).ok())
175+
}
176+
_ => None,
177+
},
178+
_ => None,
179+
}
180+
}
181+
139182
fn can_be_i16(value: &WasmValue) -> bool {
140183
match value {
141184
WasmValue::I32(v) => i16::try_from(*v).is_ok(),

integration/src/womir_translation.rs

Lines changed: 24 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
use std::{collections::HashMap, ops::Range, vec};
22

3-
use crate::{instruction_builder as ib, to_field::ToField};
3+
use crate::{const_collapse::can_turn_to_lt, instruction_builder as ib, to_field::ToField};
44
use arrayvec::ArrayVec;
55
use itertools::Itertools;
66
use openvm_circuit::{
@@ -888,23 +888,32 @@ impl<'a, F: PrimeField32> Settings<'a> for OpenVMSettings<F> {
888888
}
889889
[WasmOpInput::Register(reg), WasmOpInput::Constant(c)]
890890
| [WasmOpInput::Constant(c), WasmOpInput::Register(reg)] => {
891-
// This is a little confusing, because GE and LE are using the same opcode,
892-
// but this is correct, as the operands are reversed between GE and LE.
893-
// Const folding step guarantees that only GE will have the constant on
894-
// the right side, and only LE will have the constant on the left side.
895-
//
896-
// In another words: the order of the operands will guarantee that the inverse
897-
// operation is always a less-than comparison: reg_value < constant_value.
898-
let inverse_op = match op {
891+
let opcode = match op {
899892
Op::I32GeS | Op::I32LeS => LessThanOpcode::SLT.global_opcode(),
900893
Op::I32GeU | Op::I32LeU => LessThanOpcode::SLTU.global_opcode(),
901894
Op::I64GeS | Op::I64LeS => LessThan64Opcode::SLT.global_opcode(),
902895
Op::I64GeU | Op::I64LeU => LessThan64Opcode::SLTU.global_opcode(),
903896
_ => unreachable!(),
904897
};
905898

899+
// Check whether this is the case where "c >= r" or "r <= c" can be turned into "r < c + 1".
900+
if ((matches!(op, Op::I32GeS | Op::I32GeU | Op::I64GeS | Op::I64GeU)
901+
&& matches!(&inputs[0], WasmOpInput::Constant(_)))
902+
|| (matches!(op, Op::I32LeS | Op::I32LeU | Op::I64LeS | Op::I64LeU)
903+
&& matches!(&inputs[0], WasmOpInput::Register(_))))
904+
&& let Some(inc_c) = can_turn_to_lt(&op, c)
905+
{
906+
return Directive::Instruction(ib::instr_i(
907+
opcode.as_usize(),
908+
output,
909+
reg.start as usize,
910+
i16_to_imm_field(inc_c),
911+
))
912+
.into();
913+
}
914+
906915
let c = const_i16_as_field(c);
907-
ib::instr_i(inverse_op.as_usize(), inverse_result, reg.start as usize, c)
916+
ib::instr_i(opcode.as_usize(), inverse_result, reg.start as usize, c)
908917
}
909918
_ => unreachable!("combination of inputs not possible for GE/LE operations"),
910919
};
@@ -2155,7 +2164,11 @@ fn const_i16_as_field<F: PrimeField32>(value: &WasmValue) -> F {
21552164
WasmValue::I64(v) => v as i16,
21562165
_ => panic!("not valid i16"),
21572166
};
2158-
F::from_canonical_u32((c as i32 as u32) & 0xff_ff_ff)
2167+
i16_to_imm_field(c)
2168+
}
2169+
2170+
fn i16_to_imm_field<F: PrimeField32>(value: i16) -> F {
2171+
F::from_canonical_u32((value as i32 as u32) & 0xff_ff_ff)
21592172
}
21602173

21612174
fn mem_offset<F: PrimeField32>(memarg: MemArg, c: &Ctx<F>) -> i32 {

0 commit comments

Comments
 (0)