Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Pass calldata ids to the backend #7875

Merged
merged 5 commits into from
Aug 12, 2024
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,6 @@ void build_constraints(Builder& builder,
}

// Add block constraints
assign_calldata_ids<Builder>(constraint_system.block_constraints);
for (size_t i = 0; i < constraint_system.block_constraints.size(); ++i) {
const auto& constraint = constraint_system.block_constraints.at(i);
create_block_constraints(builder, constraint, has_valid_witness_assignments);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -518,6 +518,7 @@ BlockConstraint handle_memory_init(Program::Opcode::MemoryInit const& mem_init)
// array.
if (std::holds_alternative<Program::BlockType::CallData>(mem_init.block_type.value)) {
block.type = BlockType::CallData;
block.calldata_id = std::get<Program::BlockType::CallData>(mem_init.block_type.value).value;
} else if (std::holds_alternative<Program::BlockType::ReturnData>(mem_init.block_type.value)) {
block.type = BlockType::ReturnData;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -213,20 +213,4 @@ void process_return_data_operations(const BlockConstraint& constraint, std::vect
ASSERT(constraint.trace.size() == 0);
}

// Do nothing for Ultra since it does not support Databus
template <> void assign_calldata_ids<UltraCircuitBuilder>([[maybe_unused]] std::vector<BlockConstraint>& constraints) {}

template <> void assign_calldata_ids<MegaCircuitBuilder>(std::vector<BlockConstraint>& constraints)
{
// Assign unique ID to each calldata block constraint
uint32_t calldata_id = 0;
for (auto& constraint : constraints) {
if (constraint.type == BlockType::CallData) {
constraint.calldata_id = calldata_id++;
}
}
// The backend only supports 2 calldata columns
ASSERT(calldata_id <= 2);
}

} // namespace acir_format
Original file line number Diff line number Diff line change
Expand Up @@ -48,16 +48,6 @@ void process_call_data_operations(Builder& builder,
template <typename Builder>
void process_return_data_operations(const BlockConstraint& constraint, std::vector<bb::stdlib::field_t<Builder>>& init);

/**
* @brief Assign a unique ID to each calldata block constraint based on the order in which it was recieved
* TODO(https://github.com/AztecProtocol/barretenberg/issues/1070): this is a workaround to allow calldata inputs to be
* distinguished by the backend since no identifiers are received from noir.
*
* @tparam Builder
* @param constraints
*/
template <typename Builder> void assign_calldata_ids(std::vector<BlockConstraint>& constraints);

template <typename B> inline void read(B& buf, MemOp& mem_op)
{
using serialize::read;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1107,6 +1107,8 @@ struct BlockType {
};

struct CallData {
uint32_t value;

friend bool operator==(const CallData&, const CallData&);
std::vector<uint8_t> bincodeSerialize() const;
static CallData bincodeDeserialize(std::vector<uint8_t>);
Expand Down Expand Up @@ -5578,6 +5580,9 @@ namespace Program {

inline bool operator==(const BlockType::CallData& lhs, const BlockType::CallData& rhs)
{
if (!(lhs.value == rhs.value)) {
return false;
}
return true;
}

Expand All @@ -5604,14 +5609,17 @@ template <>
template <typename Serializer>
void serde::Serializable<Program::BlockType::CallData>::serialize(const Program::BlockType::CallData& obj,
Serializer& serializer)
{}
{
serde::Serializable<decltype(obj.value)>::serialize(obj.value, serializer);
}

template <>
template <typename Deserializer>
Program::BlockType::CallData serde::Deserializable<Program::BlockType::CallData>::deserialize(
Deserializer& deserializer)
{
Program::BlockType::CallData obj;
obj.value = serde::Deserializable<decltype(obj.value)>::deserialize(deserializer);
return obj;
}

Expand Down
5 changes: 5 additions & 0 deletions noir/noir-repo/acvm-repo/acir/codegen/acir.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1042,6 +1042,8 @@ namespace Program {
};

struct CallData {
uint32_t value;

friend bool operator==(const CallData&, const CallData&);
std::vector<uint8_t> bincodeSerialize() const;
static CallData bincodeDeserialize(std::vector<uint8_t>);
Expand Down Expand Up @@ -4683,6 +4685,7 @@ Program::BlockType::Memory serde::Deserializable<Program::BlockType::Memory>::de
namespace Program {

inline bool operator==(const BlockType::CallData &lhs, const BlockType::CallData &rhs) {
if (!(lhs.value == rhs.value)) { return false; }
return true;
}

Expand All @@ -4706,12 +4709,14 @@ namespace Program {
template <>
template <typename Serializer>
void serde::Serializable<Program::BlockType::CallData>::serialize(const Program::BlockType::CallData &obj, Serializer &serializer) {
serde::Serializable<decltype(obj.value)>::serialize(obj.value, serializer);
}

template <>
template <typename Deserializer>
Program::BlockType::CallData serde::Deserializable<Program::BlockType::CallData>::deserialize(Deserializer &deserializer) {
Program::BlockType::CallData obj;
obj.value = serde::Deserializable<decltype(obj.value)>::deserialize(deserializer);
return obj;
}

Expand Down
8 changes: 4 additions & 4 deletions noir/noir-repo/acvm-repo/acir/src/circuit/opcodes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,13 @@ pub use memory_operation::{BlockId, MemOp};
#[derive(Clone, PartialEq, Eq, Serialize, Deserialize)]
pub enum BlockType {
Memory,
CallData,
CallData(u32),
ReturnData,
}

impl BlockType {
pub fn is_databus(&self) -> bool {
matches!(self, BlockType::CallData | BlockType::ReturnData)
matches!(self, BlockType::CallData(_) | BlockType::ReturnData)
}
}

Expand Down Expand Up @@ -183,10 +183,10 @@ impl<F: AcirField> std::fmt::Display for Opcode<F> {
Opcode::MemoryInit { block_id, init, block_type: databus } => {
match databus {
BlockType::Memory => write!(f, "INIT ")?,
BlockType::CallData => write!(f, "INIT CALLDATA ")?,
BlockType::CallData(id) => write!(f, "INIT CALLDATA {} ", id)?,
BlockType::ReturnData => write!(f, "INIT RETURNDATA ")?,
}
write!(f, "(id: {}, len: {}) ", block_id.0, init.len())
write!(f, "(id: {}, start: {} len: {}) ", block_id.0, init[0].0, init.len())
sirasistant marked this conversation as resolved.
Show resolved Hide resolved
}
// We keep the display for a BrilligCall and circuit Call separate as they
// are distinct in their functionality and we should maintain this separation for debugging.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1722,10 +1722,10 @@ impl<'a> Context<'a> {
{
databus = BlockType::ReturnData;
}
for array_id in self.data_bus.call_data_array() {
for (user_id, array_id) in self.data_bus.call_data_array() {
if self.block_id(&array_id) == array {
assert!(databus == BlockType::Memory);
databus = BlockType::CallData;
databus = BlockType::CallData(user_id);
break;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,13 @@ pub(crate) enum DatabusVisibility {
}
/// Used to create a data bus, which is an array of private inputs
/// replacing public inputs
#[derive(Clone, Debug)]
pub(crate) struct DataBusBuilder {
pub(crate) values: im::Vector<ValueId>,
index: usize,
pub(crate) map: HashMap<ValueId, usize>,
pub(crate) databus: Option<ValueId>,
call_data_id: Option<u32>,
}

impl DataBusBuilder {
Expand All @@ -31,6 +33,7 @@ impl DataBusBuilder {
map: HashMap::default(),
databus: None,
values: im::Vector::new(),
call_data_id: None,
}
}

Expand All @@ -54,6 +57,8 @@ impl DataBusBuilder {

#[derive(Clone, Debug)]
pub(crate) struct CallData {
/// The id to this calldata assigned by the user
pub(crate) calldata_id: u32,
pub(crate) array_id: ValueId,
pub(crate) index_map: HashMap<ValueId, usize>,
}
Expand All @@ -75,14 +80,18 @@ impl DataBus {
for (k, v) in cd.index_map.iter() {
call_data_map.insert(f(*k), *v);
}
CallData { array_id: f(cd.array_id), index_map: call_data_map }
CallData {
array_id: f(cd.array_id),
index_map: call_data_map,
calldata_id: cd.calldata_id,
}
})
.collect();
DataBus { call_data, return_data: self.return_data.map(&mut f) }
}

pub(crate) fn call_data_array(&self) -> Vec<ValueId> {
self.call_data.iter().map(|cd| cd.array_id).collect()
pub(crate) fn call_data_array(&self) -> Vec<(u32, ValueId)> {
self.call_data.iter().map(|cd| (cd.calldata_id, cd.array_id)).collect()
}
/// Construct a databus from call_data and return_data data bus builders
pub(crate) fn get_data_bus(
Expand All @@ -91,9 +100,13 @@ impl DataBus {
) -> DataBus {
let mut call_data_args = Vec::new();
for call_data_item in call_data {
if let Some(array_id) = call_data_item.databus {
call_data_args.push(CallData { array_id, index_map: call_data_item.map });
}
let array_id = call_data_item.databus.expect("Call data should have an array id");
let user_id = call_data_item.call_data_id.expect("Call data should have a user id");
call_data_args.push(CallData {
array_id,
calldata_id: user_id,
index_map: call_data_item.map,
});
}

DataBus { call_data: call_data_args, return_data: return_data.databus }
Expand Down Expand Up @@ -136,6 +149,7 @@ impl FunctionBuilder {
&mut self,
values: &[ValueId],
mut databus: DataBusBuilder,
call_data_id: Option<u32>,
) -> DataBusBuilder {
for value in values {
self.add_to_data_bus(*value, &mut databus);
Expand All @@ -150,7 +164,13 @@ impl FunctionBuilder {
None
};

DataBusBuilder { index: 0, map: databus.map, databus: array, values: im::Vector::new() }
DataBusBuilder {
index: 0,
map: databus.map,
databus: array,
values: im::Vector::new(),
call_data_id,
}
}

/// Generate the data bus for call-data, based on the parameters of the entry block
Expand Down Expand Up @@ -186,7 +206,7 @@ impl FunctionBuilder {
let mut result = Vec::new();
for id in databus_param.keys() {
let builder = DataBusBuilder::new();
let call_databus = self.initialize_data_bus(&databus_param[id], builder);
let call_databus = self.initialize_data_bus(&databus_param[id], builder, Some(*id));
result.push(call_databus);
}
result
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,8 +83,11 @@ pub(crate) fn generate_ssa(
_ => unreachable!("ICE - expect return on the last block"),
};

return_data =
function_context.builder.initialize_data_bus(&return_data_values, return_data);
return_data = function_context.builder.initialize_data_bus(
&return_data_values,
return_data,
None,
);
}
let return_instruction =
function_context.builder.current_function.dfg[block].unwrap_terminator_mut();
Expand Down
Loading