diff --git a/compiler/forget/src/CompilerPipeline.ts b/compiler/forget/src/CompilerPipeline.ts index 50ed36278f..cae551ad8e 100644 --- a/compiler/forget/src/CompilerPipeline.ts +++ b/compiler/forget/src/CompilerPipeline.ts @@ -58,6 +58,8 @@ export function* run( mergeConsecutiveBlocks(hir); yield log({ kind: "hir", name: "MergeConsecutiveBlocks", value: hir }); + validateConsistentIdentifiers(hir); + enterSSA(hir); yield log({ kind: "hir", name: "SSA", value: hir }); diff --git a/compiler/forget/src/HIR/BuildHIR.ts b/compiler/forget/src/HIR/BuildHIR.ts index 6d3ddb61c0..c7e7827837 100644 --- a/compiler/forget/src/HIR/BuildHIR.ts +++ b/compiler/forget/src/HIR/BuildHIR.ts @@ -1041,10 +1041,19 @@ function lowerExpression( // Block for the consequent (if the test is truthy) const consequentBlock = builder.enter("value", (_blockId) => { + const consequent = lowerExpressionToTemporary( + builder, + expr.get("consequent") + ); builder.push({ id: makeInstructionId(0), - lvalue: { ...place }, - value: lowerExpression(builder, expr.get("consequent")), + lvalue: buildTemporaryPlace(builder, exprLoc), + value: { + kind: "StoreLocal", + lvalue: { kind: InstructionKind.Const, place: { ...place } }, + value: consequent, + loc: exprLoc, + }, loc: exprLoc, }); return { @@ -1056,10 +1065,19 @@ function lowerExpression( }); // Block for the alternate (if the test is not truthy) const alternateBlock = builder.enter("value", (_blockId) => { + const alternate = lowerExpressionToTemporary( + builder, + expr.get("alternate") + ); builder.push({ id: makeInstructionId(0), - lvalue: { ...place }, - value: lowerExpression(builder, expr.get("alternate")), + lvalue: buildTemporaryPlace(builder, exprLoc), + value: { + kind: "StoreLocal", + lvalue: { kind: InstructionKind.Const, place: { ...place } }, + value: alternate, + loc: exprLoc, + }, loc: exprLoc, }); return { @@ -1106,10 +1124,11 @@ function lowerExpression( const consequent = builder.enter("value", () => { builder.push({ id: makeInstructionId(0), - lvalue: { ...place }, + lvalue: buildTemporaryPlace(builder, leftPlace.loc), value: { - kind: "LoadLocal", - place: { ...leftPlace }, + kind: "StoreLocal", + lvalue: { kind: InstructionKind.Const, place: { ...place } }, + value: { ...leftPlace }, loc: leftPlace.loc, }, loc: exprLoc, @@ -1122,10 +1141,16 @@ function lowerExpression( }; }); const alternate = builder.enter("value", () => { + const right = lowerExpressionToTemporary(builder, expr.get("right")); builder.push({ id: makeInstructionId(0), - lvalue: { ...place }, - value: lowerExpression(builder, expr.get("right")), + lvalue: buildTemporaryPlace(builder, right.loc), + value: { + kind: "StoreLocal", + lvalue: { kind: InstructionKind.Const, place: { ...place } }, + value: { ...right }, + loc: right.loc, + }, loc: exprLoc, }); return { diff --git a/compiler/forget/src/HIR/ValidateConsistentIdentifiers.ts b/compiler/forget/src/HIR/ValidateConsistentIdentifiers.ts index 52bff620af..6444ea7271 100644 --- a/compiler/forget/src/HIR/ValidateConsistentIdentifiers.ts +++ b/compiler/forget/src/HIR/ValidateConsistentIdentifiers.ts @@ -13,6 +13,7 @@ import { IdentifierId, SourceLocation, } from "./HIR"; +import { printPlace } from "./PrintHIR"; import { eachInstructionValueOperand, eachTerminalOperand } from "./visitors"; /** @@ -21,6 +22,7 @@ import { eachInstructionValueOperand, eachTerminalOperand } from "./visitors"; */ export function validateConsistentIdentifiers(fn: HIRFunction): void { const identifiers: Identifiers = new Map(); + const assignments: Set = new Set(); for (const [, block] of fn.body.blocks) { for (const phi of block.phis) { validate(identifiers, phi.id); @@ -35,6 +37,15 @@ export function validateConsistentIdentifiers(fn: HIRFunction): void { instr.lvalue.loc ); } + if (assignments.has(instr.lvalue.identifier.id)) { + CompilerError.invariant( + `Expected lvalues to be assigned exactly once, found duplicate assignment of '${printPlace( + instr.lvalue + )}'`, + instr.lvalue.loc + ); + } + assignments.add(instr.lvalue.identifier.id); validate(identifiers, instr.lvalue.identifier, instr.lvalue.loc); for (const operand of eachInstructionValueOperand(instr.value)) { validate(identifiers, operand.identifier, operand.loc); diff --git a/compiler/forget/src/ReactiveScopes/BuildReactiveFunction.ts b/compiler/forget/src/ReactiveScopes/BuildReactiveFunction.ts index 4dc5f48356..ca8b0757c0 100644 --- a/compiler/forget/src/ReactiveScopes/BuildReactiveFunction.ts +++ b/compiler/forget/src/ReactiveScopes/BuildReactiveFunction.ts @@ -519,16 +519,9 @@ class Driver { loc: SourceLocation ): { block: BlockId; value: ReactiveValue; place: Place; id: InstructionId } { const defaultBlock = this.cx.ir.blocks.get(id)!; - if ( - defaultBlock.terminal.kind === "goto" || - defaultBlock.terminal.kind === "branch" - ) { + if (defaultBlock.terminal.kind === "branch") { const instructions = defaultBlock.instructions; if (instructions.length === 0) { - invariant( - defaultBlock.terminal.kind === "branch", - "Expected instructions for non-branch terminal" - ); return { block: defaultBlock.id, place: defaultBlock.terminal.test, @@ -541,6 +534,11 @@ class Driver { }; } else if (defaultBlock.instructions.length === 1) { const instr = defaultBlock.instructions[0]!; + invariant( + instr.lvalue.identifier.id === + defaultBlock.terminal.test.identifier.id, + "Expected branch block to end in an instruction that sets the test value" + ); return { block: defaultBlock.id, place: instr.lvalue!, @@ -558,7 +556,58 @@ class Driver { }; return { block: defaultBlock.id, - place: instr.lvalue!, + place: defaultBlock.terminal.test, + value: sequence, + id: defaultBlock.terminal.id, + }; + } + } else if (defaultBlock.terminal.kind === "goto") { + const instructions = defaultBlock.instructions; + if (instructions.length === 0) { + invariant( + false, + "Expected goto value block to have at least one instruction" + ); + } else if (defaultBlock.instructions.length === 1) { + const instr = defaultBlock.instructions[0]!; + let place: Place = instr.lvalue!; + let value: ReactiveValue = instr.value; + if (instr.value.kind === "StoreLocal") { + place = instr.value.lvalue.place; + value = { + kind: "LoadLocal", + place: instr.value.value, + loc: instr.value.value.loc, + }; + } + return { + block: defaultBlock.id, + place, + value, + id: instr.id, + }; + } else { + const instr = defaultBlock.instructions.at(-1)!; + let place: Place = instr.lvalue!; + let value: ReactiveValue = instr.value; + if (instr.value.kind === "StoreLocal") { + place = instr.value.lvalue.place; + value = { + kind: "LoadLocal", + place: instr.value.value, + loc: instr.value.value.loc, + }; + } + const sequence: ReactiveSequenceValue = { + kind: "SequenceExpression", + instructions: defaultBlock.instructions.slice(0, -1), + id: instr.id, + value, + loc: loc, + }; + return { + block: defaultBlock.id, + place, value: sequence, id: instr.id, }; @@ -675,7 +724,7 @@ class Driver { }; invariant( consequent.place.identifier === alternate.place.identifier, - "Expected the consquent and alternate of a ternary to store a value to the same place" + "Expected the consequent and alternate of a ternary to store a value to the same place" ); return { place: { ...consequent.place },