Skip to content

Commit 7c4dc35

Browse files
committed
[compiler][rewrite] PropagateScopeDeps hir rewrite
\### Quick background: \#### Rvalues / temporaries: In the compiler, unnamed temporaries that represents the evaluation of an expression In the code snippet below, $1, $2, $3, and $4 are temporaries. ```js // input function Component({ bar} ) { const x = {a: foo(bar), b: {}}; } // gets lowered to [1] $2 = LoadGlobal(global) foo [2] $3 = LoadLocal bar$1 [3] $4 = Call $2(<unknown> $3) [4] $5 = Object { } [5] $6 = Object { a: $4, b: $5 } [6] $8 = StoreLocal Const x$7 = $6 ``` The compiler currently treats temporaries and named variables (e.g. `x`) differently in this pass. - named variables may be reassigned (in fact, since we're running after LeaveSSA, a single named identifier's IdentifierId may map to multiple `Identifier` instances -- each with its own scope and mutable range) - temporaries are replaced with their represented expressions during codegen. This is correct (mostly correct, see #29878) as we're careful to always lower the correct evaluation semantics. However, since we rewrite reactive scopes entirely (to if/else blocks), we need to track temporaries that a scope produces in `ReactiveScope.declarations` and later promote them to named variables. In the same example, $4, $5, and $6 need to be promoted: $2 ->`t0`, $5 ->`t1`, and $6 ->`t2`. ```js [1] $2 = LoadGlobal(global) foo [2] $3 = LoadLocal bar$1 scope 0: [3] $4 = Call $2(<unknown> $3) scope 1: [4] $5 = Object { } scope 2: [5] $6 = Object { a: $4, b: $5 } [6] $8 = StoreLocal Const x$7 = $6 ``` \#### Dependencies `ReactiveScope.dependencies` records the set of (read-only) values that a reactive scope is dependent on. This is currently limited to just variables (named variables from source and promoted temporaries) and property-loads. All dependencies we record need to be hoistable -- i.e. reordered to just before the ReactiveScope begins. Not all PropertyLoads are hoistable. In this example, we should not evaluate `obj.a.b` without before creating x and checking `objIsNull`. ```js // reduce-reactive-deps/no-uncond.js function useFoo({ obj, objIsNull }) { const x = []; if (isFalse(objIsNull)) { x.push(obj.a.b); } return x; } ``` While other memoization strategies with different constraints exist, the current compiler requires that `ReactiveScope.dependencies` be re-orderable to the beginning of the reactive scope. But.. `PropertyLoad`s from null values will throw `TypeError`. This means that evaluating hoisted dependencies should throw if and only if the source program throws. (It is also a bug if source throws and compiler output does not throw. See facebook/react-forget#2709) --- \### Rough high level overview 1. Pass 1 Walk over instructions to gather every temporary used outside of its defining scope (same as ReactiveFunction version). These determine the sidemaps we produce, as temporaries used outside of their declaring scopes get promoted to named variables later (and are not considered hoistable rvals). 2. Pass 2 (collectHoistablePropertyLoads) Walk over instructions to generate sidemaps: a. temporary identifier -> named variable and property path (e.g. `$3 -> {obj: props, path: ["a", "b"]}`) b. block -> accessed variables and properties (e.g. `bb0 -> [ {obj: props, path: ["a", "b"]} ]`) c. Walk over control flow graph to understand the set of object and property paths that can be read by each basic block. This analysis: - relies on post-dominator trees - traverses the CFG from entry (producing the set of variables/paths unconditionally evaluated *before* a block). - traverses the CFG from exit (producing the set of variables/paths unconditionally evaluated *after* a block). 4. Pass 3: (collectDependencies) Walks over instructions again to record dependencies and declarations, using the previously produced sidemaps Will add more fixture tests (although most cases should be covered in `reduce-reactive-deps`). Tested by syncing internally and checking compilation output differences ([internal link](https://fburl.com/wiki_markdown/nazsiszd)) --- \### Followups: 1. Rewrite function expression deps This change produces much more optimal output as the compiler now uses the function CFG to understand which variables / paths are assumed to be non-null. However, it may exacerbate [this function-expr hoisting bug](https://github.com/facebook/react/blob/main/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-invalid-hoisting-functionexpr.ts). A short term fix here is to simply call some form of `collectNonNullObjects` on every function expression to find hoistable variable / paths. In the longer term, we should refactor out `FunctionExpression.deps`. 2. Enable optional paths (a) don't count optional load temporaries as dependencies (e.g. `collectOptionalLoadRValues(...)`). (b) add optional paths back. This is a bit tricky as we'll want to implement some merging logic for `ConditionalAccess | OptionalChain | UnconditionalAccess`. In addition, our current optional chain lowering is slightly incorrect / imprecise ghstack-source-id: 0562a03 Pull Request resolved: #30079
1 parent d878489 commit 7c4dc35

50 files changed

Lines changed: 2331 additions & 250 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Pipeline.ts

Lines changed: 16 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,7 @@ import {
9696
validatePreservedManualMemoization,
9797
validateUseMemo,
9898
} from "../Validation";
99+
import { propagateScopeDependenciesHIR } from "../HIR/PropagateScopeDependenciesHIR";
99100

100101
export type CompilerPipelineValue =
101102
| { kind: "ast"; name: string; value: CodegenFunction }
@@ -306,6 +307,13 @@ function* runWithEnvironment(
306307
});
307308
assertTerminalSuccessorsExist(hir);
308309
assertTerminalPredsExist(hir);
310+
311+
propagateScopeDependenciesHIR(hir);
312+
yield log({
313+
kind: "hir",
314+
name: "PropagateScopeDependenciesHIR",
315+
value: hir,
316+
});
309317
}
310318

311319
const reactiveFunction = buildReactiveFunction(hir);
@@ -359,16 +367,15 @@ function* runWithEnvironment(
359367
name: "FlattenScopesWithHooks",
360368
value: reactiveFunction,
361369
});
362-
}
370+
assertScopeInstructionsWithinScopes(reactiveFunction);
363371

364-
assertScopeInstructionsWithinScopes(reactiveFunction);
365-
366-
propagateScopeDependencies(reactiveFunction);
367-
yield log({
368-
kind: "reactive",
369-
name: "PropagateScopeDependencies",
370-
value: reactiveFunction,
371-
});
372+
propagateScopeDependencies(reactiveFunction);
373+
yield log({
374+
kind: "reactive",
375+
name: "PropagateScopeDependencies",
376+
value: reactiveFunction,
377+
});
378+
}
372379

373380
pruneNonEscapingScopes(reactiveFunction);
374381
yield log({

0 commit comments

Comments
 (0)