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.

Sign up for GitHub

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

feat(avm): simulator relative addr #8837

Merged
merged 2 commits into from
Oct 1, 2024
Merged

feat(avm): simulator relative addr #8837

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6a1d8ed
feat(avm): simulator relative addr
fcarreiro Sep 26, 2024
62ee1c3
fix mem ops tracking
fcarreiro Oct 1, 2024
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
15 changes: 10 additions & 5 deletions yarn-project/simulator/src/avm/avm_memory_types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -413,7 +413,7 @@ export class TaggedMemory implements TaggedMemoryInterface {
}

/** No-op. Implemented here for compatibility with the MeteredTaggedMemory. */
public assert(_operations: Partial<MemoryOperations & { indirect: number }>) {}
public assert(_operations: Partial<MemoryOperations & { addressing: Addressing }>) {}
}

/** Tagged memory wrapper with metering for each memory read and write operation. */
Expand All @@ -435,10 +435,15 @@ export class MeteredTaggedMemory implements TaggedMemoryInterface {
* Asserts that the exact number of memory operations have been performed.
* Indirect represents the flags for indirect accesses: each bit set to one counts as an extra read.
*/
public assert(operations: Partial<MemoryOperations & { indirect: number }>) {
const { reads: expectedReads, writes: expectedWrites, indirect } = { reads: 0, writes: 0, ...operations };

const totalExpectedReads = expectedReads + Addressing.fromWire(indirect ?? 0).count(AddressingMode.INDIRECT);
public assert(operations: Partial<MemoryOperations & { addressing: Addressing }>) {
const {
reads: expectedReads,
writes: expectedWrites,
addressing,
} = { reads: 0, writes: 0, addressing: new Addressing([]), ...operations };

const totalExpectedReads =
expectedReads + addressing.count(AddressingMode.INDIRECT) + addressing.count(AddressingMode.RELATIVE);
const { reads: actualReads, writes: actualWrites } = this.reset();
if (actualReads !== totalExpectedReads) {
throw new InstructionExecutionError(
Expand Down
64 changes: 28 additions & 36 deletions yarn-project/simulator/src/avm/opcodes/accrued_substate.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,13 +28,11 @@ export class NoteHashExists extends Instruction {
}

public async execute(context: AvmContext): Promise<void> {
const memoryOperations = { reads: 2, writes: 1, indirect: this.indirect };
const memory = context.machineState.memory.track(this.type);
context.machineState.consumeGas(this.gasCost());
const [noteHashOffset, leafIndexOffset, existsOffset] = Addressing.fromWire(this.indirect).resolve(
[this.noteHashOffset, this.leafIndexOffset, this.existsOffset],
memory,
);
const operands = [this.noteHashOffset, this.leafIndexOffset, this.existsOffset];
const addressing = Addressing.fromWire(this.indirect, operands.length);
const [noteHashOffset, leafIndexOffset, existsOffset] = addressing.resolve(operands, memory);
memory.checkTags(TypeTag.FIELD, noteHashOffset, leafIndexOffset);

// Note that this instruction accepts any type in memory, and converts to Field.
Expand All @@ -48,7 +46,7 @@ export class NoteHashExists extends Instruction {
);
memory.set(existsOffset, exists ? new Uint8(1) : new Uint8(0));

memory.assert(memoryOperations);
memory.assert({ reads: 2, writes: 1, addressing });
context.machineState.incrementPc();
}
}
Expand All @@ -64,11 +62,12 @@ export class EmitNoteHash extends Instruction {
}

public async execute(context: AvmContext): Promise<void> {
const memoryOperations = { reads: 1, indirect: this.indirect };
const memory = context.machineState.memory.track(this.type);
context.machineState.consumeGas(this.gasCost());

const [noteHashOffset] = Addressing.fromWire(this.indirect).resolve([this.noteHashOffset], memory);
const operands = [this.noteHashOffset];
const addressing = Addressing.fromWire(this.indirect, operands.length);
const [noteHashOffset] = addressing.resolve(operands, memory);
memory.checkTag(TypeTag.FIELD, noteHashOffset);

if (context.environment.isStaticCall) {
Expand All @@ -78,7 +77,7 @@ export class EmitNoteHash extends Instruction {
const noteHash = memory.get(noteHashOffset).toFr();
context.persistableState.writeNoteHash(context.environment.storageAddress, noteHash);

memory.assert(memoryOperations);
memory.assert({ reads: 1, addressing });
context.machineState.incrementPc();
}
}
Expand All @@ -105,14 +104,12 @@ export class NullifierExists extends Instruction {
}

public async execute(context: AvmContext): Promise<void> {
const memoryOperations = { reads: 2, writes: 1, indirect: this.indirect };
const memory = context.machineState.memory.track(this.type);
context.machineState.consumeGas(this.gasCost());

const [nullifierOffset, addressOffset, existsOffset] = Addressing.fromWire(this.indirect).resolve(
[this.nullifierOffset, this.addressOffset, this.existsOffset],
memory,
);
const operands = [this.nullifierOffset, this.addressOffset, this.existsOffset];
const addressing = Addressing.fromWire(this.indirect, operands.length);
const [nullifierOffset, addressOffset, existsOffset] = addressing.resolve(operands, memory);
memory.checkTags(TypeTag.FIELD, nullifierOffset, addressOffset);

const nullifier = memory.get(nullifierOffset).toFr();
Expand All @@ -121,7 +118,7 @@ export class NullifierExists extends Instruction {

memory.set(existsOffset, exists ? new Uint8(1) : new Uint8(0));

memory.assert(memoryOperations);
memory.assert({ reads: 2, writes: 1, addressing });
context.machineState.incrementPc();
}
}
Expand All @@ -141,11 +138,12 @@ export class EmitNullifier extends Instruction {
throw new StaticCallAlterationError();
}

const memoryOperations = { reads: 1, indirect: this.indirect };
const memory = context.machineState.memory.track(this.type);
context.machineState.consumeGas(this.gasCost());

const [nullifierOffset] = Addressing.fromWire(this.indirect).resolve([this.nullifierOffset], memory);
const operands = [this.nullifierOffset];
const addressing = Addressing.fromWire(this.indirect, operands.length);
const [nullifierOffset] = addressing.resolve(operands, memory);
memory.checkTag(TypeTag.FIELD, nullifierOffset);

const nullifier = memory.get(nullifierOffset).toFr();
Expand All @@ -162,7 +160,7 @@ export class EmitNullifier extends Instruction {
}
}

memory.assert(memoryOperations);
memory.assert({ reads: 1, addressing });
context.machineState.incrementPc();
}
}
Expand All @@ -189,14 +187,12 @@ export class L1ToL2MessageExists extends Instruction {
}

public async execute(context: AvmContext): Promise<void> {
const memoryOperations = { reads: 2, writes: 1, indirect: this.indirect };
const memory = context.machineState.memory.track(this.type);
context.machineState.consumeGas(this.gasCost());

const [msgHashOffset, msgLeafIndexOffset, existsOffset] = Addressing.fromWire(this.indirect).resolve(
[this.msgHashOffset, this.msgLeafIndexOffset, this.existsOffset],
memory,
);
const operands = [this.msgHashOffset, this.msgLeafIndexOffset, this.existsOffset];
const addressing = Addressing.fromWire(this.indirect, operands.length);
const [msgHashOffset, msgLeafIndexOffset, existsOffset] = addressing.resolve(operands, memory);
memory.checkTags(TypeTag.FIELD, msgHashOffset, msgLeafIndexOffset);

const msgHash = memory.get(msgHashOffset).toFr();
Expand All @@ -208,7 +204,7 @@ export class L1ToL2MessageExists extends Instruction {
);
memory.set(existsOffset, exists ? new Uint8(1) : new Uint8(0));

memory.assert(memoryOperations);
memory.assert({ reads: 2, writes: 1, addressing });
context.machineState.incrementPc();
}
}
Expand All @@ -230,22 +226,20 @@ export class EmitUnencryptedLog extends Instruction {

const memory = context.machineState.memory.track(this.type);

const [logOffset, logSizeOffset] = Addressing.fromWire(this.indirect).resolve(
[this.logOffset, this.logSizeOffset],
memory,
);
const operands = [this.logOffset, this.logSizeOffset];
const addressing = Addressing.fromWire(this.indirect, operands.length);
const [logOffset, logSizeOffset] = addressing.resolve(operands, memory);
memory.checkTag(TypeTag.UINT32, logSizeOffset);
const logSize = memory.get(logSizeOffset).toNumber();
memory.checkTagsRange(TypeTag.FIELD, logOffset, logSize);

const contractAddress = context.environment.address;

const memoryOperations = { reads: 1 + logSize, indirect: this.indirect };
context.machineState.consumeGas(this.gasCost(logSize));
const log = memory.getSlice(logOffset, logSize).map(f => f.toFr());
context.persistableState.writeUnencryptedLog(contractAddress, log);

memory.assert(memoryOperations);
memory.assert({ reads: 1 + logSize, addressing });
context.machineState.incrementPc();
}
}
Expand All @@ -265,20 +259,18 @@ export class SendL2ToL1Message extends Instruction {
throw new StaticCallAlterationError();
}

