Skip to content

Commit 62a6b3d

Browse files
josephsavonajosephsavona
committed
[compiler] Distinguish Alias/Assign effects
Alias was akward becuase it had a specific data flow / mutability relationship — direct mutations affect both sides - but also populated the output. We need the ability to express the aliasing dataflwo/mutability relationship w/o expressing actual assignment. An example of this is that for an unknown method call `const z = x.y()`, we want to express the Alias-style mutability relationship btw z and x (Muate(z) => Mutate(x)), but we want to assume that `z !== x`. This ends up being really clean, the places that should use Assign are very obvious (LoadLocal, StoreLocal) and most places keep using Alias. ghstack-source-id: ae2b35b Pull Request resolved: #33385
1 parent 109f884 commit 62a6b3d

File tree

7 files changed

+130
-43
lines changed

7 files changed

+130
-43
lines changed

‎compiler/packages/babel-plugin-react-compiler/src/HIR/Environment.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -246,7 +246,7 @@ export const EnvironmentConfigSchema = z.object({
246246
/**
247247
* Enable a new model for mutability and aliasing inference
248248
*/
249-
enableNewMutationAliasingModel: z.boolean().default(true),
249+
enableNewMutationAliasingModel: z.boolean().default(false),
250250

251251
/**
252252
* Enables inference of optional dependency chains. Without this flag

‎compiler/packages/babel-plugin-react-compiler/src/HIR/ObjectShape.ts‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -569,6 +569,32 @@ addObject(BUILTIN_SHAPES, BuiltInSetId, [
569569
calleeEffect: Effect.Store,
570570
// returnValueKind is technically dependent on the ValueKind of the set itself
571571
returnValueKind: ValueKind.Mutable,
572+
aliasing: {
573+
receiver: makeIdentifierId(0),
574+
params: [],
575+
rest: makeIdentifierId(1),
576+
returns: makeIdentifierId(2),
577+
temporaries: [],
578+
effects: [
579+
// Set.add returns the receiver Set
580+
{
581+
kind: 'Assign',
582+
from: signatureArgument(0),
583+
into: signatureArgument(2),
584+
},
585+
// Set.add mutates the set itself
586+
{
587+
kind: 'Mutate',
588+
value: signatureArgument(0),
589+
},
590+
// Captures the rest params into the set
591+
{
592+
kind: 'Capture',
593+
from: signatureArgument(1),
594+
into: signatureArgument(0),
595+
},
596+
],
597+
},
572598
}),
573599
],
574600
[

‎compiler/packages/babel-plugin-react-compiler/src/HIR/PrintHIR.ts‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -948,6 +948,9 @@ function getFunctionName(
948948

949949
export function printAliasingEffect(effect: AliasingEffect): string {
950950
switch (effect.kind) {
951+
case 'Assign': {
952+
return `Assign ${printPlaceForAliasEffect(effect.into)} = ${printPlaceForAliasEffect(effect.from)}`;
953+
}
951954
case 'Alias': {
952955
return `Alias ${printPlaceForAliasEffect(effect.into)} = ${printPlaceForAliasEffect(effect.from)}`;
953956
}
@@ -963,6 +966,9 @@ export function printAliasingEffect(effect: AliasingEffect): string {
963966
case 'CreateFrom': {
964967
return `Create ${printPlaceForAliasEffect(effect.into)} = kindOf(${printPlaceForAliasEffect(effect.from)})`;
965968
}
969+
case 'CreateFunction': {
970+
return `Function ${printPlaceForAliasEffect(effect.into)} = Function captures=[${effect.captures.map(printPlaceForAliasEffect).join(', ')}]`;
971+
}
966972
case 'Apply': {
967973
const receiverCallee =
968974
effect.receiver.identifier.id === effect.function.identifier.id
@@ -988,9 +994,6 @@ export function printAliasingEffect(effect: AliasingEffect): string {
988994
}
989995
return `Apply ${printPlaceForAliasEffect(effect.into)} = ${receiverCallee}(${args})${signature != '' ? '\n ' : ''}${signature}`;
990996
}
991-
case 'CreateFunction': {
992-
return `Function ${printPlaceForAliasEffect(effect.into)} = Function captures=[${effect.captures.map(printPlaceForAliasEffect).join(', ')}]`;
993-
}
994997
case 'Freeze': {
995998
return `Freeze ${printPlaceForAliasEffect(effect.value)} ${effect.reason}`;
996999
}

‎compiler/packages/babel-plugin-react-compiler/src/Inference/AnalyseFunctions.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,7 @@ function lowerWithMutationAliasing(fn: HIRFunction): void {
7777
const capturedOrMutated = new Set<IdentifierId>();
7878
for (const effect of effects ?? []) {
7979
switch (effect.kind) {
80+
case 'Assign':
8081
case 'Alias':
8182
case 'Capture':
8283
case 'CreateFrom': {

‎compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutationAliasingEffects.ts‎

Lines changed: 59 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -310,6 +310,9 @@ function applySignature(
310310
console.log(
311311
prettyFormat(state.debugAbstractValue(state.kind(instruction.lvalue))),
312312
);
313+
console.log(
314+
effects.map(effect => ` ${printAliasingEffect(effect)}`).join('\n'),
315+
);
313316
}
314317
if (
315318
!(state.isDefined(instruction.lvalue) && state.kind(instruction.lvalue))
@@ -403,8 +406,8 @@ function applyEffect(
403406
break;
404407
}
405408
default: {
406-
// TODO: should this be Alias??
407409
effects.push({
410+
// OK: recording information flow
408411
kind: 'Alias',
409412
from: effect.from,
410413
into: effect.into,
@@ -446,6 +449,7 @@ function applyEffect(
446449
}
447450
break;
448451
}
452+
case 'Alias':
449453
case 'Capture': {
450454
/*
451455
* Capture describes potential information flow: storing a pointer to one value
@@ -493,7 +497,7 @@ function applyEffect(
493497
}
494498
break;
495499
}
496-
case 'Alias': {
500+
case 'Assign': {
497501
/*
498502
* Alias represents potential pointer aliasing. If the type is a global,
499503
* a primitive (copy-on-write semantics) then we can prune the effect
@@ -549,12 +553,6 @@ function applyEffect(
549553
}
550554
case 'Apply': {
551555
const functionValues = state.values(effect.function);
552-
console.log('function is ' + printPlace(effect.function));
553-
console.log(
554-
functionValues.length +
555-
' ' +
556-
functionValues.map(v => v.kind).join(', '),
557-
);
558556
if (
559557
functionValues.length === 1 &&
560558
functionValues[0].kind === 'FunctionExpression'
@@ -567,15 +565,18 @@ function applyEffect(
567565
state.env,
568566
functionValues[0],
569567
);
570-
console.log(
571-
`constructed alias signature:\n${printAliasingSignature(signature)}`,
572-
);
568+
if (DEBUG) {
569+
console.log(
570+
`constructed alias signature:\n${printAliasingSignature(signature)}`,
571+
);
572+
}
573573
const signatureEffects = computeEffectsForSignature(
574574
state.env,
575575
signature,
576576
effect.into,
577577
effect.receiver,
578578
effect.args,
579+
functionValues[0].loweredFunc.func.context,
579580
);
580581
if (signatureEffects != null) {
581582
if (DEBUG) {
@@ -680,17 +681,9 @@ function applyEffect(
680681
effects,
681682
);
682683
}
683-
/*
684-
* TODO: this should be Alias, since the function could be identity.
685-
* Ie local mutation of the result could change the input.
686-
* But if we emit multiple Alias calls, currently the last one will win
687-
* when we update the inferencestate in applySignature. So we may need to group
688-
* them here, or coalesce them in applySignature
689-
*
690-
* maybe make `from: Place | Array<Place>`
691-
*/
692684
applyEffect(
693685
state,
686+
// OK: recording information flow
694687
{kind: 'Alias', from: operand, into: effect.into},
695688
instruction,
696689
effectInstructionValueCache,
@@ -713,6 +706,10 @@ function applyEffect(
713706
applyEffect(
714707
state,
715708
{
709+
/*
710+
* OK: a function might store one operand into another,
711+
* but it can't force one to alias another
712+
*/
716713
kind: 'Capture',
717714
from: operand,
718715
into: other,
@@ -1310,7 +1307,8 @@ function computeSignatureForInstruction(
13101307
from: value.value,
13111308
into: value.object,
13121309
});
1313-
effects.push({kind: 'Alias', from: value.value, into: lvalue});
1310+
// OK: lvalues are assigned to
1311+
effects.push({kind: 'Assign', from: value.value, into: lvalue});
13141312
break;
13151313
}
13161314
case 'PostfixUpdate':
@@ -1479,34 +1477,34 @@ function computeSignatureForInstruction(
14791477
into: patternLValue,
14801478
});
14811479
}
1482-
effects.push({kind: 'Alias', from: value.value, into: lvalue});
1480+
effects.push({kind: 'Assign', from: value.value, into: lvalue});
14831481
break;
14841482
}
14851483
case 'LoadContext': {
1486-
effects.push({kind: 'Alias', from: value.place, into: lvalue});
1484+
effects.push({kind: 'Assign', from: value.place, into: lvalue});
14871485
break;
14881486
}
14891487
case 'StoreContext': {
14901488
effects.push({kind: 'Mutate', value: value.lvalue.place});
14911489
effects.push({
1492-
kind: 'Alias',
1490+
kind: 'Assign',
14931491
from: value.value,
14941492
into: value.lvalue.place,
14951493
});
1496-
effects.push({kind: 'Alias', from: value.value, into: lvalue});
1494+
effects.push({kind: 'Assign', from: value.value, into: lvalue});
14971495
break;
14981496
}
14991497
case 'LoadLocal': {
1500-
effects.push({kind: 'Alias', from: value.place, into: lvalue});
1498+
effects.push({kind: 'Assign', from: value.place, into: lvalue});
15011499
break;
15021500
}
15031501
case 'StoreLocal': {
15041502
effects.push({
1505-
kind: 'Alias',
1503+
kind: 'Assign',
15061504
from: value.value,
15071505
into: value.lvalue.place,
15081506
});
1509-
effects.push({kind: 'Alias', from: value.value, into: lvalue});
1507+
effects.push({kind: 'Assign', from: value.value, into: lvalue});
15101508
break;
15111509
}
15121510
case 'StoreGlobal': {
@@ -1516,7 +1514,7 @@ function computeSignatureForInstruction(
15161514
});
15171515
}
15181516
case 'TypeCastExpression': {
1519-
effects.push({kind: 'Alias', from: value.value, into: lvalue});
1517+
effects.push({kind: 'Assign', from: value.value, into: lvalue});
15201518
break;
15211519
}
15221520
case 'LoadGlobal': {
@@ -1772,6 +1770,8 @@ function computeEffectsForSignature(
17721770
lvalue: Place,
17731771
receiver: Place,
17741772
args: Array<Place | SpreadPattern | Hole>,
1773+
// Used for signatures constructed dynamically which reference context variables
1774+
context: Array<Place> = [],
17751775
): Array<AliasingEffect> | null {
17761776
if (
17771777
// Not enough args
@@ -1816,6 +1816,15 @@ function computeEffectsForSignature(
18161816
}
18171817
}
18181818

1819+
/*
1820+
* Signatures constructed dynamically from function expressions will reference values
1821+
* other than their receiver/args/etc. We populate the substitution table with these
1822+
* values so that we can still exit for unpopulated substitutions
1823+
*/
1824+
for (const operand of context) {
1825+
substitutions.set(operand.identifier.id, [operand]);
1826+
}
1827+
18191828
const effects: Array<AliasingEffect> = [];
18201829
for (const signatureTemporary of signature.temporaries) {
18211830
const temp = createTemporaryPlace(env, receiver.loc);
@@ -1825,6 +1834,7 @@ function computeEffectsForSignature(
18251834
// Apply substitutions
18261835
for (const effect of signature.effects) {
18271836
switch (effect.kind) {
1837+
case 'Assign':
18281838
case 'ImmutableCapture':
18291839
case 'Alias':
18301840
case 'CreateFrom':
@@ -2068,18 +2078,32 @@ export type AliasingEffect =
20682078
*/
20692079
| {kind: 'MutateTransitiveConditionally'; value: Place}
20702080
/**
2071-
* Records indirect aliasing from flow from `from` to `into`. Local mutation (Mutate vs MutateTransitive)
2072-
* of `into` will *not* affect `from`.
2081+
* Records information flow from `from` to `into` in cases where local mutation of the destination
2082+
* will *not* mutate the source:
2083+
*
2084+
* - Capture a -> b and Mutate(b) X=> (does not imply) Mutate(a)
2085+
* - Capture a -> b and MutateTransitive(b) => (does imply) Mutate(a)
20732086
*
2074-
* Example: `x[0] = y[1]`. Information from y (from) is aliased into x (into), but there is not a
2075-
* direct aliasing of y as x.
2087+
* Example: `array.push(item)`. Information from item is captured into array, but there is not a
2088+
* direct aliasing, and local mutations of array will not modify item.
20762089
*/
20772090
| {kind: 'Capture'; from: Place; into: Place}
20782091
/**
2079-
* Records direct aliasing of `from` as `into`. Local mutation (Mutate vs MutateTransitive)
2080-
* of `into` *will* affect `from`.
2092+
* Records information flow from `from` to `into` in cases where local mutation of the destination
2093+
* *will* mutate the source:
2094+
*
2095+
* - Alias a -> b and Mutate(b) => (does imply) Mutate(a)
2096+
* - Alias a -> b and MutateTransitive(b) => (does imply) Mutate(a)
2097+
*
2098+
* Example: `c = identity(a)`. We don't know what `identity()` returns so we can't use Assign.
2099+
* But we have to assume that it _could_ be returning its input, such that a local mutation of
2100+
* c could be mutating a.
20812101
*/
20822102
| {kind: 'Alias'; from: Place; into: Place}
2103+
/**
2104+
* Records direct assignment: `into = from`.
2105+
*/
2106+
| {kind: 'Assign'; from: Place; into: Place}
20832107
/**
20842108
* Creates a value of the given type at the given place
20852109
*/

‎compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutationAliasingFunctionEffects.ts‎

Lines changed: 30 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,14 @@
55
* LICENSE file in the root directory of this source tree.
66
*/
77

8-
import {HIRFunction, IdentifierId, Place, ScopeId} from '../HIR';
8+
import {
9+
HIRFunction,
10+
IdentifierId,
11+
isPrimitiveType,
12+
Place,
13+
ScopeId,
14+
ValueKind,
15+
} from '../HIR';
916
import {getOrInsertDefault} from '../Utils/utils';
1017
import {AliasingEffect} from './InferMutationAliasingEffects';
1118

@@ -59,7 +66,7 @@ export function inferMutationAliasingFunctionEffects(
5966
* the value is used. Eg capturing an alias => capture. See joinEffects() helper.
6067
*/
6168
type AliasedIdentifier = {
62-
kind: 'Capture' | 'Alias' | 'CreateFrom';
69+
kind: AliasingKind;
6370
place: Place;
6471
};
6572
const dataFlow = new Map<IdentifierId, Array<AliasedIdentifier>>();
@@ -101,6 +108,7 @@ export function inferMutationAliasingFunctionEffects(
101108
if (instr.effects == null) continue;
102109
for (const effect of instr.effects) {
103110
if (
111+
effect.kind === 'Assign' ||
104112
effect.kind === 'Capture' ||
105113
effect.kind === 'Alias' ||
106114
effect.kind === 'CreateFrom'
@@ -188,6 +196,7 @@ export function inferMutationAliasingFunctionEffects(
188196
}
189197
}
190198
// Create aliasing effects based on observed data flow
199+
let hasReturn = false;
191200
for (const [into, from] of dataFlow) {
192201
const input = tracked.get(into);
193202
if (input == null) {
@@ -196,8 +205,24 @@ export function inferMutationAliasingFunctionEffects(
196205
for (const aliased of from) {
197206
const effect = {kind: aliased.kind, from: aliased.place, into: input};
198207
effects.push(effect);
208+
if (
209+
into === fn.returns.identifier.id &&
210+
(aliased.kind === 'Assign' || aliased.kind === 'CreateFrom')
211+
) {
212+
hasReturn = true;
213+
}
199214
}
200215
}
216+
// TODO: more precise return effect inference
217+
if (!hasReturn) {
218+
effects.push({
219+
kind: 'Create',
220+
into: fn.returns,
221+
value: isPrimitiveType(fn.returns.identifier)
222+
? ValueKind.Primitive
223+
: ValueKind.Mutable,
224+
});
225+
}
201226

202227
return effects;
203228
}
@@ -208,13 +233,15 @@ enum MutationKind {
208233
Definite = 2,
209234
}
210235

211-
type AliasingKind = 'Alias' | 'Capture' | 'CreateFrom';
236+
type AliasingKind = 'Alias' | 'Capture' | 'CreateFrom' | 'Assign';
212237
function joinEffects(
213238
effect1: AliasingKind,
214239
effect2: AliasingKind,
215240
): AliasingKind {
216241
if (effect1 === 'Capture' || effect2 === 'Capture') {
217242
return 'Capture';
243+
} else if (effect1 === 'Assign' || effect2 === 'Assign') {
244+
return 'Assign';
218245
} else {
219246
return 'Alias';
220247
}

0 commit comments

Comments
 (0)