Skip to content

Commit

Permalink
fix: Make constant_to_radix emit a slice instead of an array (#4049)
Browse files Browse the repository at this point in the history
# Description

The compiler optimizes uses of the `ToRadix` intrinsic applied to
constants by precomputing the results.

## Problem\*

Resolves #4048

The optimization produces a fixed sized array instead of a slice.

## Summary\*

The PR changes the `constant_to_radix` function to produce a slice and
generates a tuple with the length and the slice itself, which fits the
internal representation of slices in SSA.

## Additional Context

This was found and fixed during the implementation of the debugger (see
#3015).

## Documentation\*

Check one:
- [X] No documentation needed.
- [ ] Documentation included in this PR.
- [ ] **[Exceptional Case]** Documentation to be submitted in a separate
PR.

# PR Checklist\*

- [X] I have tested the changes locally.
- [X] I have formatted the changes with [Prettier](https://prettier.io/)
and/or `cargo fmt` on default settings.
  • Loading branch information
ggiraldez committed Jan 16, 2024
1 parent f8a1fb7 commit 5cdb1d0
Showing 1 changed file with 19 additions and 18 deletions.
37 changes: 19 additions & 18 deletions compiler/noirc_evaluator/src/ssa/ir/instruction/call.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,13 +47,8 @@ pub(super) fn simplify_call(
let field = constant_args[0];
let limb_count = constant_args[1].to_u128() as u32;

let result_slice = constant_to_radix(endian, field, 2, limb_count, dfg);

let length = dfg
.try_get_array_length(result_slice)
.expect("ICE: a constant array should have an associated length");
let len_value =
dfg.make_constant(FieldElement::from(length as u128), Type::field());
let (len_value, result_slice) =
constant_to_radix(endian, field, 2, limb_count, dfg);

// `Intrinsic::ToBits` returns slices which are represented
// by tuples with the structure (length, slice contents)
Expand All @@ -68,13 +63,8 @@ pub(super) fn simplify_call(
let radix = constant_args[1].to_u128() as u32;
let limb_count = constant_args[2].to_u128() as u32;

let result_slice = constant_to_radix(endian, field, radix, limb_count, dfg);

let length = dfg
.try_get_array_length(result_slice)
.expect("ICE: a constant array should have an associated length");
let len_value =
dfg.make_constant(FieldElement::from(length as u128), Type::field());
let (len_value, result_slice) =
constant_to_radix(endian, field, radix, limb_count, dfg);

// `Intrinsic::ToRadix` returns slices which are represented
// by tuples with the structure (length, slice contents)
Expand Down Expand Up @@ -466,14 +456,26 @@ fn make_constant_array(dfg: &mut DataFlowGraph, results: Vec<FieldElement>, typ:
dfg.make_array(result_constants.into(), typ)
}

/// Returns a Value::Array of constants corresponding to the limbs of the radix decomposition.
fn make_constant_slice(
dfg: &mut DataFlowGraph,
results: Vec<FieldElement>,
typ: Type,
) -> (ValueId, ValueId) {
let result_constants = vecmap(results, |element| dfg.make_constant(element, typ.clone()));

let typ = Type::Slice(Rc::new(vec![typ]));
let length = FieldElement::from(result_constants.len() as u128);
(dfg.make_constant(length, Type::field()), dfg.make_array(result_constants.into(), typ))
}

/// Returns a slice (represented by a tuple (len, slice)) of constants corresponding to the limbs of the radix decomposition.
fn constant_to_radix(
endian: Endian,
field: FieldElement,
radix: u32,
limb_count: u32,
dfg: &mut DataFlowGraph,
) -> ValueId {
) -> (ValueId, ValueId) {
let bit_size = u32::BITS - (radix - 1).leading_zeros();
let radix_big = BigUint::from(radix);
assert_eq!(BigUint::from(2u128).pow(bit_size), radix_big, "ICE: Radix must be a power of 2");
Expand All @@ -488,8 +490,7 @@ fn constant_to_radix(
if endian == Endian::Big {
limbs.reverse();
}

make_constant_array(dfg, limbs, Type::unsigned(bit_size))
make_constant_slice(dfg, limbs, Type::unsigned(bit_size))
}

fn to_u8_vec(dfg: &DataFlowGraph, values: im::Vector<Id<Value>>) -> Vec<u8> {
Expand Down

0 comments on commit 5cdb1d0

Please sign in to comment.