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.

Sign up for GitHub

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

Jump to bottom

fix(avm): comments and assert #5956

Merged
merged 1 commit into from
Apr 23, 2024
Merged

fix(avm): comments and assert #5956

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
17 changes: 11 additions & 6 deletions barretenberg/cpp/src/barretenberg/vm/avm_trace/avm_alu_trace.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -781,6 +781,7 @@ FF AvmAluTraceBuilder::op_shr(FF const& a, FF const& b, AvmMemoryTag in_tag, uin
uint256_t a_u256{ a };
// Check that the shifted amount is an 8-bit integer
ASSERT(uint256_t(b) < 256);
ASSERT(in_tag != AvmMemoryTag::U0 || in_tag != AvmMemoryTag::FF);

uint8_t b_u8 = static_cast<uint8_t>(uint256_t(b));
uint256_t c_u256 = a_u256 >> b_u8;
Expand All @@ -792,6 +793,7 @@ FF AvmAluTraceBuilder::op_shr(FF const& a, FF const& b, AvmMemoryTag in_tag, uin
if (b_u8 >= num_bits) {
u8_pow_2_counters[1][b_u8 - num_bits]++;
// Even though the registers are trivially zero, we call this function to increment the lookup counters
// Future workaround would be to decouple the range_check toggle and the counter from this function
[[maybe_unused]] auto [alu_u8_r0, alu_u8_r1, alu_u16_reg] = AvmAluTraceBuilder::to_alu_slice_registers(0);
alu_trace.push_back(AvmAluTraceBuilder::AluTraceEntry{
.alu_clk = clk,
Expand All @@ -803,7 +805,7 @@ FF AvmAluTraceBuilder::op_shr(FF const& a, FF const& b, AvmMemoryTag in_tag, uin
.alu_u64_tag = in_tag == AvmMemoryTag::U64,
.alu_u128_tag = in_tag == AvmMemoryTag::U128,
.alu_ia = a,
.alu_ib = FF(b_u8),
.alu_ib = b,
.alu_ic = 0,
.hi_lo_limbs = { 0, 0, 0, 0 },
.mem_tag_bits = num_bits,
Expand All @@ -820,7 +822,7 @@ FF AvmAluTraceBuilder::op_shr(FF const& a, FF const& b, AvmMemoryTag in_tag, uin
uint256_t rng_chk_lo = (uint256_t(1) << b_u8) - x_lo - 1;
uint256_t rng_chk_hi = (uint256_t(1) << (num_bits - b_u8)) - x_hi - 1;

// Each hi and lo limb is range checked over 128bits
// Each hi and lo limb is range checked over 128 bits
uint256_t limb = rng_chk_lo + (rng_chk_hi << uint256_t(128));
// Load the range check values into the ALU registers
auto [alu_u8_r0, alu_u8_r1, alu_u16_reg] = AvmAluTraceBuilder::to_alu_slice_registers(limb);
Expand Down Expand Up @@ -868,6 +870,8 @@ FF AvmAluTraceBuilder::op_shl(FF const& a, FF const& b, AvmMemoryTag in_tag, uin
uint256_t a_u256{ a };
// Check that the shift amount is an 8-bit integer
ASSERT(uint256_t(b) < 256);
ASSERT(in_tag != AvmMemoryTag::U0 || in_tag != AvmMemoryTag::FF);

uint8_t b_u8 = static_cast<uint8_t>(uint256_t(b));

uint256_t c_u256 = a_u256 << b_u8;
Expand All @@ -878,6 +882,7 @@ FF AvmAluTraceBuilder::op_shl(FF const& a, FF const& b, AvmMemoryTag in_tag, uin
if (b_u8 >= num_bits) {
u8_pow_2_counters[1][b_u8 - num_bits]++;
// Even though the registers are trivially zero, we call this function to increment the lookup counters
// Future workaround would be to decouple the range_check toggle and the counter from this function
[[maybe_unused]] auto [alu_u8_r0, alu_u8_r1, alu_u16_reg] = AvmAluTraceBuilder::to_alu_slice_registers(0);
alu_trace.push_back(AvmAluTraceBuilder::AluTraceEntry{
.alu_clk = clk,
Expand All @@ -889,7 +894,7 @@ FF AvmAluTraceBuilder::op_shl(FF const& a, FF const& b, AvmMemoryTag in_tag, uin
.alu_u64_tag = in_tag == AvmMemoryTag::U64,
.alu_u128_tag = in_tag == AvmMemoryTag::U128,
.alu_ia = a,
.alu_ib = FF(b_u8),
.alu_ib = b,
.alu_ic = 0,
.hi_lo_limbs = { 0, 0, 0, 0 },
.mem_tag_bits = num_bits,
Expand All @@ -908,7 +913,7 @@ FF AvmAluTraceBuilder::op_shl(FF const& a, FF const& b, AvmMemoryTag in_tag, uin
uint256_t rng_chk_lo = uint256_t(uint256_t(1) << (num_bits - b_u8)) - x_lo - 1;
uint256_t rng_chk_hi = uint256_t(uint256_t(1) << b_u8) - x_hi - 1;

// Each hi and lo limb is range checked over 128bits
// Each hi and lo limb is range checked over 128 bits
uint256_t limb = rng_chk_lo + (rng_chk_hi << 128);
// Load the range check values into the ALU registers
auto [alu_u8_r0, alu_u8_r1, alu_u16_reg] = AvmAluTraceBuilder::to_alu_slice_registers(limb);
Expand All @@ -930,10 +935,10 @@ FF AvmAluTraceBuilder::op_shl(FF const& a, FF const& b, AvmMemoryTag in_tag, uin
case AvmMemoryTag::U128:
c = FF{ uint256_t::from_uint128(uint128_t(c_u256)) };
break;
// Unsupported instruction tags
// Unsupported instruction tags, asserted earlier in function
case AvmMemoryTag::U0:
case AvmMemoryTag::FF:
return 0;
__builtin_unreachable();
}

alu_trace.push_back(AvmAluTraceBuilder::AluTraceEntry{
Expand Down
2 changes: 1 addition & 1 deletion barretenberg/cpp/src/barretenberg/vm/tests/avm_bitwise.test.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ void common_validate_shift_op(std::vector<Row> const& trace,
EXPECT_EQ(row->avm_main_mem_op_a, FF(1));
EXPECT_EQ(row->avm_main_rwa, FF(0));

// Check that ia register is correctly set with memory load operations.
// Check that ib register is correctly set with memory load operations.
EXPECT_EQ(row->avm_main_ib, b);
EXPECT_EQ(row->avm_main_mem_idx_b, addr_b);
EXPECT_EQ(row->avm_main_mem_op_b, FF(1));
Expand Down
Loading