const memoryOperations = { reads: 2, indirect: this.indirect };
const memory = context.machineState.memory.track(this.type);
context.machineState.consumeGas(this.gasCost());

const [recipientOffset, contentOffset] = Addressing.fromWire(this.indirect).resolve(
[this.recipientOffset, this.contentOffset],
memory,
);
const operands = [this.recipientOffset, this.contentOffset];
const addressing = Addressing.fromWire(this.indirect, operands.length);
const [recipientOffset, contentOffset] = addressing.resolve(operands, memory);

const recipient = memory.get(recipientOffset).toFr();
const content = memory.get(contentOffset).toFr();
context.persistableState.writeL2ToL1Message(recipient, content);

memory.assert(memoryOperations);
memory.assert({ reads: 2, addressing });
context.machineState.incrementPc();
}
}
51 changes: 27 additions & 24 deletions yarn-project/simulator/src/avm/opcodes/addressing_mode.ts
View file
Open in desktop
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hard to parse what's going on in to/from wire here. Not going to block merge, but some additional comments would be helpful. If this might change again soon, then I'm fine with procrastinating on more comments for now.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes you are right that it's not explicit in the code, I will add some comments in some other PR. The encoding is explained here: https://docs.google.com/document/d/1uiSp8Q2Pu8L2UCba1PPPNQfxxK3d78_-vrzud7PECnw/edit#bookmark=id.wmo1rdovzz9f

