Skip to content

Commit 8a46716

Browse files
committed
revert interface change, and check the limbs number
1 parent 00cdeab commit 8a46716

5 files changed

Lines changed: 64 additions & 41 deletions

File tree

acvm-repo/brillig/src/black_box.rs

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -50,19 +50,19 @@ pub enum BlackBoxOp {
5050
Sha256Compression { input: HeapArray, hash_values: HeapArray, output: HeapArray },
5151
/// Returns a decomposition in `num_limbs` limbs of the given input over the given radix.
5252
///
53-
/// - The value stored at `input` must be a `Field`.
54-
/// - The value stored at `radix` must be a `U32` in the range [2, 256].
55-
/// - `num_limbs` must be at least one if the value stored at `input` is not zero.
56-
/// - `output_bits` specifies whether we should decompose into bits.
57-
/// The value stored at `radix` must be two if `output_bits` is true.
53+
/// - The value stored in `radix` must be in the range [2, 256]
54+
/// - `num_limbs` must be at least one if the value stored in `input` is not zero.
55+
/// - The value stored in `output_bits` must have a `bit_size` of one.
56+
/// That value specifies whether we should decompose into bits. The value stored in
57+
/// the `radix` address must be two if the value stored in `output_bits` is equal to one.
5858
///
5959
/// Native to the Brillig VM and not supported as an ACIR black box function.
6060
ToRadix {
6161
input: MemoryAddress,
6262
radix: MemoryAddress,
6363
output_pointer: MemoryAddress,
64-
num_limbs: u32,
65-
output_bits: bool,
64+
num_limbs: MemoryAddress,
65+
output_bits: MemoryAddress,
6666
},
6767
}
6868

acvm-repo/brillig_vm/src/black_box.rs

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -297,8 +297,12 @@ pub(crate) fn evaluate_black_box<F: AcirField, Solver: BlackBoxFunctionSolver<F>
297297
let MemoryValue::U32(radix) = memory.read(*radix) else {
298298
panic!("ToRadix opcode's radix bit size does not match expected bit size 32")
299299
};
300+
let num_limbs = memory.read(*num_limbs).to_u32();
301+
let MemoryValue::U1(output_bits) = memory.read(*output_bits) else {
302+
panic!("ToRadix opcode's output_bits size does not match expected bit size 1")
303+
};
300304

301-
let output = to_be_radix(input, radix, assert_usize(*num_limbs), *output_bits)?;
305+
let output = to_be_radix(input, radix, assert_usize(num_limbs), output_bits)?;
302306

303307
memory.write_slice(memory.read_ref(*output_pointer), &output);
304308

compiler/noirc_evaluator/src/brillig/brillig_check.rs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -608,9 +608,11 @@ trait OpcodeAddressVisitor {
608608
self.read_heap_array(hash_values, location);
609609
self.write_heap_array(output, location);
610610
}
611-
BlackBoxOp::ToRadix { input, radix, output_pointer, num_limbs: _, output_bits: _ } => {
611+
BlackBoxOp::ToRadix { input, radix, output_pointer, num_limbs, output_bits } => {
612612
self.read(input, location);
613613
self.read(radix, location);
614+
self.read(num_limbs, location);
615+
self.read(output_bits, location);
614616
self.read(output_pointer, location); // indirect
615617
}
616618
}

compiler/noirc_evaluator/src/brillig/brillig_gen/tests/call.rs

