Skip to content

Commit

Permalink
Port over SimpleStackPromotion (#770)
Browse files Browse the repository at this point in the history
Summary:
Port over `SimpleStackPromotion` from SH.

Closes #770

Reviewed By: tmikov

Differential Revision: D47885364

fbshipit-source-id: 56e77a6114cda199e476549e3c00f3a49995024d
  • Loading branch information
neildhar authored and facebook-github-bot committed Aug 4, 2023
1 parent 7606c97 commit 85cf592
Show file tree
Hide file tree
Showing 21 changed files with 464 additions and 234 deletions.
1 change: 1 addition & 0 deletions include/hermes/Optimizer/PassManager/Passes.def
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ PASS(Mem2Reg, "mem2reg", "Construct SSA")
PASS(InstSimplify, "instsimplify", "Simplify instructions")
PASS(SimplifyCFG, "simplifycfg", "Simplify CFG")
PASS(StackPromotion, "stackpromotion", "Stack promotion")
PASS(SimpleStackPromotion, "simplestackpromotion", "Simple stack promotion")
PASS(TypeInference, "typeinference", "Type inference")
PASS(Inlining, "inlining", "Inlining")
PASS(ResolveStaticRequire, "staticrequire", "Resolve static require")
Expand Down
1 change: 1 addition & 0 deletions lib/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ add_hermes_library(hermesOptimizer
Optimizer/Scalar/Mem2Reg.cpp
Optimizer/Scalar/TypeInference.cpp
Optimizer/Scalar/StackPromotion.cpp
Optimizer/Scalar/SimpleStackPromotion.cpp
Optimizer/Scalar/InstSimplify.cpp
Optimizer/Scalar/Auditor.cpp
Optimizer/Wasm/WasmSimplify.cpp
Expand Down
8 changes: 4 additions & 4 deletions lib/Optimizer/PassManager/Pipeline.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
#include "hermes/Optimizer/Scalar/DCE.h"
#include "hermes/Optimizer/Scalar/ScopeTransformations.h"
#include "hermes/Optimizer/Scalar/SimplifyCFG.h"
#include "hermes/Optimizer/Scalar/StackPromotion.h"
#include "hermes/Optimizer/Scalar/TypeInference.h"

#include "llvh/Support/Debug.h"
Expand Down Expand Up @@ -57,13 +56,14 @@ void hermes::runFullOptimizationPasses(Module &M) {

PM.addTypeInference();
PM.addSimplifyCFG();
PM.addStackPromotion();
PM.addSimpleStackPromotion();
PM.addMem2Reg();
PM.addStackPromotion();
PM.addSimpleStackPromotion();
PM.addInlining();
PM.addStackPromotion();
PM.addSimpleStackPromotion();
PM.addInstSimplify();
PM.addDCE();
PM.addSimpleStackPromotion();

#ifdef HERMES_RUN_WASM
if (M.getContext().getUseUnsafeIntrinsics()) {
Expand Down
198 changes: 198 additions & 0 deletions lib/Optimizer/Scalar/SimpleStackPromotion.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,198 @@
/*
* 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.
*/

#define DEBUG_TYPE "simplestackpromotion"

#include "hermes/IR/IRBuilder.h"
#include "hermes/IR/Instrs.h"
#include "hermes/Optimizer/PassManager/Pass.h"
#include "hermes/Optimizer/Scalar/Utils.h"
#include "hermes/Support/Statistic.h"

#include "llvh/Support/Debug.h"

STATISTIC(NumConstProm, "Number of loads of known constants promoted");
STATISTIC(NumVarCopy, "Number of variables copied to the stack");
STATISTIC(NumStoreOnlyVarDel, "Number of store only variables deleted");

namespace hermes {
namespace {
/// If \p var is only ever stored to with a single literal value, replace all
/// loads from it with that value, and delete all stores.
/// \return true if \p var was promoted.
bool tryPromoteConstVariable(Variable *var) {
Value *storedVal = isStoreOnceVariable(var);

// Not a single-store variable with a literal value, give up.
if (!storedVal || !llvh::isa<Literal>(storedVal))
return false;

LLVM_DEBUG(
llvh::dbgs() << "Promoting const variable " << var->getName()
<< " with literal value " << storedVal->getKindStr()
<< "\n");
IRBuilder::InstructionDestroyer destroyer;

// Delete all stores to the variable, and replace all loads with the literal.
for (auto *U : var->getUsers()) {
if (llvh::isa<LoadFrameInst>(U)) {
U->replaceAllUsesWith(storedVal);
} else {
assert(llvh::isa<StoreFrameInst>(U) && "Access must be a load or store.");
}
destroyer.add(U);
}
NumConstProm++;
return true;
}

/// If \p var is only ever stored to in its owning function \p func, create a
/// copy of \p var on the stack. Use this stack variable for any loads from \p
/// var in \p func. Note that the stores to the frame are not deleted, which
/// means that loads in inner functions will continue to work correctly.
/// \return true if the variable was promoted.
bool tryCopyToStack(Variable *var) {
auto *func = var->getParent()->getFunction();
bool hasLoadInOwningFunction = false;
bool hasStoreInInnerFunction = false;
for (auto *U : var->getUsers()) {
bool owningFunc = U->getParent()->getParent() == func;
if (llvh::isa<LoadFrameInst>(U))
hasLoadInOwningFunction |= owningFunc;
else
hasStoreInInnerFunction |= !owningFunc;
}

// If the variable is stored to from an inner function, we cannot attempt to
// create a copy of it on the stack. Note that this applies even if all stores
// are in the same inner function, because recursive invocations of that
// function need to modify the same frame variable.
// Likewise, if it doesn't have any loads that can use the new stack value, we
// don't need to create it. The latter prevents us from repeatedly creating
// new stack variables every time the pass runs.
if (hasStoreInInnerFunction || !hasLoadInOwningFunction)
return false;

LLVM_DEBUG(llvh::dbgs() << "Promoting Variable: " << var->getName() << "\n");

IRBuilder builder(func->getParent());

// AllocStack will be inserted at the very start of the function.
builder.setInsertionPoint(&*func->begin()->begin());
auto *stackVar = builder.createAllocStackInst(var->getName());

IRBuilder::InstructionDestroyer destroyer;

for (auto *U : var->getUsers()) {
// Replace all loads in the owning function with loads from the stack.
if (llvh::isa<LoadFrameInst>(U)) {
if (U->getParent()->getParent() == func) {
builder.setInsertionPoint(U);
auto *loadStack = builder.createLoadStackInst(stackVar);
U->replaceAllUsesWith(loadStack);
destroyer.add(U);
}
} else {
// Duplicate all stores so they also store to the stack.
auto *store = llvh::cast<StoreFrameInst>(U);
builder.setInsertionPoint(store);
builder.createStoreStackInst(store->getValue(), stackVar);
}
}

NumVarCopy++;
return true;
}

/// Delete all stores to \p var if there are no loads from it.
/// \return true if stores to the variable were deleted.
bool tryDeleteStoreOnlyVariable(Variable *var) {
if (!var->hasUsers())
return false;
for (auto *U : var->getUsers())
if (!llvh::isa<StoreFrameInst>(U))
return false;
LLVM_DEBUG(
llvh::dbgs() << "Deleting stores to store-only Variable: "
<< var->getName() << "\n");
IRBuilder::InstructionDestroyer destroyer;
for (auto *U : var->getUsers())
destroyer.add(llvh::cast<StoreFrameInst>(U));
NumStoreOnlyVarDel++;
return true;
}

/// Run SimpleStackPromotion on a single function.
/// Iterates all variables of \p func, replacing them usage of them with SSA and
/// stack values.
bool runOnFunction(Function *F) {
bool changed = false;
LLVM_DEBUG(
llvh::dbgs() << "Promoting variables in " << F->getInternalNameStr()
<< "\n");

llvh::SmallVectorImpl<Variable *> &vars =
F->getFunctionScopeDesc()->getMutableVariables();
for (Variable *var : vars) {
// Attempt to replace var with a literal if possible. Note that this removes
// all loads and stores from var, so we can skip the rest of the pass.
if (tryPromoteConstVariable(var)) {
changed = true;
continue;
}
// Try creating a copy of the variable on the stack if it is only ever
// stored to in the owning function. This allows us to eliminate all loads
// from the variable in the owning function.
changed |= tryCopyToStack(var);
// If the variable no longer has any loads at all, delete any remaining
// stores. This may happen because the program never loads from the
// variable, or because all loads from the variable were in the owning
// function as well, and were therefore eliminated by the stack copy.
changed |= tryDeleteStoreOnlyVariable(var);
}

// Delete variables without any users.
for (Variable *&var : vars) {
if (!var->hasUsers()) {
Value::destroy(var);
var = nullptr;
changed = true;
}
}
// Clean up the variable list to remove destroyed entries.
llvh::erase_if(vars, [](Variable *var) { return !var; });

return changed;
}

/// Promotes all non-captured variables into stack allocations.
/// Replaces usage of variables with known literal values.
/// Creates stack copies of variables that are only stored to from the owning
/// function, to allow local values to be forwarded.
bool runSimpleStackPromotion(Module *M) {
bool changed = false;
for (Function &func : *M)
changed |= runOnFunction(&func);
return changed;
}

} // namespace

std::unique_ptr<Pass> createSimpleStackPromotion() {
class ThisPass : public ModulePass {
public:
explicit ThisPass() : ModulePass("SimpleStackPromotion") {}
~ThisPass() override = default;

bool runOnModule(Module *M) override {
return runSimpleStackPromotion(M);
}
};
return std::make_unique<ThisPass>();
}
} // namespace hermes
#undef DEBUG_TYPE
49 changes: 29 additions & 20 deletions test/BCGen/HBC/exceptions.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,31 +28,40 @@ function foo(a) {

//CHECK-LABEL:Function<foo>(2 params, 11 registers, 0 symbols):
//CHECK-NEXT:Offset in debug table: {{.*}}
//CHECK-NEXT:{{.*}} LoadParam 2<Reg8>, 1<UInt8>
//CHECK-NEXT:{{.*}} LoadConstUndefined 1<Reg8>
//CHECK-NEXT:{{.*}} LoadConstUndefined 3<Reg8>
//CHECK-NEXT:{{.*}} Call1 0<Reg8>, 2<Reg8>, 1<Reg8>
//CHECK-NEXT:{{.*}} Jmp 26<Addr8>
//CHECK-NEXT:{{.*}} LoadParam 1<Reg8>, 1<UInt8>
//CHECK-NEXT:{{.*}} Mov 3<Reg8>, 1<Reg8>
//CHECK-NEXT:{{.*}} LoadConstUndefined 0<Reg8>
//CHECK-NEXT:{{.*}} Call1 0<Reg8>, 3<Reg8>, 0<Reg8>
//CHECK-NEXT:{{.*}} Jmp 32<Addr8>
//CHECK-NEXT:{{.*}} Catch 2<Reg8>
//CHECK-NEXT:{{.*}} Mov 3<Reg8>, 2<Reg8>
//CHECK-NEXT:{{.*}} LoadConstUndefined 0<Reg8>
//CHECK-NEXT:{{.*}} Call1 0<Reg8>, 3<Reg8>, 0<Reg8>
//CHECK-NEXT:{{.*}} Jmp 10<Addr8>
//CHECK-NEXT:{{.*}} Catch 3<Reg8>
//CHECK-NEXT:{{.*}} Mov 0<Reg8>, 3<Reg8>
//CHECK-NEXT:{{.*}} Call1 0<Reg8>, 0<Reg8>, 1<Reg8>
//CHECK-NEXT:{{.*}} Jmp 8<Addr8>
//CHECK-NEXT:{{.*}} Catch 0<Reg8>
//CHECK-NEXT:{{.*}} Call1 0<Reg8>, 0<Reg8>, 1<Reg8>
//CHECK-NEXT:{{.*}} Mov 0<Reg8>, 3<Reg8>
//CHECK-NEXT:{{.*}} Call1 0<Reg8>, 0<Reg8>, 1<Reg8>
//CHECK-NEXT:{{.*}} Call1 0<Reg8>, 2<Reg8>, 1<Reg8>
//CHECK-NEXT:{{.*}} Ret 1<Reg8>
//CHECK-NEXT:{{.*}} LoadConstUndefined 0<Reg8>
//CHECK-NEXT:{{.*}} Call1 0<Reg8>, 3<Reg8>, 0<Reg8>
//CHECK-NEXT:{{.*}} Mov 3<Reg8>, 2<Reg8>
//CHECK-NEXT:{{.*}} LoadConstUndefined 0<Reg8>
//CHECK-NEXT:{{.*}} Call1 0<Reg8>, 3<Reg8>, 0<Reg8>
//CHECK-NEXT:{{.*}} Mov 3<Reg8>, 1<Reg8>
//CHECK-NEXT:{{.*}} LoadConstUndefined 0<Reg8>
//CHECK-NEXT:{{.*}} Call1 3<Reg8>, 3<Reg8>, 0<Reg8>
//CHECK-NEXT:{{.*}} Ret 0<Reg8>
//CHECK-NEXT:{{.*}} Catch 0<Reg8>
//CHECK-NEXT:{{.*}} Call1 3<Reg8>, 3<Reg8>, 1<Reg8>
//CHECK-NEXT:{{.*}} Mov 3<Reg8>, 2<Reg8>
//CHECK-NEXT:{{.*}} LoadConstUndefined 2<Reg8>
//CHECK-NEXT:{{.*}} Call1 2<Reg8>, 3<Reg8>, 2<Reg8>
//CHECK-NEXT:{{.*}} Throw 0<Reg8>
//CHECK-NEXT:{{.*}} Catch 0<Reg8>
//CHECK-NEXT:{{.*}} Mov 2<Reg8>, 1<Reg8>
//CHECK-NEXT:{{.*}} LoadConstUndefined 1<Reg8>
//CHECK-NEXT:{{.*}} Call1 1<Reg8>, 2<Reg8>, 1<Reg8>
//CHECK-NEXT:{{.*}} Throw 0<Reg8>

//CHECK-LABEL: Exception Handlers:
//CHECK-NEXT: 0: start = 15, end = 22, target = 24
//CHECK-NEXT: 1: start = 7, end = 11, target = 13
//CHECK-NEXT: 2: start = 15, end = 30, target = 43
//CHECK-NEXT: 3: start = 7, end = 37, target = 51
//CHECK-NEXT: 4: start = 43, end = 51, target = 51
//CHECK-NEXT: 0: start = 16, end = 25, target = 27
//CHECK-NEXT: 1: start = 3, end = 12, target = 14
//CHECK-NEXT: 2: start = 16, end = 35, target = 55
//CHECK-NEXT: 3: start = 3, end = 44, target = 68
//CHECK-NEXT: 4: start = 55, end = 68, target = 68
25 changes: 7 additions & 18 deletions test/BCGen/HBC/store_to_env.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,27 +25,21 @@ function foo() {
return myFunc;
}

// CHECK: Function<foo>(1 params, 10 registers, 6 symbols):
// CHECK: Function<foo>(1 params, 10 registers, 3 symbols):
// CHECK-NEXT: Offset in debug table: {{.*}}
// CHECK-NEXT: CreateEnvironment r0
// CHECK-NEXT: LoadConstInt r1, 1234
// CHECK-NEXT: StoreNPToEnvironment r0, 0, r1
// CHECK-NEXT: LoadConstTrue r1
// CHECK-NEXT: StoreNPToEnvironment r0, 1, r1
// CHECK-NEXT: LoadConstString r1, "a string"
// CHECK-NEXT: StoreToEnvironment r0, 2, r1
// CHECK-NEXT: GetGlobalObject r1
// CHECK-NEXT: TryGetById r1, r1, 1, "Object"
// CHECK-NEXT: GetByIdShort r2, r1, 2, "prototype"
// CHECK-NEXT: CreateThis r2, r2, r1
// CHECK-NEXT: Mov r3, r2
// CHECK-NEXT: Construct r1, r1, 1
// CHECK-NEXT: SelectObject r1, r2, r1
// CHECK-NEXT: StoreToEnvironment r0, 3, r1
// CHECK-NEXT: StoreToEnvironment r0, 1, r1
// CHECK-NEXT: CreateRegExp r1, "Hermes", "i", 0
// CHECK-NEXT: StoreToEnvironment r0, 4, r1
// CHECK-NEXT: LoadConstString r1, "temp string"
// CHECK-NEXT: StoreToEnvironment r0, 5, r1
// CHECK-NEXT: StoreToEnvironment r0, 2, r1
// CHECK-NEXT: CreateClosure r0, r0, Function<bar>
// CHECK-NEXT: Ret r0

Expand All @@ -55,17 +49,12 @@ function foo() {
// CHECK-NEXT: LoadFromEnvironment r0, r1, 0
// CHECK-NEXT: Inc r0, r0
// CHECK-NEXT: StoreToEnvironment r1, 0, r0
// CHECK-NEXT: LoadConstFalse r0
// CHECK-NEXT: StoreNPToEnvironment r1, 1, r0
// CHECK-NEXT: LoadConstString r0, "new string"
// CHECK-NEXT: StoreToEnvironment r1, 2, r0
// CHECK-NEXT: GetGlobalObject r2
// CHECK-NEXT: TryGetById r4, r2, 1, "print"
// CHECK-NEXT: LoadFromEnvironment r3, r1, 3
// CHECK-NEXT: LoadFromEnvironment r3, r1, 1
// CHECK-NEXT: LoadConstUndefined r0
// CHECK-NEXT: Call2 r3, r4, r0, r3
// CHECK-NEXT: TryGetById r3, r2, 1, "print"
// CHECK-NEXT: LoadFromEnvironment r2, r1, 4
// CHECK-NEXT: Call2 r2, r3, r0, r2
// CHECK-NEXT: StoreNPToEnvironment r1, 5, r0
// CHECK-NEXT: TryGetById r2, r2, 1, "print"
// CHECK-NEXT: LoadFromEnvironment r1, r1, 2
// CHECK-NEXT: Call2 r1, r2, r0, r1
// CHECK-NEXT: Ret r0

0 comments on commit 85cf592

Please sign in to comment.