Skip to content

Commit

Permalink
chore(avm): remove field support for comparators and bitwise ops (#4516)
Browse files Browse the repository at this point in the history
  • Loading branch information
fcarreiro authored Feb 8, 2024
1 parent 3750b26 commit 87a9663
Show file tree
Hide file tree
Showing 7 changed files with 51 additions and 58 deletions.
7 changes: 0 additions & 7 deletions yarn-project/simulator/src/avm/avm_memory_types.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -175,13 +175,6 @@ 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
15 changes: 9 additions & 6 deletions yarn-project/simulator/src/avm/avm_memory_types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ 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 @@ -32,6 +31,8 @@ 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: MemoryValue): boolean;
}

// TODO: Optimize calculation of mod, etc. Can only do once per class?
Expand Down Expand Up @@ -200,10 +201,6 @@ 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 Expand Up @@ -284,7 +281,13 @@ export class TaggedMemory {
*/
public checkTag(tag: TypeTag, offset: number) {
if (this.getTag(offset) !== tag) {
throw new TagCheckError(offset, TypeTag[this.getTag(offset)], TypeTag[tag]);
throw TagCheckError.forOffset(offset, TypeTag[this.getTag(offset)], TypeTag[tag]);
}
}

public static checkIsIntegralTag(tag: TypeTag) {
if (![TypeTag.UINT8, TypeTag.UINT16, TypeTag.UINT32, TypeTag.UINT64, TypeTag.UINT128].includes(tag)) {
throw TagCheckError.forTag(TypeTag[tag], 'integral');
}
}

Expand Down
12 changes: 10 additions & 2 deletions yarn-project/simulator/src/avm/errors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,16 @@ export class InstructionExecutionError extends AvmExecutionError {
* Error thrown on failed AVM memory tag check.
*/
export class TagCheckError extends AvmExecutionError {
constructor(offset: number, gotTag: string, expectedTag: string) {
super(`Memory offset ${offset} has tag ${gotTag}, expected ${expectedTag}`);
public static forOffset(offset: number, gotTag: string, expectedTag: string): TagCheckError {
return new TagCheckError(`Tag mismatch at offset ${offset}, got ${gotTag}, expected ${expectedTag}`);
}

public static forTag(gotTag: string, expectedTag: string): TagCheckError {
return new TagCheckError(`Tag mismatch, got ${gotTag}, expected ${expectedTag}`);
}

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

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('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('InTag is checked', async () => {
context.machineState.memory.setSlice(0, [new Field(1), new Uint32(2), new Uint16(3)]);

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

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('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('InTag is checked', async () => {
Expand Down
11 changes: 7 additions & 4 deletions yarn-project/simulator/src/avm/opcodes/comparators.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
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 @@ -34,9 +35,10 @@ 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.get(this.aOffset);
const b = context.machineState.memory.get(this.bOffset);
const a = context.machineState.memory.getAs<IntegralValue>(this.aOffset);
const b = context.machineState.memory.getAs<IntegralValue>(this.bOffset);

// Result will be of the same type as 'a'.
const dest = a.build(a.lt(b) ? 1n : 0n);
Expand All @@ -56,9 +58,10 @@ 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.get(this.aOffset);
const b = context.machineState.memory.get(this.bOffset);
const a = context.machineState.memory.getAs<IntegralValue>(this.aOffset);
const b = context.machineState.memory.getAs<IntegralValue>(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
16 changes: 8 additions & 8 deletions yellow-paper/docs/public-vm/gen/_InstructionSet.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -576,7 +576,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](./state-model#tags-and-tagged-memory) to check inputs against and tag the destination with.
- **inTag**: The [tag/size](./state-model#tags-and-tagged-memory) to check inputs against and tag the destination with. `field` type is NOT supported for this instruction.
- **Args**:
- **aOffset**: memory offset of the operation's left input
- **bOffset**: memory offset of the operation's right input
Expand All @@ -597,7 +597,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](./state-model#tags-and-tagged-memory) to check inputs against and tag the destination with.
- **inTag**: The [tag/size](./state-model#tags-and-tagged-memory) to check inputs against and tag the destination with. `field` type is NOT supported for this instruction.
- **Args**:
- **aOffset**: memory offset of the operation's left input
- **bOffset**: memory offset of the operation's right input
Expand All @@ -618,7 +618,7 @@ Bitwise AND (a & b)
- **Category**: Compute - Bitwise
- **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](./state-model#tags-and-tagged-memory) to check inputs against and tag the destination with.
- **inTag**: The [tag/size](./state-model#tags-and-tagged-memory) to check inputs against and tag the destination with. `field` type is NOT supported for this instruction.
- **Args**:
- **aOffset**: memory offset of the operation's left input
- **bOffset**: memory offset of the operation's right input
Expand All @@ -639,7 +639,7 @@ Bitwise OR (a | b)
- **Category**: Compute - Bitwise
- **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](./state-model#tags-and-tagged-memory) to check inputs against and tag the destination with.
- **inTag**: The [tag/size](./state-model#tags-and-tagged-memory) to check inputs against and tag the destination with. `field` type is NOT supported for this instruction.
- **Args**:
- **aOffset**: memory offset of the operation's left input
- **bOffset**: memory offset of the operation's right input
Expand All @@ -660,7 +660,7 @@ Bitwise XOR (a ^ b)
- **Category**: Compute - Bitwise
- **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](./state-model#tags-and-tagged-memory) to check inputs against and tag the destination with.
- **inTag**: The [tag/size](./state-model#tags-and-tagged-memory) to check inputs against and tag the destination with. `field` type is NOT supported for this instruction.
- **Args**:
- **aOffset**: memory offset of the operation's left input
- **bOffset**: memory offset of the operation's right input
Expand All @@ -681,7 +681,7 @@ Bitwise NOT (inversion)
- **Category**: Compute - Bitwise
- **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](./state-model#tags-and-tagged-memory) to check inputs against and tag the destination with.
- **inTag**: The [tag/size](./state-model#tags-and-tagged-memory) to check inputs against and tag the destination with. `field` type is NOT supported for this instruction.
- **Args**:
- **aOffset**: memory offset of the operation's input
- **dstOffset**: memory offset specifying where to store operation's result
Expand All @@ -701,7 +701,7 @@ Bitwise leftward shift (a << b)
- **Category**: Compute - Bitwise
- **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](./state-model#tags-and-tagged-memory) to check inputs against and tag the destination with.
- **inTag**: The [tag/size](./state-model#tags-and-tagged-memory) to check inputs against and tag the destination with. `field` type is NOT supported for this instruction.
- **Args**:
- **aOffset**: memory offset of the operation's left input
- **bOffset**: memory offset of the operation's right input
Expand All @@ -722,7 +722,7 @@ Bitwise rightward shift (a >> b)
- **Category**: Compute - Bitwise
- **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](./state-model#tags-and-tagged-memory) to check inputs against and tag the destination with.
- **inTag**: The [tag/size](./state-model#tags-and-tagged-memory) to check inputs against and tag the destination with. `field` type is NOT supported for this instruction.
- **Args**:
- **aOffset**: memory offset of the operation's left input
- **bOffset**: memory offset of the operation's right input
Expand Down
17 changes: 9 additions & 8 deletions yellow-paper/src/preprocess/InstructionSet/InstructionSet.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ const TOPICS_IN_SECTIONS = [
];

const IN_TAG_DESCRIPTION = "The [tag/size](./state-model#tags-and-tagged-memory) to check inputs against and tag the destination with.";
const IN_TAG_DESCRIPTION_NO_FIELD = IN_TAG_DESCRIPTION + " `field` type is NOT supported for this instruction.";
const DST_TAG_DESCRIPTION = "The [tag/size](./state-model#tags-and-tagged-memory) to tag the destination with but not to check inputs against.";
const INDIRECT_FLAG_DESCRIPTION = "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]`.";

Expand Down Expand Up @@ -113,7 +114,7 @@ const INSTRUCTION_SET_RAW = [
"Category": "Compute - Comparators",
"Flags": [
{"name": "indirect", "description": INDIRECT_FLAG_DESCRIPTION},
{"name": "inTag", "description": IN_TAG_DESCRIPTION},
{"name": "inTag", "description": IN_TAG_DESCRIPTION_NO_FIELD},
],
"Args": [
{"name": "aOffset", "description": "memory offset of the operation's left input"},
Expand All @@ -132,7 +133,7 @@ const INSTRUCTION_SET_RAW = [
"Category": "Compute - Comparators",
"Flags": [
{"name": "indirect", "description": INDIRECT_FLAG_DESCRIPTION},
{"name": "inTag", "description": IN_TAG_DESCRIPTION},
{"name": "inTag", "description": IN_TAG_DESCRIPTION_NO_FIELD},
],
"Args": [
{"name": "aOffset", "description": "memory offset of the operation's left input"},
Expand All @@ -151,7 +152,7 @@ const INSTRUCTION_SET_RAW = [
"Category": "Compute - Bitwise",
"Flags": [
{"name": "indirect", "description": INDIRECT_FLAG_DESCRIPTION},
{"name": "inTag", "description": IN_TAG_DESCRIPTION},
{"name": "inTag", "description": IN_TAG_DESCRIPTION_NO_FIELD},
],
"Args": [
{"name": "aOffset", "description": "memory offset of the operation's left input"},
Expand All @@ -170,7 +171,7 @@ const INSTRUCTION_SET_RAW = [
"Category": "Compute - Bitwise",
"Flags": [
{"name": "indirect", "description": INDIRECT_FLAG_DESCRIPTION},
{"name": "inTag", "description": IN_TAG_DESCRIPTION},
{"name": "inTag", "description": IN_TAG_DESCRIPTION_NO_FIELD},
],
"Args": [
{"name": "aOffset", "description": "memory offset of the operation's left input"},
Expand All @@ -189,7 +190,7 @@ const INSTRUCTION_SET_RAW = [
"Category": "Compute - Bitwise",
"Flags": [
{"name": "indirect", "description": INDIRECT_FLAG_DESCRIPTION},
{"name": "inTag", "description": IN_TAG_DESCRIPTION},
{"name": "inTag", "description": IN_TAG_DESCRIPTION_NO_FIELD},
],
"Args": [
{"name": "aOffset", "description": "memory offset of the operation's left input"},
Expand All @@ -208,7 +209,7 @@ const INSTRUCTION_SET_RAW = [
"Category": "Compute - Bitwise",
"Flags": [
{"name": "indirect", "description": INDIRECT_FLAG_DESCRIPTION},
{"name": "inTag", "description": IN_TAG_DESCRIPTION},
{"name": "inTag", "description": IN_TAG_DESCRIPTION_NO_FIELD},
],
"Args": [
{"name": "aOffset", "description": "memory offset of the operation's input"},
Expand All @@ -226,7 +227,7 @@ const INSTRUCTION_SET_RAW = [
"Category": "Compute - Bitwise",
"Flags": [
{"name": "indirect", "description": INDIRECT_FLAG_DESCRIPTION},
{"name": "inTag", "description": IN_TAG_DESCRIPTION},
{"name": "inTag", "description": IN_TAG_DESCRIPTION_NO_FIELD},
],
"Args": [
{"name": "aOffset", "description": "memory offset of the operation's left input"},
Expand All @@ -245,7 +246,7 @@ const INSTRUCTION_SET_RAW = [
"Category": "Compute - Bitwise",
"Flags": [
{"name": "indirect", "description": INDIRECT_FLAG_DESCRIPTION},
{"name": "inTag", "description": IN_TAG_DESCRIPTION},
{"name": "inTag", "description": IN_TAG_DESCRIPTION_NO_FIELD},
],
"Args": [
{"name": "aOffset", "description": "memory offset of the operation's left input"},
Expand Down

0 comments on commit 87a9663

Please sign in to comment.