Lines changed: 41 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -136,21 +136,37 @@ fn brillig_to_bits() {
136136
assert_artifact_snapshot!(foo, @r"
137137
fn foo
138138
0: sp[4] = const u32 2
139-
1: sp[3] = @1
140-
2: sp[5] = const u32 9
141-
3: @1 = u32 add @1, sp[5]
142-
4: sp[3] = indirect const u32 1
143-
5: sp[5] = u32 add sp[3], @2
144-
6: sp[6] = const u32 8
145-
7: to_radix(input: sp[2], radix: sp[4], num_limbs: 8, output_pointer: sp[5], output_bits: true)
146-
8: @3 = sp[5]
147-
9: @4 = sp[6]
148-
10: call 0 // -> ArrayReverse
149-
11: sp[2] = sp[3]
150-
12: return
139+
1: sp[5] = const bool 1
140+
2: sp[3] = @1
141+
3: sp[6] = const u32 9
142+
4: @1 = u32 add @1, sp[6]
143+
5: sp[3] = indirect const u32 1
144+
6: sp[6] = u32 add sp[3], @2
145+
7: sp[7] = const u32 8
146+
8: to_radix(input: sp[2], radix: sp[4], num_limbs: sp[7], output_pointer: sp[6], output_bits: sp[5])
147+
9: @3 = sp[6]
148+
10: @4 = sp[7]
149+
11: call 0 // -> ArrayReverse
150+
12: sp[2] = sp[3]
151+
13: return
151152
");
152153
}
153154

155+
// `to_le_bits::<N>` requesting more limbs than the field is rejected at compile time.
156+
#[test]
157+
#[should_panic(expected = "ToRadix num_limbs")]
158+
fn brillig_to_bits_rejects_more_limbs_than_field_bits() {
159+
let src = "
160+
brillig(inline) fn foo f0 {
161+
b0(v0: Field):
162+
v1 = call to_le_bits(v0) -> [u1; 255]
163+
return v1
164+
}
165+
";
166+
167+
let _ = ssa_to_brillig_artifacts(src);
168+
}
169+
154170
// Tests ToRadix intrinsic code-gen for Brillig.
155171
#[test]
156172
fn brillig_to_radix() {
@@ -166,18 +182,19 @@ fn brillig_to_radix() {
166182
let foo = &brillig.ssa_function_to_brillig[&Id::test_new(0)];
167183
assert_artifact_snapshot!(foo, @r"
168184
fn foo
169-
0: sp[4] = @1
170-
1: sp[5] = const u32 9
171-
2: @1 = u32 add @1, sp[5]
172-
3: sp[4] = indirect const u32 1
173-
4: sp[5] = u32 add sp[4], @2
174-
5: sp[6] = const u32 8
175-
6: to_radix(input: sp[2], radix: sp[3], num_limbs: 8, output_pointer: sp[5], output_bits: false)
176-
7: @3 = sp[5]
177-
8: @4 = sp[6]
178-
9: call 0 // -> ArrayReverse
179-
10: sp[2] = sp[4]
180-
11: return
185+
0: sp[5] = const bool 0
186+
1: sp[4] = @1
187+
2: sp[6] = const u32 9
188+
3: @1 = u32 add @1, sp[6]
189+
4: sp[4] = indirect const u32 1
190+
5: sp[6] = u32 add sp[4], @2
191+
6: sp[7] = const u32 8
192+
7: to_radix(input: sp[2], radix: sp[3], num_limbs: sp[7], output_pointer: sp[6], output_bits: sp[5])
193+
8: @3 = sp[6]
194+
9: @4 = sp[7]
195+
10: call 0 // -> ArrayReverse
196+
11: sp[2] = sp[4]
197+
12: return
181198
");
182199
}
183200

compiler/noirc_evaluator/src/brillig/brillig_ir/codegen_intrinsic.rs

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -78,29 +78,29 @@ impl<F: AcirField + DebugToString, Registers: RegisterAllocator> BrilligContext<
7878
) {
7979
assert!(source_field.bit_size == F::max_num_bits());
8080
assert!(radix.bit_size == 32);
81-
82-
let num_limbs: u32 = target_array.size.0;
8381
assert!(
84-
num_limbs <= F::max_num_bits(),
85-
"ToRadix num_limbs ({num_limbs}) exceeds the maximum useful number of limbs ({}) for this field",
82+
target_array.size.0 <= F::max_num_bits(),
83+
"ToRadix num_limbs ({}) exceeds the maximum useful number of limbs ({}) for this field",
84+
target_array.size.0,
8685
F::max_num_bits()
8786
);
8887

88+
let bits_register = self.make_constant_instruction(output_bits.into(), 1);
8989
self.codegen_initialize_array(target_array);
9090
let pointer = self.codegen_make_array_items_pointer(target_array);
91-
let num_limbs_register = self.make_usize_constant_instruction(num_limbs.into());
91+
let num_limbs = self.make_usize_constant_instruction(target_array.size.0.into());
9292

9393
// Perform big-endian ToRadix
9494
self.black_box_op_instruction(BlackBoxOp::ToRadix {
9595
input: source_field.address,
9696
radix: radix.address,
9797
output_pointer: *pointer,
98-
num_limbs,
99-
output_bits,
98+
num_limbs: num_limbs.address,
99+
output_bits: bits_register.address,
100100
});
101101

102102
if little_endian {
103-
self.codegen_array_reverse(*pointer, num_limbs_register.address);
103+
self.codegen_array_reverse(*pointer, num_limbs.address);
104104
}
105105
}
106106
}

0 commit comments

Comments
 (0)