Skip to content
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
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,9 @@ export function assertConsistentIdentifiers(fn: HIRFunction): void {
const assignments: Set<IdentifierId> = new Set();
for (const [, block] of fn.body.blocks) {
for (const phi of block.phis) {
validate(identifiers, phi.id);
validate(identifiers, phi.place.identifier);
for (const [, operand] of phi.operands) {
validate(identifiers, operand);
validate(identifiers, operand.identifier);
}
}
for (const instr of block.instructions) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,9 @@ import {
export function assertValidMutableRanges(fn: HIRFunction): void {
for (const [, block] of fn.body.blocks) {
for (const phi of block.phis) {
visitIdentifier(phi.id);
visitIdentifier(phi.place.identifier);
for (const [, operand] of phi.operands) {
visitIdentifier(operand);
visitIdentifier(operand.identifier);
}
}
for (const instr of block.instructions) {
Expand Down
4 changes: 2 additions & 2 deletions compiler/packages/babel-plugin-react-compiler/src/HIR/HIR.ts
Original file line number Diff line number Diff line change
Expand Up @@ -761,8 +761,8 @@ function _staticInvariantInstructionValueHasLocation(

export type Phi = {
kind: 'Phi';
id: Identifier;
operands: Map<BlockId, Identifier>;
place: Place;
operands: Map<BlockId, Place>;
};

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,20 +84,14 @@ export function mergeConsecutiveBlocks(fn: HIRFunction): void {
id: predecessor.terminal.id,
lvalue: {
kind: 'Identifier',
identifier: phi.id,
identifier: phi.place.identifier,
effect: Effect.ConditionallyMutate,
reactive: false,
loc: GeneratedSource,
},
value: {
kind: 'LoadLocal',
place: {
kind: 'Identifier',
identifier: operand,
effect: Effect.Read,
reactive: false,
loc: GeneratedSource,
},
place: {...operand},
loc: GeneratedSource,
},
loc: GeneratedSource,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -163,13 +163,13 @@ export function printInstruction(instr: ReactiveInstruction): string {

export function printPhi(phi: Phi): string {
const items = [];
items.push(printIdentifier(phi.id));
items.push(printMutableRange(phi.id));
items.push(printType(phi.id.type));
items.push(printPlace(phi.place));
items.push(printMutableRange(phi.place.identifier));
items.push(printType(phi.place.identifier.type));
items.push(': phi(');
const phis = [];
for (const [blockId, id] of phi.operands) {
phis.push(`bb${blockId}: ${printIdentifier(id)}`);
for (const [blockId, place] of phi.operands) {
phis.push(`bb${blockId}: ${printPlace(place)}`);
}

items.push(phis.join(', '));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -607,7 +607,7 @@ function collectDependencies(
// Record referenced optional chains in phis
for (const phi of block.phis) {
for (const operand of phi.operands) {
const maybeOptionalChain = temporaries.get(operand[1].id);
const maybeOptionalChain = temporaries.get(operand[1].identifier.id);
if (maybeOptionalChain) {
context.visitDependency(maybeOptionalChain);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,11 @@ export function inferAliasForPhis(
for (const [_, block] of func.body.blocks) {
for (const phi of block.phis) {
const isPhiMutatedAfterCreation: boolean =
phi.id.mutableRange.end >
phi.place.identifier.mutableRange.end >
(block.instructions.at(0)?.id ?? block.terminal.id);
if (isPhiMutatedAfterCreation) {
for (const [, operand] of phi.operands) {
aliases.union([phi.id, operand]);
aliases.union([phi.place.identifier, operand.identifier]);
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -116,19 +116,23 @@ export function inferMutableLifetimes(
for (const [_, block] of func.body.blocks) {
for (const phi of block.phis) {
const isPhiMutatedAfterCreation: boolean =
phi.id.mutableRange.end >
phi.place.identifier.mutableRange.end >
(block.instructions.at(0)?.id ?? block.terminal.id);
if (
inferMutableRangeForStores &&
isPhiMutatedAfterCreation &&
phi.id.mutableRange.start === 0
phi.place.identifier.mutableRange.start === 0
) {
for (const [, operand] of phi.operands) {
if (phi.id.mutableRange.start === 0) {
phi.id.mutableRange.start = operand.mutableRange.start;
if (phi.place.identifier.mutableRange.start === 0) {
phi.place.identifier.mutableRange.start =
operand.identifier.mutableRange.start;
} else {
phi.id.mutableRange.start = makeInstructionId(
Math.min(phi.id.mutableRange.start, operand.mutableRange.start),
phi.place.identifier.mutableRange.start = makeInstructionId(
Math.min(
phi.place.identifier.mutableRange.start,
operand.identifier.mutableRange.start,
),
);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -162,23 +162,23 @@ export function inferReactivePlaces(fn: HIRFunction): void {
let hasReactiveControl = isReactiveControlledBlock(block.id);

for (const phi of block.phis) {
if (reactiveIdentifiers.isReactiveIdentifier(phi.id)) {
if (reactiveIdentifiers.isReactive(phi.place)) {
// Already marked reactive on a previous pass
continue;
}
let isPhiReactive = false;
for (const [, operand] of phi.operands) {
if (reactiveIdentifiers.isReactiveIdentifier(operand)) {
if (reactiveIdentifiers.isReactive(operand)) {
isPhiReactive = true;
break;
}
}
if (isPhiReactive) {
reactiveIdentifiers.markReactiveIdentifier(phi.id);
reactiveIdentifiers.markReactive(phi.place);
} else {
for (const [pred] of phi.operands) {
if (isReactiveControlledBlock(pred)) {
reactiveIdentifiers.markReactiveIdentifier(phi.id);
Copy link
Member

Choose a reason for hiding this comment

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

awesome! would be nice to remove markReactiveIdentifier if possible (fine for a follow-up)

reactiveIdentifiers.markReactive(phi.place);
break;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -635,7 +635,7 @@ class InferenceState {
inferPhi(phi: Phi): void {
const values: Set<InstructionValue> = new Set();
for (const [_, operand] of phi.operands) {
const operandValues = this.#variables.get(operand.id);
const operandValues = this.#variables.get(operand.identifier.id);
// This is a backedge that will be handled later by State.merge
if (operandValues === undefined) continue;
for (const v of operandValues) {
Expand All @@ -644,7 +644,7 @@ class InferenceState {
}

if (values.size > 0) {
this.#variables.set(phi.id.id, values);
this.#variables.set(phi.place.identifier.id, values);
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ function applyConstantPropagation(
for (const phi of block.phis) {
let value = evaluatePhi(phi, constants);
if (value !== null) {
constants.set(phi.id.id, value);
constants.set(phi.place.identifier.id, value);
}
}

Expand Down Expand Up @@ -167,7 +167,7 @@ function applyConstantPropagation(
function evaluatePhi(phi: Phi, constants: Constants): Constant | null {
let value: Constant | null = null;
for (const [, operand] of phi.operands) {
const operandValue = constants.get(operand.id) ?? null;
const operandValue = constants.get(operand.identifier.id) ?? null;
// did not find a constant, can't constant propogate
if (operandValue === null) {
return null;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ export function deadCodeElimination(fn: HIRFunction): void {
*/
for (const [, block] of fn.body.blocks) {
for (const phi of block.phis) {
if (!state.isIdOrNameUsed(phi.id)) {
if (!state.isIdOrNameUsed(phi.place.identifier)) {
block.phis.delete(phi);
}
}
Expand Down Expand Up @@ -159,9 +159,9 @@ function findReferencedIdentifiers(fn: HIRFunction): State {
}
}
for (const phi of block.phis) {
if (state.isIdOrNameUsed(phi.id)) {
if (state.isIdOrNameUsed(phi.place.identifier)) {
for (const [_pred, operand] of phi.operands) {
state.reference(operand);
state.reference(operand.identifier);
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ import {
removeUnnecessaryTryCatch,
removeUnreachableForUpdates,
} from '../HIR/HIRBuilder';
import {printIdentifier} from '../HIR/PrintHIR';
import {printPlace} from '../HIR/PrintHIR';

/*
* This pass prunes `maybe-throw` terminals for blocks that can provably *never* throw.
Expand Down Expand Up @@ -55,7 +55,7 @@ export function pruneMaybeThrows(fn: HIRFunction): void {
loc: GeneratedSource,
description: `Could not find mapping for predecessor bb${predecessor} in block bb${
block.id
} for phi ${printIdentifier(phi.id)}`,
} for phi ${printPlace(phi.place)}`,
suggestions: null,
});
phi.operands.delete(predecessor);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -281,22 +281,25 @@ export function findDisjointMutableValues(
*/
for (const phi of block.phis) {
if (
phi.id.mutableRange.start + 1 !== phi.id.mutableRange.end &&
phi.id.mutableRange.end >
phi.place.identifier.mutableRange.start + 1 !==
phi.place.identifier.mutableRange.end &&
phi.place.identifier.mutableRange.end >
(block.instructions.at(0)?.id ?? block.terminal.id)
) {
const operands = [phi.id];
const declaration = declarations.get(phi.id.declarationId);
const operands = [phi.place.identifier];
const declaration = declarations.get(
phi.place.identifier.declarationId,
);
if (declaration !== undefined) {
operands.push(declaration);
}
for (const [_, phiId] of phi.operands) {
operands.push(phiId);
operands.push(phiId.identifier);
}
scopeIdentifiers.union(operands);
} else if (fn.env.config.enableForest) {
for (const [, phiId] of phi.operands) {
scopeIdentifiers.union([phi.id, phiId]);
scopeIdentifiers.union([phi.place.identifier, phiId.identifier]);
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,18 +68,13 @@ export function eliminateRedundantPhi(
// Find any redundant phis
phis: for (const phi of block.phis) {
// Remap phis in case operands are from eliminated phis
phi.operands = new Map(
Array.from(phi.operands).map(([block, id]) => [
block,
rewrites.get(id) ?? id,
]),
);
phi.operands.forEach((place, _) => rewritePlace(place, rewrites));
// Find if the phi can be eliminated
let same: Identifier | null = null;
for (const [_, operand] of phi.operands) {
if (
(same !== null && operand.id === same.id) ||
operand.id === phi.id.id
(same !== null && operand.identifier.id === same.id) ||
operand.identifier.id === phi.place.identifier.id
) {
/*
* This operand is the same as the phi or is the same as the
Expand All @@ -94,7 +89,7 @@ export function eliminateRedundantPhi(
continue phis;
} else {
// First non-phi operand
same = operand;
same = operand.identifier;
}
}
CompilerError.invariant(same !== null, {
Expand All @@ -103,7 +98,7 @@ export function eliminateRedundantPhi(
loc: null,
suggestions: null,
});
rewrites.set(phi.id, same);
rewrites.set(phi.place.identifier, same);
block.phis.delete(phi);
}

Expand Down
Loading