From 1a5eb6923adb2f469021715182c1c5443e2d415c Mon Sep 17 00:00:00 2001 From: Facundo Date: Mon, 18 Mar 2024 11:45:43 +0000 Subject: [PATCH] chore(avm-transpiler): return u8 in comparison ops (#5280) This makes sense and allows us to remove a Brillig hack. cc @sirasistant --- avm-transpiler/src/transpile.rs | 17 ----------------- .../src/avm/opcodes/comparators.test.ts | 14 +++++++------- .../simulator/src/avm/opcodes/comparators.ts | 8 ++++---- .../docs/public-vm/gen/_instruction-set.mdx | 6 +++--- .../preprocess/InstructionSet/InstructionSet.js | 6 +++--- 5 files changed, 17 insertions(+), 34 deletions(-) diff --git a/avm-transpiler/src/transpile.rs b/avm-transpiler/src/transpile.rs index 4af9ab4078e..917b1155e35 100644 --- a/avm-transpiler/src/transpile.rs +++ b/avm-transpiler/src/transpile.rs @@ -61,10 +61,6 @@ pub fn brillig_to_avm(brillig: &Brillig) -> Vec { }, ], }); - // 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, @@ -107,10 +103,6 @@ pub fn brillig_to_avm(brillig: &Brillig) -> Vec { }, ], }); - // 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 { @@ -998,15 +990,6 @@ fn map_brillig_pcs_to_avm_pcs(initial_offset: usize, brillig: &Brillig) -> Vec 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 diff --git a/yarn-project/simulator/src/avm/opcodes/comparators.test.ts b/yarn-project/simulator/src/avm/opcodes/comparators.test.ts index c8001c7759f..97fff094a6a 100644 --- a/yarn-project/simulator/src/avm/opcodes/comparators.test.ts +++ b/yarn-project/simulator/src/avm/opcodes/comparators.test.ts @@ -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'; @@ -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 () => { @@ -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 () => { @@ -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 () => { @@ -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 () => { @@ -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 () => { @@ -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 () => { diff --git a/yarn-project/simulator/src/avm/opcodes/comparators.ts b/yarn-project/simulator/src/avm/opcodes/comparators.ts index 00cb33708d5..62145da0b84 100644 --- a/yarn-project/simulator/src/avm/opcodes/comparators.ts +++ b/yarn-project/simulator/src/avm/opcodes/comparators.ts @@ -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'; @@ -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(); @@ -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(); @@ -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(); diff --git a/yellow-paper/docs/public-vm/gen/_instruction-set.mdx b/yellow-paper/docs/public-vm/gen/_instruction-set.mdx index 4025bd5c60d..73902c179ef 100644 --- a/yellow-paper/docs/public-vm/gen/_instruction-set.mdx +++ b/yellow-paper/docs/public-vm/gen/_instruction-set.mdx @@ -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) @@ -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) @@ -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) diff --git a/yellow-paper/src/preprocess/InstructionSet/InstructionSet.js b/yellow-paper/src/preprocess/InstructionSet/InstructionSet.js index 13df656750a..ef04c328b79 100644 --- a/yellow-paper/src/preprocess/InstructionSet/InstructionSet.js +++ b/yellow-paper/src/preprocess/InstructionSet/InstructionSet.js @@ -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", @@ -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", @@ -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",