Skip to content

Commit

Permalink
chore(avm-transpiler): return u8 in comparison ops (#5280)
Browse files Browse the repository at this point in the history
This makes sense and allows us to remove a Brillig hack.

cc @sirasistant
  • Loading branch information
fcarreiro authored Mar 18, 2024
1 parent 20d9c0c commit 1a5eb69
Show file tree
Hide file tree
Showing 5 changed files with 17 additions and 34 deletions.
17 changes: 0 additions & 17 deletions avm-transpiler/src/transpile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -61,10 +61,6 @@ pub fn brillig_to_avm(brillig: &Brillig) -> Vec<u8> {
},
],
});
// Brillig currently expects comparison instructions to return an u1 (for us, an u8).
if avm_opcode == AvmOpcode::EQ {
avm_instrs.push(generate_cast_instruction(destination.to_usize() as u32, destination.to_usize() as u32, AvmTypeTag::UINT8));
}
}
BrilligOpcode::BinaryIntOp {
destination,
Expand Down Expand Up @@ -107,10 +103,6 @@ pub fn brillig_to_avm(brillig: &Brillig) -> Vec<u8> {
},
],
});
// Brillig currently expects comparison instructions to return an u1 (for us, an u8).
if avm_opcode == AvmOpcode::EQ || avm_opcode == AvmOpcode::LT || avm_opcode == AvmOpcode::LTE {
avm_instrs.push(generate_cast_instruction(destination.to_usize() as u32, destination.to_usize() as u32, AvmTypeTag::UINT8));
}
}
BrilligOpcode::CalldataCopy { destination_address, size, offset } => {
avm_instrs.push(AvmInstruction {
Expand Down Expand Up @@ -998,15 +990,6 @@ fn map_brillig_pcs_to_avm_pcs(initial_offset: usize, brillig: &Brillig) -> Vec<u
for i in 0..brillig.bytecode.len() - 1 {
let num_avm_instrs_for_this_brillig_instr = match &brillig.bytecode[i] {
BrilligOpcode::Const { bit_size: 254, .. } => 2,
// Brillig currently expects comparison instructions to return an u1 (for us, an u8).
BrilligOpcode::BinaryIntOp {
op: BinaryIntOp::Equals | BinaryIntOp::LessThan | BinaryIntOp::LessThanEquals,
..
} => 2,
BrilligOpcode::BinaryFieldOp {
op: BinaryFieldOp::Equals,
..
} => 2,
_ => 1,
};
// next Brillig pc will map to an AVM pc offset by the
Expand Down
14 changes: 7 additions & 7 deletions yarn-project/simulator/src/avm/opcodes/comparators.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { AvmContext } from '../avm_context.js';
import { Field, TypeTag, Uint16, Uint32 } from '../avm_memory_types.js';
import { Field, TypeTag, Uint8, Uint16, Uint32 } from '../avm_memory_types.js';
import { TagCheckError } from '../errors.js';
import { initContext } from '../fixtures/index.js';
import { Eq, Lt, Lte } from './comparators.js';
Expand Down Expand Up @@ -43,7 +43,7 @@ describe('Comparators', () => {
].forEach(i => i.execute(context));

const actual = context.machineState.memory.getSlice(/*offset=*/ 10, /*size=*/ 4);
expect(actual).toEqual([new Uint32(0), new Uint32(0), new Uint32(1)]);
expect(actual).toEqual([new Uint8(0), new Uint8(0), new Uint8(1)]);
});

it('Works on field elements', async () => {
Expand All @@ -56,7 +56,7 @@ describe('Comparators', () => {
].forEach(i => i.execute(context));

const actual = context.machineState.memory.getSlice(/*offset=*/ 10, /*size=*/ 4);
expect(actual).toEqual([new Field(0), new Field(0), new Field(1)]);
expect(actual).toEqual([new Uint8(0), new Uint8(0), new Uint8(1)]);
});

it('InTag is checked', async () => {
Expand Down Expand Up @@ -107,7 +107,7 @@ describe('Comparators', () => {
].forEach(i => i.execute(context));

const actual = context.machineState.memory.getSlice(/*offset=*/ 10, /*size=*/ 4);
expect(actual).toEqual([new Uint32(0), new Uint32(1), new Uint32(0)]);
expect(actual).toEqual([new Uint8(0), new Uint8(1), new Uint8(0)]);
});

it('Works on field elements', async () => {
Expand All @@ -120,7 +120,7 @@ describe('Comparators', () => {
].forEach(i => i.execute(context));

const actual = context.machineState.memory.getSlice(/*offset=*/ 10, /*size=*/ 4);
expect(actual).toEqual([new Field(0), new Field(1), new Field(0)]);
expect(actual).toEqual([new Uint8(0), new Uint8(1), new Uint8(0)]);
});

it('InTag is checked', async () => {
Expand Down Expand Up @@ -171,7 +171,7 @@ describe('Comparators', () => {
].forEach(i => i.execute(context));

const actual = context.machineState.memory.getSlice(/*offset=*/ 10, /*size=*/ 4);
expect(actual).toEqual([new Uint32(1), new Uint32(1), new Uint32(0)]);
expect(actual).toEqual([new Uint8(1), new Uint8(1), new Uint8(0)]);
});

it('Works on field elements', async () => {
Expand All @@ -184,7 +184,7 @@ describe('Comparators', () => {
].forEach(i => i.execute(context));

const actual = context.machineState.memory.getSlice(/*offset=*/ 10, /*size=*/ 4);
expect(actual).toEqual([new Field(1), new Field(1), new Field(0)]);
expect(actual).toEqual([new Uint8(1), new Uint8(1), new Uint8(0)]);
});

it('InTag is checked', async () => {
Expand Down
8 changes: 4 additions & 4 deletions yarn-project/simulator/src/avm/opcodes/comparators.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import type { AvmContext } from '../avm_context.js';
import { TaggedMemory } from '../avm_memory_types.js';
import { Uint8 } from '../avm_memory_types.js';
import { Opcode } from '../serialization/instruction_serialization.js';
import { ThreeOperandInstruction } from './instruction_impl.js';

Expand All @@ -17,7 +17,7 @@ export class Eq extends ThreeOperandInstruction {
const a = context.machineState.memory.get(this.aOffset);
const b = context.machineState.memory.get(this.bOffset);

const dest = TaggedMemory.buildFromTagOrDie(a.equals(b) ? 1n : 0n, this.inTag);
const dest = new Uint8(a.equals(b) ? 1 : 0);
context.machineState.memory.set(this.dstOffset, dest);

context.machineState.incrementPc();
Expand All @@ -38,7 +38,7 @@ export class Lt extends ThreeOperandInstruction {
const a = context.machineState.memory.get(this.aOffset);
const b = context.machineState.memory.get(this.bOffset);

const dest = TaggedMemory.buildFromTagOrDie(a.lt(b) ? 1n : 0n, this.inTag);
const dest = new Uint8(a.lt(b) ? 1 : 0);
context.machineState.memory.set(this.dstOffset, dest);

context.machineState.incrementPc();
Expand All @@ -59,7 +59,7 @@ export class Lte extends ThreeOperandInstruction {
const a = context.machineState.memory.get(this.aOffset);
const b = context.machineState.memory.get(this.bOffset);

const dest = TaggedMemory.buildFromTagOrDie(a.lt(b) || a.equals(b) ? 1n : 0n, this.inTag);
const dest = new Uint8(a.lt(b) || a.equals(b) ? 1 : 0);
context.machineState.memory.set(this.dstOffset, dest);

context.machineState.incrementPc();
Expand Down
6 changes: 3 additions & 3 deletions yellow-paper/docs/public-vm/gen/_instruction-set.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -604,7 +604,7 @@ Equality check (a == b)
- **dstOffset**: memory offset specifying where to store operation's result
- **Expression**: `M[dstOffset] = M[aOffset] == M[bOffset] ? 1 : 0`
- **Tag checks**: `T[aOffset] == T[bOffset] == inTag`
- **Tag updates**: `T[dstOffset] = inTag`
- **Tag updates**: `T[dstOffset] = u8`
- **Bit-size**: 128

[![](./images/bit-formats/EQ.png)](./images/bit-formats/EQ.png)
Expand All @@ -625,7 +625,7 @@ Less-than check (a < b)
- **dstOffset**: memory offset specifying where to store operation's result
- **Expression**: `M[dstOffset] = M[aOffset] < M[bOffset] ? 1 : 0`
- **Tag checks**: `T[aOffset] == T[bOffset] == inTag`
- **Tag updates**: `T[dstOffset] = inTag`
- **Tag updates**: `T[dstOffset] = u8`
- **Bit-size**: 128

[![](./images/bit-formats/LT.png)](./images/bit-formats/LT.png)
Expand All @@ -646,7 +646,7 @@ Less-than-or-equals check (a <= b)
- **dstOffset**: memory offset specifying where to store operation's result
- **Expression**: `M[dstOffset] = M[aOffset] <= M[bOffset] ? 1 : 0`
- **Tag checks**: `T[aOffset] == T[bOffset] == inTag`
- **Tag updates**: `T[dstOffset] = inTag`
- **Tag updates**: `T[dstOffset] = u8`
- **Bit-size**: 128

[![](./images/bit-formats/LTE.png)](./images/bit-formats/LTE.png)
Expand Down
6 changes: 3 additions & 3 deletions yellow-paper/src/preprocess/InstructionSet/InstructionSet.js
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ const INSTRUCTION_SET_RAW = [
"Summary": "Equality check (a == b)",
"Details": "",
"Tag checks": "`T[aOffset] == T[bOffset] == inTag`",
"Tag updates": "`T[dstOffset] = inTag`",
"Tag updates": "`T[dstOffset] = u8`",
},
{
"id": "lt",
Expand All @@ -159,7 +159,7 @@ const INSTRUCTION_SET_RAW = [
"Summary": "Less-than check (a < b)",
"Details": "",
"Tag checks": "`T[aOffset] == T[bOffset] == inTag`",
"Tag updates": "`T[dstOffset] = inTag`",
"Tag updates": "`T[dstOffset] = u8`",
},
{
"id": "lte",
Expand All @@ -178,7 +178,7 @@ const INSTRUCTION_SET_RAW = [
"Summary": "Less-than-or-equals check (a <= b)",
"Details": "",
"Tag checks": "`T[aOffset] == T[bOffset] == inTag`",
"Tag updates": "`T[dstOffset] = inTag`",
"Tag updates": "`T[dstOffset] = u8`",
},
{
"id": "and",
Expand Down

0 comments on commit 1a5eb69

Please sign in to comment.