Skip to content

Commit

Permalink
chore(avm-simulator): revive field comparison (#4957)
Browse files Browse the repository at this point in the history
Partially roll back #4516. Field comparison is needed in Brillig to assert bit sizes.
  • Loading branch information
fcarreiro authored Mar 6, 2024
1 parent c1d1865 commit ee21374
Show file tree
Hide file tree
Showing 6 changed files with 43 additions and 21 deletions.
7 changes: 7 additions & 0 deletions yarn-project/simulator/src/avm/avm_memory_types.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,13 @@ describe('Field', () => {
expect(field1.equals(field3)).toBe(false);
});

it(`Should check if one Field is less than another correctly`, () => {
const field1 = new Field(5);
const field2 = new Field(10);
expect(field1.lt(field2)).toBe(true);
expect(field2.lt(field1)).toBe(false);
});

it(`Should convert Field to BigInt correctly`, () => {
const field = new Field(5);
expect(field.toBigInt()).toStrictEqual(5n);
Expand Down
7 changes: 5 additions & 2 deletions yarn-project/simulator/src/avm/avm_memory_types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ export abstract class MemoryValue {
public abstract div(rhs: MemoryValue): MemoryValue;

public abstract equals(rhs: MemoryValue): boolean;
public abstract lt(rhs: MemoryValue): boolean;

// We need this to be able to build an instance of the subclasses.
public abstract build(n: bigint): MemoryValue;
Expand All @@ -37,8 +38,6 @@ export abstract class IntegralValue extends MemoryValue {
public abstract or(rhs: IntegralValue): IntegralValue;
public abstract xor(rhs: IntegralValue): IntegralValue;
public abstract not(): IntegralValue;

public abstract lt(rhs: IntegralValue): boolean;
}

/**
Expand Down Expand Up @@ -164,6 +163,10 @@ export class Field extends MemoryValue {
return this.rep.equals(rhs.rep);
}

public lt(rhs: Field): boolean {
return this.rep.lt(rhs.rep);
}

public toBigInt(): bigint {
return this.rep.toBigInt();
}
Expand Down
31 changes: 23 additions & 8 deletions yarn-project/simulator/src/avm/opcodes/comparators.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -110,11 +110,19 @@ describe('Comparators', () => {
expect(actual).toEqual([new Uint32(0), new Uint32(1), new Uint32(0)]);
});

it('Does not work on field elements', async () => {
await expect(() =>
new Lt(/*indirect=*/ 0, TypeTag.FIELD, /*aOffset=*/ 0, /*bOffset=*/ 1, /*dstOffset=*/ 10).execute(context),
).rejects.toThrow(TagCheckError);
it('Works on field elements', async () => {
context.machineState.memory.setSlice(0, [new Field(1), new Field(2), new Field(0)]);

[
new Lt(/*indirect=*/ 0, TypeTag.FIELD, /*aOffset=*/ 0, /*bOffset=*/ 0, /*dstOffset=*/ 10),
new Lt(/*indirect=*/ 0, TypeTag.FIELD, /*aOffset=*/ 0, /*bOffset=*/ 1, /*dstOffset=*/ 11),
new Lt(/*indirect=*/ 0, TypeTag.FIELD, /*aOffset=*/ 0, /*bOffset=*/ 2, /*dstOffset=*/ 12),
].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)]);
});

