Skip to content

Commit

Permalink
Infer CreateThis return type in FunctionAnalysis
Browse files Browse the repository at this point in the history
Summary:
If we know that `CreateThis` is being used on a JS function, then we
know it will return an object. This condition will soon be complicated
when considering classes, but that is still unlanded.

Reviewed By: avp

Differential Revision: D66544706

fbshipit-source-id: 81d7988ff08914631309728d738baa716a8f46a8
  • Loading branch information
fbmal7 authored and facebook-github-bot committed Dec 18, 2024
1 parent bb1c7be commit 5ff83f1
Show file tree
Hide file tree
Showing 11 changed files with 53 additions and 44 deletions.
18 changes: 18 additions & 0 deletions lib/Optimizer/Scalar/FunctionAnalysis.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,11 @@ void analyzeCreateCallable(BaseCreateCallableInst *create) {
Module *M = F->getParent();
IRBuilder builder(M);

// The only kind of function which does not expect a made `this` parameter in
// a construct call is a derived class constructor.
bool funcExpectsThisInConstruct =
F->getDefinitionKind() != Function::DefinitionKind::ES6DerivedConstructor;

/// Define an element in the worklist below.
struct UserInfo {
/// An instruction that is known to have either the value of the closure, or
Expand Down Expand Up @@ -170,6 +175,8 @@ void analyzeCreateCallable(BaseCreateCallableInst *create) {
// to avoid going back and forth between the corresponding loads.
llvh::SmallPtrSet<Instruction *, 2> visited{};

IRBuilder::InstructionDestroyer destroyer{};

worklist.push_back({create, true, create->getScope()});
while (!worklist.empty()) {
// Instruction whose only possible closure value is the one being analyzed.
Expand Down Expand Up @@ -202,6 +209,17 @@ void analyzeCreateCallable(BaseCreateCallableInst *create) {
assert(
CTI->getClosure() == closureInst &&
"Closure must be closure argument to CreateThisInst");
if (isAlwaysClosure) {
if (funcExpectsThisInConstruct) {
CTI->setType(Type::createObject());
} else {
// If a function does not expect any `this`, then we can just remove
// the `CreateThis` altogether since it was just going to return
// undefined.
CTI->replaceAllUsesWith(builder.getLiteralUndefined());
destroyer.add(CTI);
}
}
// CreateThis leaks the closure because the created object can still
// access the function via its parent's `.constructor` prototype.
F->getAttributesRef(M)._allCallsitesKnownInStrictMode = false;
Expand Down
7 changes: 3 additions & 4 deletions test/Optimizer/cache-new-object-analysis.js
Original file line number Diff line number Diff line change
Expand Up @@ -36,10 +36,9 @@ return new simple(1, 2);
// CHECK-NEXT:%BB0:
// CHECK-NEXT: %0 = GetParentScopeInst (:environment) %VS0: any, %parentScope: environment
// CHECK-NEXT: %1 = CreateFunctionInst (:object) %0: environment, %simple(): functionCode
// CHECK-NEXT: %2 = CreateThisInst (:undefined|object) %1: object, empty: any
// CHECK-NEXT: %3 = CallInst (:undefined) %1: object, %simple(): functionCode, true: boolean, empty: any, undefined: undefined, %2: undefined|object, 1: number, 2: number
// CHECK-NEXT: %4 = GetConstructedObjectInst (:object) %2: undefined|object, undefined: undefined
// CHECK-NEXT: ReturnInst %4: object
// CHECK-NEXT: %2 = CreateThisInst (:object) %1: object, empty: any
// CHECK-NEXT: %3 = CallInst (:undefined) %1: object, %simple(): functionCode, true: boolean, empty: any, undefined: undefined, %2: object, 1: number, 2: number
// CHECK-NEXT: ReturnInst %2: object
// CHECK-NEXT:function_end

// CHECK:function simple(x: any, y: any): undefined
Expand Down
7 changes: 3 additions & 4 deletions test/Optimizer/callee.js
Original file line number Diff line number Diff line change
Expand Up @@ -80,10 +80,9 @@ function load_store_multiple_test() {
// CHECK-NEXT:%BB0:
// CHECK-NEXT: %0 = GetParentScopeInst (:environment) %VS0: any, %parentScope: environment
// CHECK-NEXT: %1 = CreateFunctionInst (:object) %0: environment, %"foo 1#"(): functionCode
// CHECK-NEXT: %2 = CreateThisInst (:undefined|object) %1: object, empty: any
// CHECK-NEXT: %3 = CallInst (:any) %1: object, %"foo 1#"(): functionCode, true: boolean, empty: any, undefined: undefined, %2: undefined|object, 12: number
// CHECK-NEXT: %4 = GetConstructedObjectInst (:object) %2: undefined|object, 12: number
// CHECK-NEXT: ReturnInst %4: object
// CHECK-NEXT: %2 = CreateThisInst (:object) %1: object, empty: any
// CHECK-NEXT: %3 = CallInst (:any) %1: object, %"foo 1#"(): functionCode, true: boolean, empty: any, undefined: undefined, %2: object, 12: number
// CHECK-NEXT: ReturnInst %2: object
// CHECK-NEXT:function_end

// CHECK:scope %VS1 [k: object]
Expand Down
14 changes: 6 additions & 8 deletions test/Optimizer/constructor_callee.js
Original file line number Diff line number Diff line change
Expand Up @@ -50,10 +50,9 @@ function ctor_load_store_test() {
// CHECK-NEXT:%BB0:
// CHECK-NEXT: %0 = GetParentScopeInst (:environment) %VS0: any, %parentScope: environment
// CHECK-NEXT: %1 = CreateFunctionInst (:object) %0: environment, %use_this(): functionCode
// CHECK-NEXT: %2 = CreateThisInst (:undefined|object) %1: object, empty: any
// CHECK-NEXT: %3 = CallInst (:any) %1: object, %use_this(): functionCode, true: boolean, empty: any, undefined: undefined, %2: undefined|object, 12: number
// CHECK-NEXT: %4 = GetConstructedObjectInst (:object) %2: undefined|object, %2: undefined|object
// CHECK-NEXT: ReturnInst %4: object
// CHECK-NEXT: %2 = CreateThisInst (:object) %1: object, empty: any
// CHECK-NEXT: %3 = CallInst (:any) %1: object, %use_this(): functionCode, true: boolean, empty: any, undefined: undefined, %2: object, 12: number
// CHECK-NEXT: ReturnInst %2: object
// CHECK-NEXT:function_end

// CHECK:scope %VS1 [use_this: object]
Expand Down Expand Up @@ -89,8 +88,7 @@ function ctor_load_store_test() {
// CHECK-NEXT:%BB0:
// CHECK-NEXT: %0 = GetParentScopeInst (:environment) %VS1: any, %parentScope: environment
// CHECK-NEXT: %1 = LoadFrameInst (:object) %0: environment, [%VS1.use_this]: object
// CHECK-NEXT: %2 = CreateThisInst (:undefined|object) %1: object, empty: any
// CHECK-NEXT: %3 = CallInst (:undefined) %1: object, %"use_this 1#"(): functionCode, true: boolean, empty: any, undefined: undefined, %2: undefined|object, 12: number
// CHECK-NEXT: %4 = GetConstructedObjectInst (:object) %2: undefined|object, undefined: undefined
// CHECK-NEXT: ReturnInst %4: object
// CHECK-NEXT: %2 = CreateThisInst (:object) %1: object, empty: any
// CHECK-NEXT: %3 = CallInst (:undefined) %1: object, %"use_this 1#"(): functionCode, true: boolean, empty: any, undefined: undefined, %2: object, 12: number
// CHECK-NEXT: ReturnInst %2: object
// CHECK-NEXT:function_end
4 changes: 2 additions & 2 deletions test/Optimizer/constructor_callee_regexp.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,8 @@ function ctor_this_test() {
// CHECK-NEXT:%BB0:
// CHECK-NEXT: %0 = GetParentScopeInst (:environment) %VS0: any, %parentScope: environment
// CHECK-NEXT: %1 = CreateFunctionInst (:object) %0: environment, %use_this(): functionCode
// CHECK-NEXT: %2 = CreateThisInst (:undefined|object) %1: object, empty: any
// CHECK-NEXT: %3 = CallInst (:object) %1: object, %use_this(): functionCode, true: boolean, empty: any, undefined: undefined, %2: undefined|object, 12: number
// CHECK-NEXT: %2 = CreateThisInst (:object) %1: object, empty: any
// CHECK-NEXT: %3 = CallInst (:object) %1: object, %use_this(): functionCode, true: boolean, empty: any, undefined: undefined, %2: object, 12: number
// CHECK-NEXT: ReturnInst %3: object
// CHECK-NEXT:function_end

Expand Down
7 changes: 3 additions & 4 deletions test/Optimizer/function-analysis-construction-store.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,10 +33,9 @@ function main() {
// CHECK-NEXT:%BB0:
// CHECK-NEXT: %0 = GetParentScopeInst (:environment) %VS0: any, %parentScope: environment
// CHECK-NEXT: %1 = CreateFunctionInst (:object) %0: environment, %f(): functionCode
// CHECK-NEXT: %2 = CreateThisInst (:undefined|object) %1: object, empty: any
// CHECK-NEXT: %3 = CallInst (:undefined) %1: object, %f(): functionCode, true: boolean, empty: any, undefined: undefined, %2: undefined|object
// CHECK-NEXT: %4 = GetConstructedObjectInst (:object) %2: undefined|object, undefined: undefined
// CHECK-NEXT: ReturnInst %4: object
// CHECK-NEXT: %2 = CreateThisInst (:object) %1: object, empty: any
// CHECK-NEXT: %3 = CallInst (:undefined) %1: object, %f(): functionCode, true: boolean, empty: any, undefined: undefined, %2: object
// CHECK-NEXT: ReturnInst %2: object
// CHECK-NEXT:function_end

// CHECK:function f(): undefined
Expand Down
7 changes: 3 additions & 4 deletions test/Optimizer/function-analysis-construction.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,10 +32,9 @@ function main() {
// CHECK-NEXT:%BB0:
// CHECK-NEXT: %0 = GetParentScopeInst (:environment) %VS0: any, %parentScope: environment
// CHECK-NEXT: %1 = CreateFunctionInst (:object) %0: environment, %f(): functionCode
// CHECK-NEXT: %2 = CreateThisInst (:undefined|object) %1: object, empty: any
// CHECK-NEXT: %3 = CallInst (:undefined) %1: object, %f(): functionCode, true: boolean, empty: any, undefined: undefined, %2: undefined|object
// CHECK-NEXT: %4 = GetConstructedObjectInst (:object) %2: undefined|object, undefined: undefined
// CHECK-NEXT: ReturnInst %4: object
// CHECK-NEXT: %2 = CreateThisInst (:object) %1: object, empty: any
// CHECK-NEXT: %3 = CallInst (:undefined) %1: object, %f(): functionCode, true: boolean, empty: any, undefined: undefined, %2: object
// CHECK-NEXT: ReturnInst %2: object
// CHECK-NEXT:function_end

// CHECK:function f(): undefined
Expand Down
7 changes: 3 additions & 4 deletions test/Optimizer/function-analysis-new-target.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,10 +35,9 @@ function main() {
// CHECK-NEXT:%BB0:
// CHECK-NEXT: %0 = GetParentScopeInst (:environment) %VS0: any, %parentScope: environment
// CHECK-NEXT: %1 = CreateFunctionInst (:object) %0: environment, %f(): functionCode
// CHECK-NEXT: %2 = CreateThisInst (:undefined|object) %1: object, empty: any
// CHECK-NEXT: %3 = CallInst (:undefined) %1: object, %f(): functionCode, true: boolean, empty: any, %1: object, %2: undefined|object
// CHECK-NEXT: %4 = GetConstructedObjectInst (:object) %2: undefined|object, undefined: undefined
// CHECK-NEXT: ReturnInst %4: object
// CHECK-NEXT: %2 = CreateThisInst (:object) %1: object, empty: any
// CHECK-NEXT: %3 = CallInst (:undefined) %1: object, %f(): functionCode, true: boolean, empty: any, %1: object, %2: object
// CHECK-NEXT: ReturnInst %2: object
// CHECK-NEXT:function_end

// CHECK:function f(): undefined
Expand Down
6 changes: 3 additions & 3 deletions test/Optimizer/inline-new-target.js
Original file line number Diff line number Diff line change
Expand Up @@ -61,8 +61,8 @@ function outer2(){
// CHECK-NEXT: %2 = CreateFunctionInst (:object) %1: environment, %foo(): functionCode
// CHECK-NEXT: StoreFrameInst %1: environment, %2: object, [%VS1.foo]: object
// CHECK-NEXT: %4 = CreateFunctionInst (:object) %1: environment, %bar(): functionCode
// CHECK-NEXT: %5 = CreateThisInst (:undefined|object) %4: object, empty: any
// CHECK-NEXT: %6 = CallInst (:object) %4: object, %bar(): functionCode, true: boolean, %1: environment, undefined: undefined, %5: undefined|object
// CHECK-NEXT: %5 = CreateThisInst (:object) %4: object, empty: any
// CHECK-NEXT: %6 = CallInst (:object) %4: object, %bar(): functionCode, true: boolean, %1: environment, undefined: undefined, %5: object
// CHECK-NEXT: ReturnInst %6: object
// CHECK-NEXT:function_end

Expand All @@ -76,6 +76,6 @@ function outer2(){
// CHECK-NEXT:%BB0:
// CHECK-NEXT: %0 = GetParentScopeInst (:environment) %VS1: any, %parentScope: environment
// CHECK-NEXT: %1 = LoadFrameInst (:object) %0: environment, [%VS1.foo]: object
// CHECK-NEXT: %2 = CreateThisInst (:undefined|object) %1: object, empty: any
// CHECK-NEXT: %2 = CreateThisInst (:object) %1: object, empty: any
// CHECK-NEXT: ReturnInst %1: object
// CHECK-NEXT:function_end
11 changes: 5 additions & 6 deletions test/Optimizer/inline-sibling-constructors.js
Original file line number Diff line number Diff line change
Expand Up @@ -66,10 +66,9 @@ function outer() {
// CHECK-NEXT: %2 = LoadParamInst (:any) %y: any
// CHECK-NEXT: %3 = LoadParamInst (:any) %z: any
// CHECK-NEXT: %4 = LoadFrameInst (:object) %0: environment, [%VS1.Point]: object
// CHECK-NEXT: %5 = CreateThisInst (:undefined|object) %4: object, empty: any
// CHECK-NEXT: StorePropertyStrictInst %1: any, %5: undefined|object, "x": string
// CHECK-NEXT: StorePropertyStrictInst %2: any, %5: undefined|object, "y": string
// CHECK-NEXT: StorePropertyStrictInst %3: any, %5: undefined|object, "z": string
// CHECK-NEXT: %9 = GetConstructedObjectInst (:object) %5: undefined|object, undefined: undefined
// CHECK-NEXT: ReturnInst %9: object
// CHECK-NEXT: %5 = CreateThisInst (:object) %4: object, empty: any
// CHECK-NEXT: StorePropertyStrictInst %1: any, %5: object, "x": string
// CHECK-NEXT: StorePropertyStrictInst %2: any, %5: object, "y": string
// CHECK-NEXT: StorePropertyStrictInst %3: any, %5: object, "z": string
// CHECK-NEXT: ReturnInst %5: object
// CHECK-NEXT:function_end
9 changes: 4 additions & 5 deletions test/Optimizer/regress-construct.js
Original file line number Diff line number Diff line change
Expand Up @@ -58,11 +58,10 @@ function main() {
// CHECK-NEXT: CondBranchInst %1: null|object, %BB2, %BB1
// CHECK-NEXT:%BB1:
// CHECK-NEXT: %3 = LoadFrameInst (:object) %0: environment, [%VS1.y]: object
// CHECK-NEXT: %4 = CreateThisInst (:undefined|object) %3: object, empty: any
// CHECK-NEXT: %5 = GetConstructedObjectInst (:object) %4: undefined|object, undefined: undefined
// CHECK-NEXT: StoreFrameInst %0: environment, %5: object, [%VS1.x]: null|object
// CHECK-NEXT: %4 = CreateThisInst (:object) %3: object, empty: any
// CHECK-NEXT: StoreFrameInst %0: environment, %4: object, [%VS1.x]: null|object
// CHECK-NEXT: BranchInst %BB2
// CHECK-NEXT:%BB2:
// CHECK-NEXT: %8 = PhiInst (:null|object) %1: null|object, %BB0, %5: object, %BB1
// CHECK-NEXT: ReturnInst %8: null|object
// CHECK-NEXT: %7 = PhiInst (:null|object) %1: null|object, %BB0, %4: object, %BB1
// CHECK-NEXT: ReturnInst %7: null|object
// CHECK-NEXT:function_end

0 comments on commit 5ff83f1

Please sign in to comment.