diff --git a/lib/Optimizer/Scalar/CacheNewObject.cpp b/lib/Optimizer/Scalar/CacheNewObject.cpp index 7dbe93fe073..39298a1489b 100644 --- a/lib/Optimizer/Scalar/CacheNewObject.cpp +++ b/lib/Optimizer/Scalar/CacheNewObject.cpp @@ -5,12 +5,207 @@ * LICENSE file in the root directory of this source tree. */ +//===----------------------------------------------------------------------===// +/// \file +/// +/// The CacheNewObject optimization adds a CacheNewObjectInst at the start +/// of the function if it detects that the function has an "initialization +/// block", i.e. a sequence of writes to the `this` parameter of fixed +/// properties. +/// For example: +/// this.x = x; +/// this.y = y; +/// // etc. +/// constitutes an initialization block and we want to be able to cache the +/// hidden class and just fetch it so we don't have to rebuild it each time +/// this function is called to make a new object. +/// +/// The instruction checks new.target to ensure that the function is being +/// called as a constructor, ensuring `this` is a new object. +/// +/// This optimization has a few requirements: +/// 1. Property initializations must be ordered so that every user of 'this' +/// dominates subsequent users. +/// 2. The only users of "this" in the chain must be StorePropertyInsts +/// which store to it. +/// 3. Property writes to "this" must use literal keys (not use computed keys). +/// +/// As a result, the optimization is fairly conservative but should account +/// for many of the most common construction patterns. +/// +/// TODO: There are ways to make this optimization more complex: +/// * Use a more involved heuristic for determining if a function is likely +/// to be invoked as a constructor. +/// * When we have classes, we know which functions are constructors. + #define DEBUG_TYPE "cachenewobject" +#include "hermes/IR/CFG.h" +#include "hermes/IR/IRBuilder.h" +#include "hermes/IR/IRUtils.h" #include "hermes/IR/Instrs.h" #include "hermes/Optimizer/PassManager/Pass.h" namespace hermes { +namespace { + +/// The minimum number of cachable property names to actually insert the cache +/// instruction. +constexpr size_t kMinPropertiesForCache = 2; + +/// \param thisParam the instruction loading the 'this' param in the function. +/// \return vector of unique property names which can be cached, in the order +/// in which they must be cached. +static std::vector getPropsForCaching( + const DominanceInfo &domInfo, + Function *func, + Instruction *thisParam) { + BasicBlock *entryBlock = &func->front(); + + // Put the users of 'this' into a set for fast checking of which instructions + // we care about. + llvh::DenseSet thisUsers{}; + llvh::DenseSet thisUsersBlocks{}; + for (Instruction *user : thisParam->getUsers()) { + thisUsers.insert(user); + thisUsersBlocks.insert(user->getParent()); + } + + auto orderedBlocks = orderBlocksByDominance( + domInfo, entryBlock, [&thisUsersBlocks](BasicBlock *block) -> bool { + // Must dominate all returns as well, because the caller can read the + // value of 'this' after the constructor returns. + return thisUsersBlocks.count(block) != 0 || + llvh::isa(block->getTerminator()); + }); + + // Result vector of property keys to cache. + std::vector props{}; + + // Keep track of keys that we've seen, so we only add unique keys. + llvh::DenseSet seenProps{}; + + // Go through instructions in order to find the StorePropertyInsts we + // care about. + for (BasicBlock *block : orderedBlocks) { + for (Instruction &inst : *block) { + // From here on we only care about instructions that use 'this'. + if (!thisUsers.count(&inst)) + continue; + + StorePropertyInst *store = llvh::dyn_cast(&inst); + + // 'this' used outside of a StorePropertyInst, bail. + if (!store) + return props; + + auto *prop = llvh::dyn_cast(store->getProperty()); + + // Property name is not a literal string, bail. + if (!prop) + return props; + + // Check if "this" is being used in a non-Object operand position. + for (uint32_t i = 0, e = store->getNumOperands(); i < e; ++i) { + if (i != StorePropertyInst::ObjectIdx && + store->getOperand(i) == thisParam) { + return props; + } + } + + // Valid store for caching, append to the list if it's new. + if (!seenProps.count(prop)) { + props.push_back(prop); + seenProps.insert(prop); + } + } + } + + return props; +} + +/// Insert the CacheNewObject instruction into \p func after \p thisParam. +/// \param keys the literal names of the keys of the this object. +/// \param thisParam the instruction loading the 'this' param in the function. +static void insertCacheInstruction( + Function *func, + llvh::ArrayRef keys, + Instruction *thisParam) { + IRBuilder builder{func}; + + builder.setInsertionPointAfter(thisParam); + GetNewTargetInst *newTargetInst = + builder.createGetNewTargetInst(func->getNewTargetParam()); + builder.createCacheNewObjectInst(thisParam, newTargetInst, keys); +} + +/// Attempt to cache the new object in \p F. +/// \return true if the function was modified. +static bool cacheNewObjectInFunction(Function *func) { + LLVM_DEBUG( + llvh::dbgs() << "Attempting to cache new object in function: '" + << func->getInternalNameStr() << "'\n"); + + if (func->getDefinitionKind() != Function::DefinitionKind::ES5Function) { + // Bail if the function is not a normal function. + // TODO: Apply this optimization to ES6 constructors once they're + // implemented. + return false; + } + + JSDynamicParam *thisDynParam = func->getJSDynamicParam(0); + if (!thisDynParam->hasOneUser()) { + // Bail if there's no users or if there's more than one LoadParam. + return false; + } + + Instruction *thisParam = + llvh::dyn_cast(thisDynParam->getUsers().front()); + + if (!thisParam || !thisParam->hasUsers()) { + // No usage of 'this', don't cache anything. + return false; + } + + // In loose functions, 'this' can also be coerced into an object, + // so check for that and update to the 'this' that's actually used. + // If the function is invoked as a constructor, + // 'this' is already an object, CoerceThisNS is effectively a Mov, + // and it actually is the same object. + for (Instruction *user : thisParam->getUsers()) { + if (auto *coerce = llvh::dyn_cast(user)) { + thisParam = coerce; + break; + } + } + + DominanceInfo domInfo{func}; + std::vector keys = + getPropsForCaching(domInfo, func, thisParam); + + // Not enough stores to cache. + if (keys.size() < kMinPropertiesForCache) { + LLVM_DEBUG( + llvh::dbgs() << llvh::format( + "Not caching new object, needs at least %u keys, found %u\n", + kMinPropertiesForCache, + keys.size())); + return false; + } + + LLVM_DEBUG(llvh::dbgs() << llvh::format("Caching %u keys\n", keys.size())); + + static_assert( + kMinPropertiesForCache > 0, + "CacheNewObjectInst requires at least one key"); + + // Actually insert the CacheNewObject instruction. + insertCacheInstruction(func, keys, thisParam); + + return true; +} + +} // namespace Pass *createCacheNewObject() { /// Inserts the CacheNewObjectInst if possible, to reduce the time spent @@ -21,7 +216,7 @@ Pass *createCacheNewObject() { ~ThisPass() override = default; bool runOnFunction(Function *F) override { - return false; + return cacheNewObjectInFunction(F); } }; diff --git a/test/BCGen/HBC/cache-new-object.js b/test/BCGen/HBC/cache-new-object.js new file mode 100644 index 00000000000..5ae52e7357b --- /dev/null +++ b/test/BCGen/HBC/cache-new-object.js @@ -0,0 +1,80 @@ +/** + * Copyright (c) Meta Platforms, Inc. and affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +// RUN: %hermesc -O -dump-bytecode %s | %FileCheckOrRegen --match-full-lines %s + +function simple(x, y) { + // Check that CacheNewObject is used and refers to the shape table entry. + this.x = x; + this.y = y; +} + +// Auto-generated content below. Please do not modify manually. + +// CHECK:Bytecode File Information: +// CHECK-NEXT: Bytecode version number: {{.*}} +// CHECK-NEXT: Source hash: {{.*}} +// CHECK-NEXT: Function count: 2 +// CHECK-NEXT: String count: 4 +// CHECK-NEXT: BigInt count: 0 +// CHECK-NEXT: String Kind Entry count: 2 +// CHECK-NEXT: RegExp count: 0 +// CHECK-NEXT: Segment ID: 0 +// CHECK-NEXT: CommonJS module count: 0 +// CHECK-NEXT: CommonJS module count (static): 0 +// CHECK-NEXT: Function source count: 0 +// CHECK-NEXT: Bytecode options: +// CHECK-NEXT: staticBuiltins: 0 +// CHECK-NEXT: cjsModulesStaticallyResolved: 0 + +// CHECK:Global String Table: +// CHECK-NEXT:s0[ASCII, 0..5]: global +// CHECK-NEXT:i1[ASCII, 6..11] #147A1A16: simple +// CHECK-NEXT:i2[ASCII, 12..12] #0001E7F9: x +// CHECK-NEXT:i3[ASCII, 13..13] #0001E3E8: y + +// CHECK:Object Key Buffer: +// CHECK-NEXT:[String 2] +// CHECK-NEXT:[String 3] +// CHECK-NEXT:Object Shape Table: +// CHECK-NEXT:0[0, 2] +// CHECK-NEXT:Function(1 params, 3 registers, 0 numbers, 1 non-pointers): +// CHECK-NEXT:Offset in debug table: source 0x0000, lexical 0x0000 +// CHECK-NEXT: DeclareGlobalVar "simple" +// CHECK-NEXT: CreateTopLevelEnvironment r1, 0 +// CHECK-NEXT: CreateClosure r2, r1, Function +// CHECK-NEXT: GetGlobalObject r1 +// CHECK-NEXT: PutByIdLoose r1, r2, 1, "simple" +// CHECK-NEXT: LoadConstUndefined r0 +// CHECK-NEXT: Ret r0 + +// CHECK:Function(3 params, 3 registers, 0 numbers, 1 non-pointers): +// CHECK-NEXT:Offset in debug table: source 0x000a, lexical 0x0000 +// CHECK-NEXT: LoadThisNS r2 +// CHECK-NEXT: GetNewTarget r1 +// CHECK-NEXT: CacheNewObject r2, 0 +// CHECK-NEXT: LoadParam r1, 1 +// CHECK-NEXT: PutByIdLoose r2, r1, 1, "x" +// CHECK-NEXT: LoadParam r1, 2 +// CHECK-NEXT: PutByIdLoose r2, r1, 2, "y" +// CHECK-NEXT: LoadConstUndefined r0 +// CHECK-NEXT: Ret r0 + +// CHECK:Debug filename table: +// CHECK-NEXT: 0: {{.*}}cache-new-object.js + +// CHECK:Debug file table: +// CHECK-NEXT: source table offset 0x0000: filename id 0 + +// CHECK:Debug source table: +// CHECK-NEXT: 0x0000 function idx 0, starts at line 10 col 1 +// CHECK-NEXT: bc 0: line 10 col 1 +// CHECK-NEXT: bc 18: line 10 col 1 +// CHECK-NEXT: 0x000a function idx 1, starts at line 10 col 1 +// CHECK-NEXT: bc 13: line 12 col 10 +// CHECK-NEXT: bc 22: line 13 col 10 +// CHECK-NEXT: 0x0014 end of debug source table diff --git a/test/Optimizer/cache-new-object-analysis.js b/test/Optimizer/cache-new-object-analysis.js new file mode 100644 index 00000000000..c8079f19629 --- /dev/null +++ b/test/Optimizer/cache-new-object-analysis.js @@ -0,0 +1,56 @@ +/** + * Copyright (c) Meta Platforms, Inc. and affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +// RUN: %hermesc -fno-inline -dump-ir %s -O | %FileCheckOrRegen --match-full-lines %s + +function main() { + +// Make sure allCallsitesKnownInStrictMode is true. +function simple(x, y) { + this.x = x; + this.y = y; +} + +return new simple(1, 2); + +} + +// Auto-generated content below. Please do not modify manually. + +// CHECK:scope %VS0 [] + +// CHECK:function global(): undefined +// CHECK-NEXT:%BB0: +// CHECK-NEXT: %0 = CreateScopeInst (:environment) %VS0: any, empty: any +// CHECK-NEXT: DeclareGlobalVarInst "main": string +// CHECK-NEXT: %2 = CreateFunctionInst (:object) %0: environment, %main(): functionCode +// CHECK-NEXT: StorePropertyLooseInst %2: object, globalObject: object, "main": string +// CHECK-NEXT: ReturnInst undefined: undefined +// CHECK-NEXT:function_end + +// CHECK:function main(): object +// 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:function_end + +// CHECK:function simple(x: any, y: any): undefined [allCallsitesKnownInStrictMode] +// CHECK-NEXT:%BB0: +// CHECK-NEXT: %0 = LoadParamInst (:any) %: any +// CHECK-NEXT: %1 = CoerceThisNSInst (:object) %0: any +// CHECK-NEXT: %2 = GetNewTargetInst (:undefined|object) %new.target: undefined|object +// CHECK-NEXT: CacheNewObjectInst %1: object, %2: undefined|object, "x": string, "y": string +// CHECK-NEXT: %4 = LoadParamInst (:any) %x: any +// CHECK-NEXT: %5 = LoadParamInst (:any) %y: any +// CHECK-NEXT: StorePropertyLooseInst %4: any, %1: object, "x": string +// CHECK-NEXT: StorePropertyLooseInst %5: any, %1: object, "y": string +// CHECK-NEXT: ReturnInst undefined: undefined +// CHECK-NEXT:function_end diff --git a/test/Optimizer/cache-new-object-bail.js b/test/Optimizer/cache-new-object-bail.js new file mode 100644 index 00000000000..6ce0dadcbfe --- /dev/null +++ b/test/Optimizer/cache-new-object-bail.js @@ -0,0 +1,40 @@ +/** + * Copyright (c) Meta Platforms, Inc. and affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +// RUN: %hermesc -dump-ir %s -O | %FileCheck --match-full-lines %s + +// A variety of different functions which don't trigger CacheNewObjectInst +// to be emitted. + +function cond(x, y) { + if (y) + this.y = y; + this.x = x; +} + +function assignProp(x, y) { + x[this] = y; +} + +function assignNotLit(x, y) { + this[x] = x; +} + +function usesThis(x, y) { + this.x = x; + this.y = this.z; +} + +function noUses(x, y) { + print(x); +} + +function callArg(x, y) { + print(this); +} + +// CHECK-NOT: %{{.*}} CacheNewObjectInst {{.*}} diff --git a/test/Optimizer/cache-new-object.js b/test/Optimizer/cache-new-object.js new file mode 100644 index 00000000000..13770637f25 --- /dev/null +++ b/test/Optimizer/cache-new-object.js @@ -0,0 +1,123 @@ +/** + * Copyright (c) Meta Platforms, Inc. and affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +// RUN: %hermesc -dump-ir %s -O | %FileCheckOrRegen --match-full-lines %s + +function simple(x, y) { + this.x = x; + this.y = y; +} + +function simpleWithBranch(x, y) { + this.x = x; + this.y = y; + this.z = x ? 1 : 2; +} + +function beforeCond(x, y, z) { + this.x = x; + this.y = y; + if (z) { + this.z = z; + } +} + +function uniq(x, y, z) { + this.x = x; + this.y = y; + this.z = z; + this.x = x; +} + +// Auto-generated content below. Please do not modify manually. + +// CHECK:scope %VS0 [] + +// CHECK:function global(): undefined +// CHECK-NEXT:%BB0: +// CHECK-NEXT: %0 = CreateScopeInst (:environment) %VS0: any, empty: any +// CHECK-NEXT: DeclareGlobalVarInst "simple": string +// CHECK-NEXT: DeclareGlobalVarInst "simpleWithBranch": string +// CHECK-NEXT: DeclareGlobalVarInst "beforeCond": string +// CHECK-NEXT: DeclareGlobalVarInst "uniq": string +// CHECK-NEXT: %5 = CreateFunctionInst (:object) %0: environment, %simple(): functionCode +// CHECK-NEXT: StorePropertyLooseInst %5: object, globalObject: object, "simple": string +// CHECK-NEXT: %7 = CreateFunctionInst (:object) %0: environment, %simpleWithBranch(): functionCode +// CHECK-NEXT: StorePropertyLooseInst %7: object, globalObject: object, "simpleWithBranch": string +// CHECK-NEXT: %9 = CreateFunctionInst (:object) %0: environment, %beforeCond(): functionCode +// CHECK-NEXT: StorePropertyLooseInst %9: object, globalObject: object, "beforeCond": string +// CHECK-NEXT: %11 = CreateFunctionInst (:object) %0: environment, %uniq(): functionCode +// CHECK-NEXT: StorePropertyLooseInst %11: object, globalObject: object, "uniq": string +// CHECK-NEXT: ReturnInst undefined: undefined +// CHECK-NEXT:function_end + +// CHECK:function simple(x: any, y: any): undefined +// CHECK-NEXT:%BB0: +// CHECK-NEXT: %0 = LoadParamInst (:any) %: any +// CHECK-NEXT: %1 = CoerceThisNSInst (:object) %0: any +// CHECK-NEXT: %2 = GetNewTargetInst (:undefined|object) %new.target: undefined|object +// CHECK-NEXT: CacheNewObjectInst %1: object, %2: undefined|object, "x": string, "y": string +// CHECK-NEXT: %4 = LoadParamInst (:any) %x: any +// CHECK-NEXT: %5 = LoadParamInst (:any) %y: any +// CHECK-NEXT: StorePropertyLooseInst %4: any, %1: object, "x": string +// CHECK-NEXT: StorePropertyLooseInst %5: any, %1: object, "y": string +// CHECK-NEXT: ReturnInst undefined: undefined +// CHECK-NEXT:function_end + +// CHECK:function simpleWithBranch(x: any, y: any): undefined +// CHECK-NEXT:%BB0: +// CHECK-NEXT: %0 = LoadParamInst (:any) %: any +// CHECK-NEXT: %1 = CoerceThisNSInst (:object) %0: any +// CHECK-NEXT: %2 = GetNewTargetInst (:undefined|object) %new.target: undefined|object +// CHECK-NEXT: CacheNewObjectInst %1: object, %2: undefined|object, "x": string, "y": string, "z": string +// CHECK-NEXT: %4 = LoadParamInst (:any) %x: any +// CHECK-NEXT: %5 = LoadParamInst (:any) %y: any +// CHECK-NEXT: StorePropertyLooseInst %4: any, %1: object, "x": string +// CHECK-NEXT: StorePropertyLooseInst %5: any, %1: object, "y": string +// CHECK-NEXT: CondBranchInst %4: any, %BB1, %BB2 +// CHECK-NEXT:%BB1: +// CHECK-NEXT: BranchInst %BB2 +// CHECK-NEXT:%BB2: +// CHECK-NEXT: %10 = PhiInst (:number) 1: number, %BB1, 2: number, %BB0 +// CHECK-NEXT: StorePropertyLooseInst %10: number, %1: object, "z": string +// CHECK-NEXT: ReturnInst undefined: undefined +// CHECK-NEXT:function_end + +// CHECK:function beforeCond(x: any, y: any, z: any): undefined +// CHECK-NEXT:%BB0: +// CHECK-NEXT: %0 = LoadParamInst (:any) %: any +// CHECK-NEXT: %1 = CoerceThisNSInst (:object) %0: any +// CHECK-NEXT: %2 = GetNewTargetInst (:undefined|object) %new.target: undefined|object +// CHECK-NEXT: CacheNewObjectInst %1: object, %2: undefined|object, "x": string, "y": string +// CHECK-NEXT: %4 = LoadParamInst (:any) %x: any +// CHECK-NEXT: %5 = LoadParamInst (:any) %y: any +// CHECK-NEXT: %6 = LoadParamInst (:any) %z: any +// CHECK-NEXT: StorePropertyLooseInst %4: any, %1: object, "x": string +// CHECK-NEXT: StorePropertyLooseInst %5: any, %1: object, "y": string +// CHECK-NEXT: CondBranchInst %6: any, %BB1, %BB2 +// CHECK-NEXT:%BB1: +// CHECK-NEXT: StorePropertyLooseInst %6: any, %1: object, "z": string +// CHECK-NEXT: BranchInst %BB2 +// CHECK-NEXT:%BB2: +// CHECK-NEXT: ReturnInst undefined: undefined +// CHECK-NEXT:function_end + +// CHECK:function uniq(x: any, y: any, z: any): undefined +// CHECK-NEXT:%BB0: +// CHECK-NEXT: %0 = LoadParamInst (:any) %: any +// CHECK-NEXT: %1 = CoerceThisNSInst (:object) %0: any +// CHECK-NEXT: %2 = GetNewTargetInst (:undefined|object) %new.target: undefined|object +// CHECK-NEXT: CacheNewObjectInst %1: object, %2: undefined|object, "x": string, "y": string, "z": string +// CHECK-NEXT: %4 = LoadParamInst (:any) %x: any +// CHECK-NEXT: %5 = LoadParamInst (:any) %y: any +// CHECK-NEXT: %6 = LoadParamInst (:any) %z: any +// CHECK-NEXT: StorePropertyLooseInst %4: any, %1: object, "x": string +// CHECK-NEXT: StorePropertyLooseInst %5: any, %1: object, "y": string +// CHECK-NEXT: StorePropertyLooseInst %6: any, %1: object, "z": string +// CHECK-NEXT: StorePropertyLooseInst %4: any, %1: object, "x": string +// CHECK-NEXT: ReturnInst undefined: undefined +// CHECK-NEXT:function_end