it('InTag is checked', async () => {
context.machineState.memory.setSlice(0, [new Field(1), new Uint32(2), new Uint16(3)]);

Expand Down Expand Up @@ -166,10 +174,17 @@ describe('Comparators', () => {
expect(actual).toEqual([new Uint32(1), new Uint32(1), new Uint32(0)]);
});

it('Does not work on field elements', async () => {
await expect(() =>
new Lte(/*indirect=*/ 0, TypeTag.FIELD, /*aOffset=*/ 0, /*bOffset=*/ 1, /*dstOffset=*/ 10).execute(context),
).rejects.toThrow(TagCheckError);
it('Works on field elements', async () => {
context.machineState.memory.setSlice(0, [new Field(1), new Field(2), new Field(0)]);

[
new Lte(/*indirect=*/ 0, TypeTag.FIELD, /*aOffset=*/ 0, /*bOffset=*/ 0, /*dstOffset=*/ 10),
new Lte(/*indirect=*/ 0, TypeTag.FIELD, /*aOffset=*/ 0, /*bOffset=*/ 1, /*dstOffset=*/ 11),
new Lte(/*indirect=*/ 0, TypeTag.FIELD, /*aOffset=*/ 0, /*bOffset=*/ 2, /*dstOffset=*/ 12),
].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)]);
});

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

Expand Down Expand Up @@ -35,10 +34,9 @@ export class Lt extends ThreeOperandInstruction {

async execute(context: AvmContext): Promise<void> {
context.machineState.memory.checkTags(this.inTag, this.aOffset, this.bOffset);
TaggedMemory.checkIsIntegralTag(this.inTag);

const a = context.machineState.memory.getAs<IntegralValue>(this.aOffset);
const b = context.machineState.memory.getAs<IntegralValue>(this.bOffset);
const a = context.machineState.memory.get(this.aOffset);
const b = context.machineState.memory.get(this.bOffset);

// Result will be of the same type as 'a'.
const dest = a.build(a.lt(b) ? 1n : 0n);
Expand All @@ -58,10 +56,9 @@ export class Lte extends ThreeOperandInstruction {

async execute(context: AvmContext): Promise<void> {
context.machineState.memory.checkTags(this.inTag, this.aOffset, this.bOffset);
TaggedMemory.checkIsIntegralTag(this.inTag);

const a = context.machineState.memory.getAs<IntegralValue>(this.aOffset);
const b = context.machineState.memory.getAs<IntegralValue>(this.bOffset);
const a = context.machineState.memory.get(this.aOffset);
const b = context.machineState.memory.get(this.bOffset);

// Result will be of the same type as 'a'.
const dest = a.build(a.equals(b) || a.lt(b) ? 1n : 0n);
Expand Down
4 changes: 2 additions & 2 deletions yellow-paper/docs/public-vm/gen/_instruction-set.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -591,7 +591,7 @@ Less-than check (a < b)
- **Category**: Compute - Comparators
- **Flags**:
- **indirect**: Toggles whether each memory-offset argument is an indirect offset. Rightmost bit corresponds to 0th offset arg, etc. Indirect offsets result in memory accesses like `M[M[offset]]` instead of the more standard `M[offset]`.
- **inTag**: The [tag/size](./memory-model#tags-and-tagged-memory) to check inputs against and tag the destination with. `field` type is NOT supported for this instruction.
- **inTag**: The [tag/size](./memory-model#tags-and-tagged-memory) to check inputs against and tag the destination with.
- **Args**:
- **aOffset**: memory offset of the operation's left input
- **bOffset**: memory offset of the operation's right input
Expand All @@ -612,7 +612,7 @@ Less-than-or-equals check (a <= b)
- **Category**: Compute - Comparators
- **Flags**:
- **indirect**: Toggles whether each memory-offset argument is an indirect offset. Rightmost bit corresponds to 0th offset arg, etc. Indirect offsets result in memory accesses like `M[M[offset]]` instead of the more standard `M[offset]`.
- **inTag**: The [tag/size](./memory-model#tags-and-tagged-memory) to check inputs against and tag the destination with. `field` type is NOT supported for this instruction.
- **inTag**: The [tag/size](./memory-model#tags-and-tagged-memory) to check inputs against and tag the destination with.
- **Args**:
- **aOffset**: memory offset of the operation's left input
- **bOffset**: memory offset of the operation's right input
Expand Down
4 changes: 2 additions & 2 deletions yellow-paper/src/preprocess/InstructionSet/InstructionSet.js
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ const INSTRUCTION_SET_RAW = [
"Category": "Compute - Comparators",
"Flags": [
{"name": "indirect", "description": INDIRECT_FLAG_DESCRIPTION},
{"name": "inTag", "description": IN_TAG_DESCRIPTION_NO_FIELD},
{"name": "inTag", "description": IN_TAG_DESCRIPTION},
],
"Args": [
{"name": "aOffset", "description": "memory offset of the operation's left input"},
Expand All @@ -149,7 +149,7 @@ const INSTRUCTION_SET_RAW = [
"Category": "Compute - Comparators",
"Flags": [
{"name": "indirect", "description": INDIRECT_FLAG_DESCRIPTION},
{"name": "inTag", "description": IN_TAG_DESCRIPTION_NO_FIELD},
{"name": "inTag", "description": IN_TAG_DESCRIPTION},
],
"Args": [
{"name": "aOffset", "description": "memory offset of the operation's left input"},
Expand Down

0 comments on commit ee21374

Please sign in to comment.