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.

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

chore(avm): remove field support for comparators and bitwise ops #4516

Merged
merged 2 commits into from
Feb 8, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
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
Loading