refactor: share comparison-predicate selection across int libfuncs (#1627)

`int_range_try_new`, `int_range_pop_front`, and `bounded_int_constrain`
each picked between signed/unsigned cmpi predicates with near-identical
inline logic. Lift it into a `compare_predicate` helper on
`src/libfuncs.rs` that takes the operand's `CoreTypeConcrete`, the
registry, and the (signed, unsigned) predicate pair, and routes to
whichever variant matches the stored representation: `i8..i128` is
two's complement, while `u*` and `BoundedInt<L, U>` are stored as
non-negative integers (the latter biased to start at zero regardless
of `L`'s sign).

Adds a VM-equivalence test (`int_range_try_new_bounded_int_negative_lower_vm_equivalence`)
over `BoundedInt<-10, 10>` covering 7 input pairs around the 5-bit MSB
threshold.

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Author: TomerStarkware

src/libfuncs.rs

@@ -26,7 +26,10 @@ use cairo_lang_sierra::{
 };
 use itertools::Itertools;
 use melior::{
-    dialect::{arith, cf, llvm, ods},
+    dialect::{
+        arith::{self},
+        cf, llvm, ods,
+    },
     helpers::{ArithBlockExt, BuiltinBlockExt, LlvmBlockExt},
     ir::{
         attribute::{FlatSymbolRefAttribute, StringAttribute, TypeAttribute},
@@ -546,6 +549,19 @@ fn increment_builtin_counter_conditionally_by<'ctx: 'a, 'a>(
     ))?)
 }
 
