Skip to content

Commit 6c69f01

Browse files
josephsavonajosephsavona
committed
[compiler] InferMutationAliasingRanges precisely models which values mutate when
It turns out that InferMutationAliasingRanges does need a fixpoint loop, but the approach is arguably simpler overall and more precise than the previous implementation. Like InferMutationAliasingEffects (which is the new InferReferenceEffects), we build an abstract model of the heap. But here we know what the effects are, so we can do abstract interpretation of the effects. Each abstract value stores a set of values that it has captured (for transitive mutation), while each variable keeps a set of values it may directly mutate (for assign/alias/capturefrom). This means that at each mutation, we can mark _exactly_ the set of variables/values that are affected by that specific instruction. This means we can correctly infer that `mutate(b)` can't impact `a` here: ``` a = make(); b = make(); mutate(b); // when we interpret the mutation here, a isn't captured yet b.a = a; ``` We will need to make this a fixpoint, but only if there are backedges in the CFG. ghstack-source-id: 6fd1662 Pull Request resolved: #33401
1 parent b9917a9 commit 6c69f01

File tree

3 files changed

+281
-79
lines changed

3 files changed

+281
-79
lines changed

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

Lines changed: 190 additions & 79 deletions
Original file line numberDiff line numberDiff line change
@@ -5,46 +5,61 @@
55
* LICENSE file in the root directory of this source tree.
66
*/
77

8-
import {CompilerError} from '..';
8+
import prettyFormat from 'pretty-format';
9+
import {CompilerError, SourceLocation} from '..';
910
import {
1011
Effect,
1112
HIRFunction,
1213
Identifier,
1314
IdentifierId,
15+
InstructionId,
1416
makeInstructionId,
17+
Place,
1518
} from '../HIR/HIR';
1619
import {
1720
eachInstructionLValue,
1821
eachInstructionValueOperand,
1922
eachTerminalOperand,
2023
} from '../HIR/visitors';
21-
import DisjointSet from '../Utils/DisjointSet';
22-
import {assertExhaustive} from '../Utils/utils';
23-
import {debugAliases} from './InferMutableRanges';
24-
import {inferMutableRangesForAlias} from './InferMutableRangesForAlias';
24+
import {assertExhaustive, getOrInsertWith} from '../Utils/utils';
25+
import {printFunction} from '../HIR';
26+
import {printInstruction} from '../HIR/PrintHIR';
27+
28+
const DEBUG = false;
2529

2630
/**
2731
* Infers mutable ranges for all values.
2832
*/
2933
export function inferMutationAliasingRanges(fn: HIRFunction): void {
34+
if (DEBUG) {
35+
console.log();
36+
}
3037
/**
31-
* Part 1
32-
* Infer ranges for transitive mutations, which includes mutations that affect
33-
* captured references and not just direct aliases. We build a distjoing set
34-
* that tracks capturing and direct aliasing, and look at transitive mutations
35-
* only.
38+
* Part 1: Infer mutable ranges for values. We build an abstract model
39+
* of the effect types, distinguishing values which can capture references
40+
* to other values and variables, which can point to one or more values.
41+
*
42+
* Transitive mutation marks value identifiers as mutated and also walks
43+
* into the identifiers captured by that abstract value: local mutation
44+
* only marks the top-level identifiers as mutated.
45+
*
46+
* TODO: add a fixpoint.
3647
*/
37-
const captures = new DisjointSet<Identifier>();
48+
const state = new AliasingState();
49+
for (const param of fn.context) {
50+
state.create(param);
51+
}
3852
for (const block of fn.body.blocks.values()) {
3953
for (const phi of block.phis) {
40-
captures.union([
41-
phi.place.identifier,
42-
...[...phi.operands.values()].map(place => place.identifier),
43-
]);
54+
state.create(phi.place);
55+
for (const operand of phi.operands.values()) {
56+
state.alias(operand, phi.place);
57+
}
4458
}
4559

4660
for (const instr of block.instructions) {
4761
for (const lvalue of eachInstructionLValue(instr)) {
62+
state.create(lvalue);
4863
if (lvalue.identifier.mutableRange.start === 0) {
4964
lvalue.identifier.mutableRange.start = instr.id;
5065
}
@@ -55,84 +70,46 @@ export function inferMutationAliasingRanges(fn: HIRFunction): void {
5570

5671
if (instr.effects == null) continue;
5772
for (const effect of instr.effects) {
58-
if (
59-
effect.kind === 'Assign' ||
60-
effect.kind === 'Alias' ||
61-
effect.kind === 'CreateFrom' ||
62-
effect.kind === 'Capture'
63-
) {
64-
captures.union([effect.from.identifier, effect.into.identifier]);
73+
if (effect.kind === 'Create' || effect.kind === 'CreateFunction') {
74+
state.create(effect.into);
75+
} else if (effect.kind === 'Assign') {
76+
state.assign(effect.from, effect.into);
77+
} else if (effect.kind === 'Alias' || effect.kind === 'CreateFrom') {
78+
state.alias(effect.from, effect.into);
79+
} else if (effect.kind === 'Capture') {
80+
state.capture(effect.from, effect.into);
6581
} else if (
6682
effect.kind === 'MutateTransitive' ||
6783
effect.kind === 'MutateTransitiveConditionally'
6884
) {
69-
const value = effect.value;
70-
value.identifier.mutableRange.end = makeInstructionId(instr.id + 1);
71-
}
72-
}
73-
}
74-
}
75-
/**
76-
* TODO: this will incorrectly mark values as mutated in the following case:
77-
* 1. Create value x
78-
* 2. Create value y
79-
* 3. Transitively mutate y
80-
* 4. Capture x -> y
81-
*
82-
* We will put these all into one alias set in captures, and then InferMutableRangesForAlias
83-
* will look at all identifiers in the set that start before the mutation. Thus it will see
84-
* that x is created before the mutation, and consider it mutated. But the capture doesn't
85-
* occur until after the mutation!
86-
*
87-
* The fix is to use a graph to precisely describe what is captured where at each instruction,
88-
* so that on a transitive mutation we can iterate all the captured values and mark them.
89-
*
90-
* This then needs a fixpoint: although we would track captures for phi operands, the operands'
91-
* own capture values won't be populated until we do a fixpoint.
92-
*/
93-
inferMutableRangesForAlias(fn, captures);
94-
95-
/**
96-
* Part 2
97-
* Infer ranges for local (non-transitive) mutations. We build a disjoint set
98-
* that only tracks direct value aliasing, and look only at local mutations
99-
* to extend ranges
100-
*
101-
* TODO: similar design to the above todo for captures, use a directed graph instead of disjoint union,
102-
* with fixpoint.
103-
*/
104-
const aliases = new DisjointSet<Identifier>();
105-
for (const block of fn.body.blocks.values()) {
106-
for (const phi of block.phis) {
107-
aliases.union([
108-
phi.place.identifier,
109-
...[...phi.operands.values()].map(place => place.identifier),
110-
]);
111-
}
112-
113-
for (const instr of block.instructions) {
114-
if (instr.effects == null) continue;
115-
for (const effect of instr.effects) {
116-
if (
117-
effect.kind === 'Assign' ||
118-
effect.kind === 'Alias' ||
119-
effect.kind === 'CreateFrom'
120-
) {
121-
aliases.union([effect.from.identifier, effect.into.identifier]);
85+
state.mutateTransitive(
86+
effect.value.identifier,
87+
makeInstructionId(instr.id + 1),
88+
effect.value.loc,
89+
);
12290
} else if (
12391
effect.kind === 'Mutate' ||
12492
effect.kind === 'MutateConditionally'
12593
) {
126-
const value = effect.value;
127-
value.identifier.mutableRange.end = makeInstructionId(instr.id + 1);
94+
state.mutate(
95+
effect.value.identifier,
96+
makeInstructionId(instr.id + 1),
97+
effect.value.loc,
98+
);
12899
}
129100
}
101+
if (DEBUG) {
102+
console.log(printInstruction(instr));
103+
console.log(state.debug());
104+
}
130105
}
131106
}
132-
inferMutableRangesForAlias(fn, aliases);
107+
if (DEBUG) {
108+
console.log(state.debug());
109+
}
133110

134111
/**
135-
* Part 3
112+
* Part 2
136113
* Add legacy operand-specific effects based on instruction effects and mutable ranges.
137114
* Also fixes up operand mutable ranges, making sure that start is non-zero if the value
138115
* is mutated (depended on by later passes like InferReactiveScopeVariables which uses this
@@ -239,4 +216,138 @@ export function inferMutationAliasingRanges(fn: HIRFunction): void {
239216
operand.effect = Effect.Read;
240217
}
241218
}
219+
220+
if (DEBUG) {
221+
console.log(printFunction(fn));
222+
}
223+
}
224+
225+
/**
226+
* TODO: similar to alias effect inference state:
227+
* - values represent the conceptual values (context vars, things that got Create*-ed)
228+
* into which other values are captured
229+
* - variables deals with aliases (Assign, Alias, CreateFrom)
230+
*
231+
* Ex:
232+
* `Capture a -> b`:
233+
* - find the values represented by `a` (aValues) by looking up a in .variables, then mapping to .values
234+
* - find the values represented by `b` (bValues) by looking up b in .variables, then mapping to .values
235+
* - add aValues into bValues
236+
*/
237+
class AliasingState {
238+
values: Map<Identifier, Set<Identifier>> = new Map();
239+
variables: Map<IdentifierId, Set<Identifier>> = new Map();
240+
241+
create(place: Place): void {
242+
this.values.set(place.identifier, new Set());
243+
this.variables.set(place.identifier.id, new Set([place.identifier]));
244+
}
245+
246+
clear(lvalue: Place): void {
247+
this.variables.set(lvalue.identifier.id, new Set());
248+
}
249+
250+
assign(from: Place, into: Place): void {
251+
const fromVariables = this.variables.get(from.identifier.id);
252+
if (fromVariables == null) {
253+
return;
254+
}
255+
this.variables.set(
256+
into.identifier.id,
257+
new Set([...fromVariables, from.identifier, into.identifier]),
258+
// fromVariables,
259+
);
260+
}
261+
262+
alias(from: Place, into: Place): void {
263+
const intoVariables = getOrInsertWith(
264+
this.variables,
265+
into.identifier.id,
266+
() => new Set(),
267+
);
268+
intoVariables.add(from.identifier);
269+
}
270+
271+
capture(from: Place, into: Place): void {
272+
const intoVariables = this.variables.get(into.identifier.id)!;
273+
for (const v of intoVariables) {
274+
const values = this.values.get(v)!;
275+
values.add(from.identifier);
276+
}
277+
}
278+
279+
mutateTransitive(
280+
place: Identifier,
281+
end: InstructionId,
282+
loc: SourceLocation,
283+
): void {
284+
const variables = this.variables.get(place.id);
285+
if (variables == null) {
286+
return;
287+
}
288+
for (const value of variables) {
289+
value.mutableRange.end = makeInstructionId(
290+
Math.max(value.mutableRange.end, end),
291+
);
292+
const captured = this.values.get(value)!;
293+
for (const capture of captured) {
294+
this.mutateTransitive(capture, end, loc);
295+
}
296+
}
297+
}
298+
299+
mutate(place: Identifier, end: InstructionId, _loc: SourceLocation): void {
300+
const variables = this.variables.get(place.id);
301+
if (variables == null) {
302+
return;
303+
}
304+
place.mutableRange.end = makeInstructionId(
305+
Math.max(place.mutableRange.end, end),
306+
);
307+
for (const value of variables) {
308+
// Unlike mutateTransitive, we don't traverse into captured values
309+
value.mutableRange.end = makeInstructionId(
310+
Math.max(value.mutableRange.end, end),
311+
);
312+
}
313+
}
314+
315+
canonicalize(): Map<IdentifierId, string> {
316+
const map = new Map<IdentifierId, string>();
317+
for (const value of this.values.keys()) {
318+
map.set(
319+
value.id,
320+
`${value.mutableRange.start}:${value.mutableRange.end}`,
321+
);
322+
}
323+
return map;
324+
}
325+
326+
debug(): string {
327+
const values = new Map<string, string>();
328+
for (const [k, v] of this.values) {
329+
values.set(
330+
`${k.name?.value ?? ''}$${k.id}:${k.mutableRange.start}:${k.mutableRange.end}`,
331+
[...v]
332+
.map(
333+
v =>
334+
`${v.name?.value ?? ''}$${v.id}:${v.mutableRange.start}:${v.mutableRange.end}`,
335+
)
336+
.join(', '),
337+
);
338+
}
339+
const variables = new Map<string, string>();
340+
for (const [k, v] of this.variables) {
341+
variables.set(
342+
`$${k}`,
343+
[...v]
344+
.map(
345+
v =>
346+
`${v.name?.value ?? ''}$${v.id}:${v.mutableRange.start}:${v.mutableRange.end}`,
347+
)
348+
.join(', '),
349+
);
350+
}
351+
return prettyFormat({values, variables});
352+
}
242353
}

‎compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/new-mutability/ssa-renaming-ternary-destruction.expect.md‎

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,70 @@
1+
2+
## Input
3+
4+
```javascript
5+
// @enablePropagateDepsInHIR @enableNewMutationAliasingModel
6+
function useFoo(props) {
7+
let x = [];
8+
x.push(props.bar);
9+
// todo: the below should memoize separately from the above
10+
// my guess is that the phi causes the different `x` identifiers
11+
// to get added to an alias group. this is where we need to track
12+
// the actual state of the alias groups at the time of the mutation
13+
props.cond ? (({x} = {x: {}}), ([x] = [[]]), x.push(props.foo)) : null;
14+
return x;
15+
}
16+
17+
export const FIXTURE_ENTRYPOINT = {
18+
fn: useFoo,
19+
params: [{cond: false, foo: 2, bar: 55}],
20+
sequentialRenders: [
21+
{cond: false, foo: 2, bar: 55},
22+
{cond: false, foo: 3, bar: 55},
23+
{cond: true, foo: 3, bar: 55},
24+
],
25+
};
26+
27+
```
28+
29+
## Code
30+
31+
```javascript
32+
import { c as _c } from "react/compiler-runtime"; // @enablePropagateDepsInHIR @enableNewMutationAliasingModel
33+
function useFoo(props) {
34+
const $ = _c(5);
35+
let x;
36+
if ($[0] !== props.bar) {
37+
x = [];
38+
x.push(props.bar);
39+
$[0] = props.bar;
40+
$[1] = x;
41+
} else {
42+
x = $[1];
43+
}
44+
if ($[2] !== props.cond || $[3] !== props.foo) {
45+
props.cond ? (([x] = [[]]), x.push(props.foo)) : null;
46+
$[2] = props.cond;
47+
$[3] = props.foo;
48+
$[4] = x;
49+
} else {
50+
x = $[4];
51+
}
52+
return x;
53+
}
54+
55+
export const FIXTURE_ENTRYPOINT = {
56+
fn: useFoo,
57+
params: [{ cond: false, foo: 2, bar: 55 }],
58+
sequentialRenders: [
59+
{ cond: false, foo: 2, bar: 55 },
60+
{ cond: false, foo: 3, bar: 55 },
61+
{ cond: true, foo: 3, bar: 55 },
62+
],
63+
};
64+
65+
```
66+
67+
### Eval output
68+
(kind: ok) [55]
69+
[55]
70+
[3]

0 commit comments

Comments
 (0)