chore(avm): radix opcode - remove immediates (#10696)

Resolves #10371

avm-transpiler/src/transpile.rs

@@ -1064,29 +1064,30 @@ fn handle_black_box_function(avm_instrs: &mut Vec<AvmInstruction>, operation: &B
                 ..Default::default()
             });
         }
-        BlackBoxOp::ToRadix { input, radix, output, output_bits } => {
-            let num_limbs = output.size as u32;
+        BlackBoxOp::ToRadix { input, radix, output_pointer, num_limbs, output_bits } => {
             let input_offset = input.to_usize() as u32;
-            let output_offset = output.pointer.to_usize() as u32;
             let radix_offset = radix.to_usize() as u32;
+            let output_offset = output_pointer.to_usize() as u32;
+            let num_limbs_offset = num_limbs.to_usize() as u32;
+            let output_bits_offset = output_bits.to_usize() as u32;
 
             avm_instrs.push(AvmInstruction {
                 opcode: AvmOpcode::TORADIXBE,
                 indirect: Some(
                     AddressingModeBuilder::default()
                         .direct_operand(input)
-                        .indirect_operand(&output.pointer)
                         .direct_operand(radix)
+                        .direct_operand(num_limbs)
+                        .direct_operand(output_bits)
+                        .indirect_operand(output_pointer)
                         .build(),
                 ),
                 operands: vec![
                     AvmOperand::U16 { value: input_offset as u16 },
-                    AvmOperand::U16 { value: output_offset as u16 },
                     AvmOperand::U16 { value: radix_offset as u16 },
-                ],
-                immediates: vec![
-                    AvmOperand::U16 { value: num_limbs as u16 },
-                    AvmOperand::U8 { value: *output_bits as u8 },
+                    AvmOperand::U16 { value: num_limbs_offset as u16 },
+                    AvmOperand::U16 { value: output_bits_offset as u16 },
+                    AvmOperand::U16 { value: output_offset as u16 },
                 ],
                 ..Default::default()
             });

barretenberg/cpp/src/barretenberg/dsl/acir_format/serde/acir.hpp

@@ -424,8 +424,9 @@ struct BlackBoxOp {
     struct ToRadix {
         Program::MemoryAddress input;
         Program::MemoryAddress radix;
-        Program::HeapArray output;
-        bool output_bits;
+        Program::MemoryAddress output_pointer;
+        Program::MemoryAddress num_limbs;
+        Program::MemoryAddress output_bits;
 
         friend bool operator==(const ToRadix&, const ToRadix&);
         std::vector<uint8_t> bincodeSerialize() const;
@@ -4611,7 +4612,10 @@ inline bool operator==(const BlackBoxOp::ToRadix& lhs, const BlackBoxOp::ToRadix
     if (!(lhs.radix == rhs.radix)) {
         return false;
     }
-    if (!(lhs.output == rhs.output)) {
+    if (!(lhs.output_pointer == rhs.output_pointer)) {
+        return false;
+    }
+    if (!(lhs.num_limbs == rhs.num_limbs)) {
         return false;
     }
     if (!(lhs.output_bits == rhs.output_bits)) {
@@ -4646,7 +4650,8 @@ void serde::Serializable<Program::BlackBoxOp::ToRadix>::serialize(const Program:
 {
     serde::Serializable<decltype(obj.input)>::serialize(obj.input, serializer);
     serde::Serializable<decltype(obj.radix)>::serialize(obj.radix, serializer);
-    serde::Serializable<decltype(obj.output)>::serialize(obj.output, serializer);
+    serde::Serializable<decltype(obj.output_pointer)>::serialize(obj.output_pointer, serializer);
+    serde::Serializable<decltype(obj.num_limbs)>::serialize(obj.num_limbs, serializer);
     serde::Serializable<decltype(obj.output_bits)>::serialize(obj.output_bits, serializer);
 }
 
@@ -4658,7 +4663,8 @@ Program::BlackBoxOp::ToRadix serde::Deserializable<Program::BlackBoxOp::ToRadix>
     Program::BlackBoxOp::ToRadix obj;
     obj.input = serde::Deserializable<decltype(obj.input)>::deserialize(deserializer);
     obj.radix = serde::Deserializable<decltype(obj.radix)>::deserialize(deserializer);
-    obj.output = serde::Deserializable<decltype(obj.output)>::deserialize(deserializer);
+    obj.output_pointer = serde::Deserializable<decltype(obj.output_pointer)>::deserialize(deserializer);
+    obj.num_limbs = serde::Deserializable<decltype(obj.num_limbs)>::deserialize(deserializer);
     obj.output_bits = serde::Deserializable<decltype(obj.output_bits)>::deserialize(deserializer);
     return obj;
 }

barretenberg/cpp/src/barretenberg/vm/avm/tests/execution.test.cpp

@@ -130,6 +130,8 @@ class AvmExecutionTests : public ::testing::Test {
     }
 };
 
+class AvmExecutionTestsToRadix : public AvmExecutionTests, public testing::WithParamInterface<bool> {};
+
 // Basic positive test with an ADD and RETURN opcode.
 // Parsing, trace generation and proving is verified.
 TEST_F(AvmExecutionTests, basicAddReturn)
@@ -854,85 +856,15 @@ TEST_F(AvmExecutionTests, setAndCastOpcodes)
     validate_trace(std::move(trace), public_inputs);
 }
 
-// Positive test with TO_RADIX_BE.
-TEST_F(AvmExecutionTests, toRadixBeOpcodeBytes)
-{
-    std::string bytecode_hex =
-        to_hex(OpCode::SET_8) +            // opcode SET
-        "00"                               // Indirect flag
-        "00"                               // dst_offset
-        + to_hex(AvmMemoryTag::U32) + "00" // val
-        + to_hex(OpCode::SET_8) +          // opcode SET
-        "00"                               // Indirect flag
-        "01"                               // dst_offset
-        + to_hex(AvmMemoryTag::U32) + "01" // val
-        + to_hex(OpCode::CALLDATACOPY) +   // opcode CALLDATACOPY
-        "00"                               // Indirect flag
-        "0000"                             // cd_offset
-        "0001"                             // copy_size
-        "0001"                             // dst_offset
-        + to_hex(OpCode::SET_8) +          // opcode SET for indirect src
-        "00"                               // Indirect flag
-        "11"                               // dst_offset 17
-        + to_hex(AvmMemoryTag::U32) + "01" // value 1 (i.e. where the src from calldata is copied)
-        + to_hex(OpCode::SET_8) +          // opcode SET for indirect dst
-        "00"                               // Indirect flag
-        "15"                               // dst_offset 21
-        + to_hex(AvmMemoryTag::U32) + "05" // value 5 (i.e. where the dst will be written to)
-        + to_hex(OpCode::SET_8) +          // opcode SET for indirect dst
-        "00"                               // Indirect flag
-        "80"                               // radix_offset 80
-        + to_hex(AvmMemoryTag::U32) + "02" // value 2 (i.e. radix 2 - perform bitwise decomposition)
-        + to_hex(OpCode::TORADIXBE) +      // opcode TO_RADIX_BE
-        "03"                               // Indirect flag
-        "0011"                             // src_offset 17 (indirect)
-        "0015"                             // dst_offset 21 (indirect)
-        "0080"                             // radix_offset 80 (direct)
-        "0100"                             // limbs: 256
-        "00"                               // output_bits: false
-        + to_hex(OpCode::SET_16) +         // opcode SET (for return size)
-        "00"                               // Indirect flag
-        "0200"                             // dst_offset=512
-        + to_hex(AvmMemoryTag::U32) +      //
-        "0100"                             // val: 256
-        + to_hex(OpCode::RETURN) +         // opcode RETURN
-        "00"                               // Indirect flag
-        "0005"                             // ret offset 5
-        "0200";                            // ret size offset 512
-
-    auto bytecode = hex_to_bytes(bytecode_hex);
-    auto [instructions, error] = Deserialization::parse_bytecode_statically(bytecode);
-    ASSERT_TRUE(is_ok(error));
-
-    // Assign a vector that we will mutate internally in gen_trace to store the return values;
-    std::vector<FF> returndata;
-    ExecutionHints execution_hints;
-    auto trace =
-        gen_trace(bytecode, std::vector<FF>{ FF::modulus - FF(1) }, public_inputs, returndata, execution_hints);
-
-    // Find the first row enabling the TORADIXBE selector
-    // Expected output is bitwise decomposition of MODULUS - 1..could hardcode the result but it's a bit long
-    size_t num_limbs = 256;
-    std::vector<FF> expected_output(num_limbs);
-    // Extract each bit.
-    for (size_t i = 0; i < num_limbs; i++) {
-        auto byte_index = num_limbs - i - 1;
-        FF expected_limb = (FF::modulus - 1) >> i & 1;
-        expected_output[byte_index] = expected_limb;
-    }
-    EXPECT_EQ(returndata, expected_output);
-
-    validate_trace(std::move(trace), public_inputs, { FF::modulus - FF(1) }, returndata);
-}
-
-// Positive test with TO_RADIX_BE.
-TEST_F(AvmExecutionTests, toRadixBeOpcodeBitsMode)
+namespace {
+std::vector<uint8_t> gen_bytecode_radix(bool is_bit_mode)
 {
+    const std::string bit_mode_hex = is_bit_mode ? "01" : "00";
     std::string bytecode_hex = to_hex(OpCode::SET_8) +          // opcode SET
                                "00"                             // Indirect flag
                                "00"                             // dst_offset
-                               + to_hex(AvmMemoryTag::U32)      //
-                               + "00"                           // val
+                               + to_hex(AvmMemoryTag::U32) +    //
+                               "00"                             // val
                                + to_hex(OpCode::SET_8) +        // opcode SET
                                "00"                             // Indirect flag
                                "01"                             // dst_offset
@@ -953,18 +885,28 @@ TEST_F(AvmExecutionTests, toRadixBeOpcodeBitsMode)
                                "15"                             // dst_offset 21
                                + to_hex(AvmMemoryTag::U32) +    //
                                "05"                             // value 5 (i.e. where the dst will be written to)
-                               + to_hex(OpCode::SET_8) +        // opcode SET for indirect dst
+                               + to_hex(OpCode::SET_8) +        // opcode SET (direct radix_offset)
                                "00"                             // Indirect flag
                                "80"                             // radix_offset 80
                                + to_hex(AvmMemoryTag::U32) +    //
                                "02"                          // value 2 (i.e. radix 2 - perform bitwise decomposition)
+                               + to_hex(OpCode::SET_16) +    // opcode SET (direct number_limbs_offset)
+                               "00"                          // Indirect flag
+                               "0090"                        // number_limbs_offset 0x90
+                               + to_hex(AvmMemoryTag::U32) + //
+                               "0100"                        // value 256
+                               + to_hex(OpCode::SET_8) +     // opcode SET (direct output_bits_offset)
+                               "00"                          // Indirect flag
+                               "95"                          // output_bits_offset 0x95
+                               + to_hex(AvmMemoryTag::U1) +  //
+                               bit_mode_hex                  // bit 1 (true for bits mode)
                                + to_hex(OpCode::TORADIXBE) + // opcode TO_RADIX_BE
-                               "03"                          // Indirect flag
+                               "0011"                        // Indirect flag
                                "0011"                        // src_offset 17 (indirect)
-                               "0015"                        // dst_offset 21 (indirect)
                                "0080"                        // radix_offset 80 (direct)
-                               "0100"                        // limbs: 256
-                               "01"                          // output_bits: true
+                               "0090"                        // num_limbs_offset (direct)
+                               "0095"                        // output_bits_offset (direct)
+                               "0015"                        // dst_offset 21 (indirect)
                                + to_hex(OpCode::SET_16) +    // opcode SET (for return size)
                                "00"                          // Indirect flag
                                "0200"                        // dst_offset=512
@@ -975,7 +917,15 @@ TEST_F(AvmExecutionTests, toRadixBeOpcodeBitsMode)
                                "0005"                        // ret offset 5
                                "0200";                       // ret size offset 512
 
-    auto bytecode = hex_to_bytes(bytecode_hex);
+    return hex_to_bytes(bytecode_hex);
+}
+} // namespace
+
+// Positive test for TORADIXBE opcode parametrized by a boolean toggling bit vs bytes mode.
+TEST_P(AvmExecutionTestsToRadix, ParamTest)
+{
+    const bool is_bit_mode = GetParam();
+    auto bytecode = gen_bytecode_radix(is_bit_mode);
     auto [instructions, error] = Deserialization::parse_bytecode_statically(bytecode);
     ASSERT_TRUE(is_ok(error));
 
@@ -1000,6 +950,9 @@ TEST_F(AvmExecutionTests, toRadixBeOpcodeBitsMode)
     validate_trace(std::move(trace), public_inputs, { FF::modulus - FF(1) }, returndata);
 }
 
+// Run the test for TORADIXBE in bit mode and then in bytes mode.
+INSTANTIATE_TEST_SUITE_P(AvmExecutionTests, AvmExecutionTestsToRadix, testing::ValuesIn({ true, false }));
+
 // // Positive test with SHA256COMPRESSION.
 TEST_F(AvmExecutionTests, sha256CompressionOpcode)
 {

barretenberg/cpp/src/barretenberg/vm/avm/trace/deserialization.cpp

@@ -179,12 +179,12 @@ const std::unordered_map<OpCode, std::vector<OperandType>> OPCODE_WIRE_FORMAT =
       { OperandType::INDIRECT8, OperandType::UINT16, OperandType::UINT16, OperandType::UINT16, OperandType::UINT16 } },
     // Gadget - Conversion
     { OpCode::TORADIXBE,
-      { OperandType::INDIRECT8,
+      { OperandType::INDIRECT16,
         OperandType::UINT16,
         OperandType::UINT16,
         OperandType::UINT16,
         OperandType::UINT16,
-        OperandType::UINT8 } },
+        OperandType::UINT16 } },
 };
 
 const std::unordered_map<OperandType, uint32_t> OPERAND_TYPE_SIZE = {

barretenberg/cpp/src/barretenberg/vm/avm/trace/execution.cpp

@@ -974,12 +974,12 @@ AvmError Execution::execute_enqueued_call(AvmTraceBuilder& trace_builder,
 
             // Conversions
         case OpCode::TORADIXBE:
-            error = trace_builder.op_to_radix_be(std::get<uint8_t>(inst.operands.at(0)),
+            error = trace_builder.op_to_radix_be(std::get<uint16_t>(inst.operands.at(0)),
                                                  std::get<uint16_t>(inst.operands.at(1)),
                                                  std::get<uint16_t>(inst.operands.at(2)),
                                                  std::get<uint16_t>(inst.operands.at(3)),
                                                  std::get<uint16_t>(inst.operands.at(4)),
-                                                 std::get<uint8_t>(inst.operands.at(5)));
+                                                 std::get<uint16_t>(inst.operands.at(5)));
             break;
 
         default:

barretenberg/cpp/src/barretenberg/vm/avm/trace/trace.cpp

@@ -4429,46 +4429,57 @@ AvmError AvmTraceBuilder::op_variable_msm(uint8_t indirect,
  *
  * @param indirect A byte encoding information about indirect/direct memory access.
  * @param src_offset An index in memory pointing to the input of the To_Radix_BE conversion.
- * @param dst_offset An index in memory pointing to the output of the To_Radix_BE conversion.
  * @param radix_offset An index in memory pointing to the strict upper bound of each converted limb, i.e., 0 <= limb
  * < radix.
- * @param num_limbs The number of limbs to the value into.
- * @param output_bits Should the output be U1s instead of U8s?
+ * @param num_limbs_offset Offset pointing to the number of limbs to the value into.
+ * @param output_bits_offset Offset pointing to a boolean telling whether bits (U1) or bytes (U8) are in the output
+ * @param dst_offset An index in memory pointing to the output of the To_Radix_BE conversion.
  */
-AvmError AvmTraceBuilder::op_to_radix_be(uint8_t indirect,
+AvmError AvmTraceBuilder::op_to_radix_be(uint16_t indirect,
                                          uint32_t src_offset,
-                                         uint32_t dst_offset,
                                          uint32_t radix_offset,
-                                         uint32_t num_limbs,
-                                         uint8_t output_bits)
+                                         uint32_t num_limbs_offset,
+                                         uint32_t output_bits_offset,
+                                         uint32_t dst_offset)
 {
     // We keep the first encountered error
     AvmError error = AvmError::NO_ERROR;
     auto clk = static_cast<uint32_t>(main_trace.size()) + 1;
 
-    // write output as bits or bytes
-    AvmMemoryTag w_in_tag = output_bits > 0 ? AvmMemoryTag::U1 // bits mode
-                                            : AvmMemoryTag::U8;
-
-    auto [resolved_addrs, res_error] = Addressing<3>::fromWire(indirect, call_ptr)
-                                           .resolve({ src_offset, dst_offset, radix_offset }, mem_trace_builder);
-    auto [resolved_src_offset, resolved_dst_offset, resolved_radix_offset] = resolved_addrs;
+    auto [resolved_addrs, res_error] =
+        Addressing<5>::fromWire(indirect, call_ptr)
+            .resolve({ src_offset, radix_offset, num_limbs_offset, output_bits_offset, dst_offset }, mem_trace_builder);
+    auto [resolved_src_offset,
+          resolved_radix_offset,
+          resolved_num_limbs_offset,
+          resolved_output_bits_offset,
+          resolved_dst_offset] = resolved_addrs;
     error = res_error;
 
+    if (is_ok(error) && !check_tag(AvmMemoryTag::U32, resolved_radix_offset) &&
+        !check_tag(AvmMemoryTag::U32, resolved_num_limbs_offset) &&
+        !check_tag(AvmMemoryTag::U1, resolved_output_bits_offset)) {
+        error = AvmError::CHECK_TAG_ERROR;
+    }
+
+    const auto num_limbs = static_cast<uint32_t>(unconstrained_read_from_memory(resolved_num_limbs_offset));
+
     // Constrain gas cost
     gas_trace_builder.constrain_gas(clk, OpCode::TORADIXBE, num_limbs);
 
+    const auto output_bits = static_cast<uint8_t>(unconstrained_read_from_memory(resolved_output_bits_offset));
+
+    // write output as bits or bytes
+    AvmMemoryTag w_in_tag = output_bits > 0 ? AvmMemoryTag::U1 // bits mode
+                                            : AvmMemoryTag::U8;
+
     auto read_src = constrained_read_from_memory(
         call_ptr, clk, resolved_src_offset, AvmMemoryTag::FF, w_in_tag, IntermRegister::IA);
     // TODO(8603): once instructions can have multiple different tags for reads, constrain the radix's read
     // TODO(9497): if simulator fails tag check here, witgen will not. Raise error flag!
     // auto read_radix = constrained_read_from_memory(
     //    call_ptr, clk, resolved_radix_offset, AvmMemoryTag::U32, AvmMemoryTag::U32, IntermRegister::IB);
 
-    if (is_ok(error) && !check_tag(AvmMemoryTag::U32, resolved_radix_offset)) {
-        error = AvmError::CHECK_TAG_ERROR;
-    }
-
     auto read_radix = unconstrained_read_from_memory(resolved_radix_offset);
 
     FF input = read_src.val;

barretenberg/cpp/src/barretenberg/vm/avm/trace/trace.hpp

@@ -220,12 +220,12 @@ class AvmTraceBuilder {
                              uint32_t output_offset,
                              uint32_t point_length_offset);
     // Conversions
-    AvmError op_to_radix_be(uint8_t indirect,
+    AvmError op_to_radix_be(uint16_t indirect,
                             uint32_t src_offset,
-                            uint32_t dst_offset,
                             uint32_t radix_offset,
-                            uint32_t num_limbs,
-                            uint8_t output_bits);
+                            uint32_t num_limbs_offset,
+                            uint32_t output_bits_offset,
+                            uint32_t dst_offset);
 
     std::vector<Row> finalize(bool apply_end_gas_assertions = false);
     void reset();