Original file line number Diff line number Diff line change
Expand Up @@ -3,26 +3,28 @@ import { strict as assert } from 'assert';
import { type TaggedMemoryInterface } from '../avm_memory_types.js';

export enum AddressingMode {
DIRECT,
INDIRECT,
INDIRECT_PLUS_CONSTANT, // Not implemented yet.
DIRECT = 0,
INDIRECT = 1,
RELATIVE = 2,
INDIRECT_RELATIVE = 3,
}

/** A class to represent the addressing mode of an instruction. */
export class Addressing {
public constructor(
/** The addressing mode for each operand. The length of this array is the number of operands of the instruction. */
private readonly modePerOperand: AddressingMode[],
) {
assert(modePerOperand.length <= 8, 'At most 8 operands are supported');
}
) {}

public static fromWire(wireModes: number): Addressing {
// TODO(facundo): 8 for backwards compatibility.
public static fromWire(wireModes: number, numOperands: number = 8): Addressing {
// The modes are stored in the wire format as a byte, with each bit representing the mode for an operand.
// The least significant bit represents the zeroth operand, and the most significant bit represents the last operand.
const modes = new Array<AddressingMode>(8);
for (let i = 0; i < 8; i++) {
modes[i] = (wireModes & (1 << i)) === 0 ? AddressingMode.DIRECT : AddressingMode.INDIRECT;
const modes = new Array<AddressingMode>(numOperands);
for (let i = 0; i < numOperands; i++) {
modes[i] =
(((wireModes >> i) & 1) * AddressingMode.INDIRECT) |
(((wireModes >> (i + numOperands)) & 1) * AddressingMode.RELATIVE);
}
Comment on lines -23 to 28
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the + numOperands doing here? Is that basically saying "'relative' bits start at bit index numOperands"? Does this need a special case for instructions like *CALL that have >4 operands and won't be able to enable relative addressing (at least not for all operands)?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes that what it means!

Does this need a special case for instructions like *CALL that have >4

It does not need special casing here, we just need to make the wire format indirect be a u16 for those operations to be usable in relative mode. Also mentioned here: https://docs.google.com/document/d/1uiSp8Q2Pu8L2UCba1PPPNQfxxK3d78_-vrzud7PECnw/edit#bookmark=id.wmo1rdovzz9f

return new Addressing(modes);
}
Expand All @@ -31,17 +33,20 @@ export class Addressing {
// The modes are stored in the wire format as a byte, with each bit representing the mode for an operand.
// The least significant bit represents the zeroth operand, and the least significant bit represents the last operand.
let wire: number = 0;
for (let i = 0; i < 8; i++) {
if (this.modePerOperand[i] === AddressingMode.INDIRECT) {
for (let i = 0; i < this.modePerOperand.length; i++) {
if (this.modePerOperand[i] & AddressingMode.INDIRECT) {
wire |= 1 << i;
}
if (this.modePerOperand[i] & AddressingMode.RELATIVE) {
wire |= 1 << (this.modePerOperand.length + i);
}
}
return wire;
}

/** Returns how many operands use the given addressing mode. */
public count(mode: AddressingMode): number {
return this.modePerOperand.filter(m => m === mode).length;
return this.modePerOperand.filter(m => (m & mode) !== 0).length;
}

/**
Expand All @@ -54,17 +59,15 @@ export class Addressing {
assert(offsets.length <= this.modePerOperand.length);
const resolved = new Array(offsets.length);
for (const [i, offset] of offsets.entries()) {
switch (this.modePerOperand[i]) {
case AddressingMode.INDIRECT:
// NOTE(reviewer): less than equal is a deviation from the spec - i dont see why this shouldnt be possible!
mem.checkIsValidMemoryOffsetTag(offset);
resolved[i] = Number(mem.get(offset).toBigInt());
break;
case AddressingMode.DIRECT:
resolved[i] = offset;
break;
default:
throw new Error(`Unimplemented addressing mode: ${AddressingMode[this.modePerOperand[i]]}`);
const mode = this.modePerOperand[i];
resolved[i] = offset;
if (mode & AddressingMode.RELATIVE) {
mem.checkIsValidMemoryOffsetTag(0);
resolved[i] += Number(mem.get(0).toBigInt());
}
if (mode & AddressingMode.INDIRECT) {
mem.checkIsValidMemoryOffsetTag(resolved[i]);
resolved[i] = Number(mem.get(resolved[i]).toBigInt());
}
Comment on lines 61 to 71
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess this is the crux of the PR. Looks good to me!

}
return resolved;
Expand Down
27 changes: 27 additions & 0 deletions yarn-project/simulator/src/avm/opcodes/arithmetic.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { type AvmContext } from '../avm_context.js';
import { Field, TypeTag, Uint8, Uint16, Uint32, Uint64, Uint128 } from '../avm_memory_types.js';
import { initContext } from '../fixtures/index.js';
import { Opcode } from '../serialization/instruction_serialization.js';
import { Addressing, AddressingMode } from './addressing_mode.js';
import { Add, Div, FieldDiv, Mul, Sub } from './arithmetic.js';

describe('Arithmetic Instructions', () => {
Expand Down Expand Up @@ -54,6 +55,32 @@ describe('Arithmetic Instructions', () => {
});
});

it('Should add in relative indirect mode', async () => {
const a = new Field(1n);
const b = new Field(2n);

context.machineState.memory.set(10, a);
context.machineState.memory.set(11, b);

context.machineState.memory.set(0, new Uint32(30)); // stack pointer
context.machineState.memory.set(32, new Uint32(5)); // indirect

await new Add(
/*indirect=*/ new Addressing([
/*aOffset*/ AddressingMode.DIRECT,
/*bOffset*/ AddressingMode.DIRECT,
/*dstOffset*/ AddressingMode.INDIRECT | AddressingMode.RELATIVE,
]).toWire(),
/*inTag=*/ TypeTag.FIELD,
/*aOffset=*/ 10,
/*bOffset=*/ 11,
/*dstOffset=*/ 2, // We expect the result to be stored at MEM[30 + 2] = 5
).execute(context);

const actual = context.machineState.memory.get(5);
expect(actual).toEqual(new Field(3n));
});

describe.each([
[new Field((Field.MODULUS + 1n) / 2n), new Field(1n), TypeTag.FIELD],
[new Uint8((1n << 7n) + 1n), new Uint8(2n), TypeTag.UINT8],
Expand Down
10 changes: 4 additions & 6 deletions yarn-project/simulator/src/avm/opcodes/arithmetic.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,12 @@ import { ThreeOperandInstruction } from './instruction_impl.js';

export abstract class ThreeOperandArithmeticInstruction extends ThreeOperandInstruction {
public async execute(context: AvmContext): Promise<void> {
const memoryOperations = { reads: 2, writes: 1, indirect: this.indirect };
const memory = context.machineState.memory.track(this.type);
context.machineState.consumeGas(this.gasCost());

const [aOffset, bOffset, dstOffset] = Addressing.fromWire(this.indirect).resolve(
[this.aOffset, this.bOffset, this.dstOffset],
memory,
);
const operands = [this.aOffset, this.bOffset, this.dstOffset];
const addressing = Addressing.fromWire(this.indirect, operands.length);
const [aOffset, bOffset, dstOffset] = addressing.resolve(operands, memory);
memory.checkTags(this.inTag, aOffset, bOffset);

const a = memory.get(aOffset);
Expand All @@ -22,7 +20,7 @@ export abstract class ThreeOperandArithmeticInstruction extends ThreeOperandInst
const dest = this.compute(a, b);
memory.set(dstOffset, dest);

memory.assert(memoryOperations);
memory.assert({ reads: 2, writes: 1, addressing });
context.machineState.incrementPc();
}

Expand Down
Loading
Loading