+/// Whether the operand's stored representation is two's-complement signed,
+/// so callers know to pick the signed `cmpi` variant over the unsigned one.
+///
+/// Plain Sierra signed ints (`i8`..`i128`) live as two's complement.
+/// Everything else — `u*` and `BoundedInt<L, U>`, which is biased to start at
+/// zero regardless of `L`'s sign — is stored as a non-negative integer.
+fn is_signed_repr(
+    ty: &CoreTypeConcrete,
+    registry: &ProgramRegistry<CoreType, CoreLibfunc>,
+) -> Result<bool> {
+    Ok(ty.integer_range(registry)?.lower < BigInt::ZERO && !ty.is_bounded_int(registry)?)
+}
+
 fn build_noop<'ctx, 'this, const N: usize, const PROCESS_BUILTINS: bool>(
     context: &'ctx Context,
     registry: &ProgramRegistry<CoreType, CoreLibfunc>,

src/libfuncs/bounded_int.rs

@@ -573,12 +573,11 @@ fn build_constrain<'ctx, 'this>(
         src_value.r#type(),
     )?;
 
-    let cmpi_predicate =
-        if src.ty.is_bounded_int(registry)? || src.range.lower.sign() != Sign::Minus {
-            CmpiPredicate::Ult
-        } else {
-            CmpiPredicate::Slt
-        };
+    let cmpi_predicate = if super::is_signed_repr(src.ty, registry)? {
+        CmpiPredicate::Slt
+    } else {
+        CmpiPredicate::Ult
+    };
     let is_lower = entry.cmpi(context, cmpi_predicate, src_value, boundary, location)?;
 
     let lower_block = helper.append_block(Block::new(&[]));

src/libfuncs/int_range.rs

@@ -1,9 +1,7 @@
 //! # Int range libfuncs
 
 use super::LibfuncHelper;
-use crate::{
-    error::Result, metadata::MetadataStorage, types::TypeBuilder, utils::ProgramRegistryExt,
-};
+use crate::{error::Result, metadata::MetadataStorage, utils::ProgramRegistryExt};
 use cairo_lang_sierra::{
     extensions::{
         core::{CoreLibfunc, CoreType},
@@ -22,7 +20,6 @@ use melior::{
     ir::{Block, Location},
     Context,
 };
-use num_bigint::BigInt;
 
 /// Select and call the correct libfunc builder function from the selector.
 pub fn build<'ctx, 'this>(
@@ -66,14 +63,12 @@ pub fn build_int_range_try_new<'ctx, 'this>(
         &info.branch_signatures()[0].vars[1].ty,
     )?;
     let inner = registry.get_type(&info.param_signatures()[1].ty)?;
-    // to know if it is signed
-    let inner_range = inner.integer_range(registry)?;
-
-    let is_valid = if inner_range.lower < BigInt::ZERO {
-        entry.cmpi(context, CmpiPredicate::Sle, x, y, location)?
+    let predicate = if super::is_signed_repr(inner, registry)? {
+        CmpiPredicate::Sle
     } else {
-        entry.cmpi(context, CmpiPredicate::Ule, x, y, location)?
+        CmpiPredicate::Ule
     };
+    let is_valid = entry.cmpi(context, predicate, x, y, location)?;
 
     let range =
         entry.append_op_result(ods::llvm::mlir_undef(context, range_ty, location).into())?;
@@ -118,14 +113,12 @@ pub fn build_int_range_pop_front<'ctx, 'this>(
     let x_p_1 = entry.addi(x, k1, location)?;
     let y = entry.extract_value(context, location, range, inner_ty, 1)?;
 
-    // to know if it is signed
-    let inner_range = inner.integer_range(registry)?;
-
-    let is_valid = if inner_range.lower < BigInt::ZERO {
-        entry.cmpi(context, CmpiPredicate::Slt, x, y, location)?
+    let predicate = if super::is_signed_repr(inner, registry)? {
+        CmpiPredicate::Slt
     } else {
-        entry.cmpi(context, CmpiPredicate::Ult, x, y, location)?
+        CmpiPredicate::Ult
     };
+    let is_valid = entry.cmpi(context, predicate, x, y, location)?;
     let range = entry.insert_value(context, location, range, x_p_1, 0)?;
 
     helper.cond_br(

test_data/programs/libfuncs/int_range_try_new_bounded_int.cairo

@@ -0,0 +1,18 @@
+#[feature("bounded-int-utils")]
+use core::internal::bounded_int::BoundedInt;
+
+pub extern type IntRange<T>;
+impl IntRangeDrop<T> of Drop<IntRange<T>>;
+
+pub extern fn int_range_try_new<T>(
+    x: T, y: T,
+) -> Result<IntRange<T>, IntRange<T>> implicits(core::RangeCheck) nopanic;
+
+fn run_test(lhs: felt252, rhs: felt252) -> bool {
+    let lhs: BoundedInt<-10, 10> = lhs.try_into().unwrap();
+    let rhs: BoundedInt<-10, 10> = rhs.try_into().unwrap();
+    match int_range_try_new(lhs, rhs) {
+        Result::Ok(_) => true,
+        Result::Err(_) => false,
+    }
+}

tests/tests/int_range.rs

@@ -0,0 +1,50 @@
+use crate::common::{compare_outputs, run_native_program, run_vm_program, DEFAULT_GAS};
+use cairo_lang_runner::Arg;
+use cairo_native::starknet::DummySyscallHandler;
+use cairo_native::utils::testing::load_program_and_runner;
+use cairo_native::Value;
+use starknet_types_core::felt::Felt;
+use test_case::test_case;
+
+// `int_range_try_new` over `BoundedInt<-10, 10>` (5-bit biased representation,
+// bias = +10) must compare biased values with an unsigned predicate. A signed
+// predicate misreads any biased value with MSB set as negative; e.g. `(10, 0)`
+// -> biased `(20, 10)` is read as `(-12, 10)` and the invalid range is
+// incorrectly accepted, while `(-10, 10)` -> biased `(0, 20)` is read as
+// `(0, -12)` and the valid range is rejected.
+#[test_case(0, 5; "valid_zero_to_five")]
+#[test_case(-10, -5; "valid_neg_ten_to_neg_five")]
+#[test_case(-10, 10; "valid_full_range")]
+#[test_case(10, 10; "valid_empty_at_upper")]
+#[test_case(0, -5; "invalid_descending_zero")]
+#[test_case(10, 0; "invalid_msb_biased_lhs")]
+#[test_case(10, -10; "invalid_max_to_min")]
+fn int_range_try_new_bounded_int_negative_lower_vm_equivalence(lhs: i64, rhs: i64) {
+    let program = &load_program_and_runner("programs/libfuncs/int_range_try_new_bounded_int");
+
+    let result_vm = run_vm_program(
+        program,
+        "run_test",
+        vec![Arg::Value(Felt::from(lhs)), Arg::Value(Felt::from(rhs))],
+        Some(DEFAULT_GAS as usize),
+    )
+    .unwrap();
+    let result_native = run_native_program(
+        program,
+        "run_test",
+        &[
+            Value::Felt252(Felt::from(lhs)),
+            Value::Felt252(Felt::from(rhs)),
+        ],
+        Some(DEFAULT_GAS),
+        Option::<DummySyscallHandler>::None,
+    );
+
+    compare_outputs(
+        &program.1,
+        &program.2.find_function("run_test").unwrap().id,
+        &result_vm,
+        &result_native,
+    )
+    .unwrap();
+}

tests/tests/mod.rs

@@ -8,6 +8,7 @@ pub mod dict;
 pub mod e2e_libfuncs;
 pub mod ec;
 pub mod felt252;
+pub mod int_range;
 pub mod libfuncs;
 pub mod programs;
 pub mod result;