Skip to content

Commit

Permalink
Add a flag for optimized eval
Browse files Browse the repository at this point in the history
Summary:
Sometimes it might be desired to run compiler optimizations on the JS source provided as an
argument to `eval`. For instance:
* If a program is mostly composed of large `eval` statements
* If we wanted to test the optimizer via `eval` statements

So add a runtime flag (that defaults to off) allowing the control of this behavior.

When the flag is set, lazy compilation is always disabled.

This change brought out a bug in DCE, which could accidentally delete the entry point of the module.
Add an explicit check for cases when this is not also global scope.

Reviewed By: dulinriley

Differential Revision: D15329403

fbshipit-source-id: 276e90d40428ccf546b8db595bb0f78056b43e70
  • Loading branch information
Wei Wang authored and facebook-github-bot committed Jun 15, 2020
1 parent 064e4a0 commit 78aef9e
Show file tree
Hide file tree
Showing 10 changed files with 24 additions and 6 deletions.
1 change: 1 addition & 0 deletions include/hermes/CompilerDriver/CompilerDriver.h
Original file line number Diff line number Diff line change
Expand Up @@ -96,5 +96,6 @@ extern llvm::cl::opt<bool> VerifyIR;
extern llvm::cl::opt<bool> EmitAsyncBreakCheck;
extern llvm::cl::opt<bool> AllowFunctionToString;
extern llvm::cl::list<std::string> InputFilenames;
extern llvm::cl::opt<bool> OptimizedEval;
} // namespace cl
#endif
3 changes: 3 additions & 0 deletions include/hermes/VM/Runtime.h
Original file line number Diff line number Diff line change
Expand Up @@ -662,6 +662,9 @@ class Runtime : public HandleRootOwner,
const bool enableEval;
/// Whether to verify the IR being generated by eval and the Function ctor.
const bool verifyEvalIR;
/// Whether to optimize the code in the string passed to eval and the Function
/// ctor.
const bool optimizedEval;

#ifdef HERMESVM_PROFILER_OPCODE
/// Track the frequency of each opcode in the interpreter.
Expand Down
9 changes: 4 additions & 5 deletions lib/BCGen/HBC/BytecodeProviderFromSrc.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,10 @@ BCProviderFromSrc::createBCProviderFromSrc(

context->setStrictMode(compileFlags.strict);
context->setEnableEval(true);
context->setLazyCompilation(compileFlags.lazy);
if (!compileFlags.optimize) {
context->setLazyCompilation(compileFlags.lazy);
}

context->setAllowFunctionToStringWithRuntimeSource(
compileFlags.allowFunctionToStringWithRuntimeSource);
#ifdef HERMES_ENABLE_DEBUGGER
Expand Down Expand Up @@ -225,10 +228,6 @@ BCProviderFromSrc::createBCProviderFromSrc(
return {nullptr, outputManager.getErrorString()};
}

assert(
(compileFlags.optimize ? !compileFlags.lazy : true) &&
"Can't optimize in lazy mode.");

if (compileFlags.optimize && runOptimizationPasses)
runOptimizationPasses(M);

Expand Down
5 changes: 5 additions & 0 deletions lib/CompilerDriver/CompilerDriver.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -293,6 +293,11 @@ opt<bool> AllowFunctionToString(
init(false),
desc("Enables Function.toString() to return source-code when available"));

opt<bool> OptimizedEval(
"optimized-eval",
desc("Turn on compiler optimizations in eval."),
init(false));

static list<std::string> IncludeGlobals(
"include-globals",
desc("Include the definitions of global properties (can be "
Expand Down
5 changes: 4 additions & 1 deletion lib/Optimizer/Scalar/DCE.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,10 @@ bool DCE::runOnModule(Module *M) {
// If the function is a top-level module.
continue;
}
if (!F.isGlobalScope() && !F.hasUsers()) {
// Don't delete the function if it is at global scope, or if it is the
// entry point of a module.
if (!F.isGlobalScope() && &F != M->getTopLevelFunction() &&
!F.hasUsers()) {
toRemove.push_back(&F);
toDestroy.push_back(&F);
changed = true;
Expand Down
1 change: 1 addition & 0 deletions lib/VM/JSLib/eval.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ CallResult<HermesValue> evalInEnvironment(
hbc::CompileFlags compileFlags;
compileFlags.strict = false;
compileFlags.includeLibHermes = false;
compileFlags.optimize = runtime->optimizedEval;
compileFlags.lazy =
utf8code.size() >= hbc::kDefaultSizeThresholdForLazyCompilation;
compileFlags.allowFunctionToStringWithRuntimeSource =
Expand Down
1 change: 1 addition & 0 deletions lib/VM/Runtime.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,7 @@ Runtime::Runtime(
// The initial heap size can't be larger than the max.
: enableEval(runtimeConfig.getEnableEval()),
verifyEvalIR(runtimeConfig.getVerifyEvalIR()),
optimizedEval(runtimeConfig.getOptimizedEval()),
heap_(
getMetadataTable(),
this,
Expand Down
3 changes: 3 additions & 0 deletions public/hermes/Public/RuntimeConfig.h
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,9 @@ class Deserializer;
/* Whether to verify the IR generated by eval and Function ctor */ \
F(constexpr, bool, VerifyEvalIR, false) \
\
/* Whether to optimize the code inside eval and Function ctor */ \
F(constexpr, bool, OptimizedEval, false) \
\
/* Support for ES6 Proxy. */ \
F(constexpr, bool, ES6Proxy, false) \
\
Expand Down
1 change: 1 addition & 0 deletions test/hermes/eval.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
*/

// RUN: LC_ALL=en_US.UTF-8 %hermes -O -Wno-direct-eval %s | %FileCheck --match-full-lines %s
// RUN: LC_ALL=en_US.UTF-8 %hermes -O -optimized-eval -Wno-direct-eval %s | %FileCheck --match-full-lines %s
"use strict";

print('eval');
Expand Down
1 change: 1 addition & 0 deletions tools/hermes/hermes.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,7 @@ static int executeHBCBytecodeFromCL(
.withEnableJIT(cl::DumpJITCode || cl::EnableJIT)
.withEnableEval(cl::EnableEval)
.withVerifyEvalIR(cl::VerifyIR)
.withOptimizedEval(cl::OptimizedEval)
.withVMExperimentFlags(cl::VMExperimentFlags)
.withES6Proxy(cl::ES6Proxy)
.withES6Symbol(cl::ES6Symbol)
Expand Down

0 comments on commit 78aef9e

Please sign in to comment.