8133785: SharedScopeCall should be enabled for non-optimistic call sites in optimistic compilation

Reviewed-by: hannesw, lagergren

nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/codegen/CodeGenerator.java

@@ -249,7 +249,7 @@ final class CodeGenerator extends NodeOperatorVisitor<CodeGeneratorLexicalContex
     private final Set<String> emittedMethods = new HashSet<>();
 
     // Function Id -> ContinuationInfo. Used by compilation of rest-of function only.
-    private final Map<Integer, ContinuationInfo> fnIdToContinuationInfo = new HashMap<>();
+    private ContinuationInfo continuationInfo;
 
     private final Deque<Label> scopeEntryLabels = new ArrayDeque<>();
 
@@ -349,11 +349,20 @@ final class CodeGenerator extends NodeOperatorVisitor<CodeGeneratorLexicalContex
         final int flags = getScopeCallSiteFlags(symbol);
         if (isFastScope(symbol)) {
             // Only generate shared scope getter for fast-scope symbols so we know we can dial in correct scope.
-            if (symbol.getUseCount() > SharedScopeCall.FAST_SCOPE_GET_THRESHOLD && !isOptimisticOrRestOf()) {
+            if (symbol.getUseCount() > SharedScopeCall.FAST_SCOPE_GET_THRESHOLD && !identNode.isOptimistic()) {
-                method.loadCompilerConstant(SCOPE);
+                // As shared scope vars are only used with non-optimistic identifiers, we switch from using TypeBounds to
-                // As shared scope vars are only used in non-optimistic compilation, we switch from using TypeBounds to
                 // just a single definitive type, resultBounds.widest.
-                loadSharedScopeVar(resultBounds.widest, symbol, flags);
+                new OptimisticOperation(identNode, TypeBounds.OBJECT) {
+                    @Override
+                    void loadStack() {
+                        method.loadCompilerConstant(SCOPE);
+                    }
+
+                    @Override
+                    void consumeStack() {
+                        loadSharedScopeVar(resultBounds.widest, symbol, flags);
+                    }
+                }.emit();
             } else {
                 new LoadFastScopeVar(identNode, resultBounds, flags).emit();
             }
@@ -384,10 +393,6 @@ final class CodeGenerator extends NodeOperatorVisitor<CodeGeneratorLexicalContex
         return continuationEntryPoints != null;
     }
 
-    private boolean isOptimisticOrRestOf() {
-        return useOptimisticTypes() || isRestOf();
-    }
-
     private boolean isCurrentContinuationEntryPoint(final int programPoint) {
         return isRestOf() && getCurrentContinuationEntryPoint() == programPoint;
     }
@@ -464,12 +469,8 @@ final class CodeGenerator extends NodeOperatorVisitor<CodeGeneratorLexicalContex
     }
 
     private MethodEmitter loadSharedScopeVar(final Type valueType, final Symbol symbol, final int flags) {
-        assert !isOptimisticOrRestOf();
+        assert isFastScope(symbol);
-        if (isFastScope(symbol)) {
+        method.load(getScopeProtoDepth(lc.getCurrentBlock(), symbol));
-            method.load(getScopeProtoDepth(lc.getCurrentBlock(), symbol));
-        } else {
-            method.load(-1);
-        }
         return lc.getScopeGet(unit, symbol, valueType, flags).generateInvoke(method);
     }
 
@@ -1573,7 +1574,7 @@ final class CodeGenerator extends NodeOperatorVisitor<CodeGeneratorLexicalContex
                     } else if (useCount <= SharedScopeCall.FAST_SCOPE_CALL_THRESHOLD
                             || !isFastScope(symbol) && useCount <= SharedScopeCall.SLOW_SCOPE_CALL_THRESHOLD
                             || CodeGenerator.this.lc.inDynamicScope()
-                            || isOptimisticOrRestOf()) {
+                            || callNode.isOptimistic()) {
                         scopeCall(node, flags);
                     } else {
                         sharedScopeCall(node, flags);
@@ -2070,8 +2071,6 @@ final class CodeGenerator extends NodeOperatorVisitor<CodeGeneratorLexicalContex
 
     @Override
     public boolean enterFunctionNode(final FunctionNode functionNode) {
-        final int fnId = functionNode.getId();
-
         if (skipFunction(functionNode)) {
             // In case we are not generating code for the function, we must create or retrieve the function object and
             // load it on the stack here.
@@ -2109,9 +2108,9 @@ final class CodeGenerator extends NodeOperatorVisitor<CodeGeneratorLexicalContex
             method.begin();
 
             if (isRestOf()) {
-                final ContinuationInfo ci = new ContinuationInfo();
+                assert continuationInfo == null;
-                fnIdToContinuationInfo.put(fnId, ci);
+                continuationInfo = new ContinuationInfo();
-                method.gotoLoopStart(ci.getHandlerLabel());
+                method.gotoLoopStart(continuationInfo.getHandlerLabel());
             }
         }
 
@@ -5308,7 +5307,7 @@ final class CodeGenerator extends NodeOperatorVisitor<CodeGeneratorLexicalContex
     }
 
     private ContinuationInfo getContinuationInfo() {
-        return fnIdToContinuationInfo.get(lc.getCurrentFunction().getId());
+        return continuationInfo;
     }
 
     private void generateContinuationHandler() {

nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/ScriptEnvironment.java

@@ -46,6 +46,11 @@ import jdk.nashorn.internal.runtime.options.Options;
  * and output and error writers, top level Namespace etc.
  */
 public final class ScriptEnvironment {
+    // Primarily intended to be used in test environments so that eager compilation tests work without an
+    // error when tested with optimistic compilation.
+    private static final boolean ALLOW_EAGER_COMPILATION_SILENT_OVERRIDE = Options.getBooleanProperty(
+            "nashorn.options.allowEagerCompilationSilentOverride", false);
+
     /** Output writer for this environment */
     private final PrintWriter out;
 
@@ -241,8 +246,20 @@ public final class ScriptEnvironment {
         }
         _fx                   = options.getBoolean("fx");
         _global_per_engine    = options.getBoolean("global.per.engine");
-        _lazy_compilation     = options.getBoolean("lazy.compilation");
         _optimistic_types     = options.getBoolean("optimistic.types");
+        final boolean lazy_compilation = options.getBoolean("lazy.compilation");
+        if (!lazy_compilation && _optimistic_types) {
+            if (!ALLOW_EAGER_COMPILATION_SILENT_OVERRIDE) {
+                throw new IllegalStateException(
+                        ECMAErrors.getMessage(
+                                "config.error.eagerCompilationConflictsWithOptimisticTypes",
+                                options.getOptionTemplateByKey("lazy.compilation").getName(),
+                                options.getOptionTemplateByKey("optimistic.types").getName()));
+            }
+            _lazy_compilation = true;
+        } else {
+            _lazy_compilation = lazy_compilation;
+        }
         _loader_per_compile   = options.getBoolean("loader.per.compile");
         _no_java              = options.getBoolean("no.java");
         _no_syntax_extensions = options.getBoolean("no.syntax.extensions");

nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/options/OptionTemplate.java

@@ -304,8 +304,8 @@ public final class OptionTemplate implements Comparable<OptionTemplate> {
         }
     }
 
-    boolean matches(final String key0) {
+    boolean nameMatches(final String aName) {
-        return key0.equals(this.shortName) || key0.equals(this.name);
+        return aName.equals(this.shortName) || aName.equals(this.name);
     }
 
     private static final int LINE_BREAK = 64;

nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/options/Options.java

@@ -520,9 +520,25 @@ public final class Options {
         }
     }
 
-    private static OptionTemplate getOptionTemplate(final String key) {
+    /**
+     * Retrieves an option template identified by key.
+     * @param shortKey the short (that is without the e.g. "nashorn.option." part) key
+     * @return the option template identified by the key
+     * @throws IllegalArgumentException if the key doesn't specify an existing template
+     */
+    public OptionTemplate getOptionTemplateByKey(final String shortKey) {
+        final String fullKey = key(shortKey);
+        for(final OptionTemplate t: validOptions) {
+            if(t.getKey().equals(fullKey)) {
+                return t;
+            }
+        }
+        throw new IllegalArgumentException(shortKey);
+    }
+
+    private static OptionTemplate getOptionTemplateByName(final String name) {
         for (final OptionTemplate t : Options.validOptions) {
-            if (t.matches(key)) {
+            if (t.nameMatches(name)) {
                 return t;
             }
         }
@@ -682,7 +698,7 @@ public final class Options {
             }
 
             final String token = st.nextToken();
-            this.template = Options.getOptionTemplate(token);
+            this.template = getOptionTemplateByName(token);
             if (this.template == null) {
                 throw new IllegalArgumentException(argument);
             }

nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/resources/Messages.properties

@@ -172,7 +172,9 @@ syntax.error.redeclare.variable=Variable "{0}" has already been declared
 syntax.error.unprotected.switch.declaration=Unsupported {0} declaration in unprotected switch statement
 
 io.error.cant.write=cannot write "{0}"
+
 config.error.no.dest=no destination directory supplied
+config.error.eagerCompilationConflictsWithOptimisticTypes={0}=false (eager compilation) is not compatible with {1}=true.
 
 uri.error.bad.uri=Bad URI "{0}" near offset {1}
 list.adapter.null.global=Attempted to create the adapter from outside a JavaScript execution context.

nashorn/test/script/basic/JDK-8053905.js

@@ -28,6 +28,7 @@
  * @runif external.octane
  * @fork
  * @option -Dnashorn.compiler.splitter.threshold=1000
+ * @option -Dnashorn.options.allowEagerCompilationSilentOverride
  * @option -scripting
  * @option --lazy-compilation=false
  */

nashorn/test/script/basic/JDK-8058561.js

@@ -26,7 +26,9 @@
  *
  * @test
  * @run
+ * @fork
  * @option --lazy-compilation=false
+ * @option -Dnashorn.options.allowEagerCompilationSilentOverride
  */
 
 // Just attempting to compile this caused the NPE

nashorn/test/script/basic/JDK-8078612_eager_1a.js

@@ -29,6 +29,7 @@
  * @option -pcc
  * @option --lazy-compilation=false
  * @option -Dnashorn.persistent.code.cache=build/nashorn_code_cache
+ * @option -Dnashorn.options.allowEagerCompilationSilentOverride
  * @fork
  */
 

nashorn/test/script/basic/JDK-8078612_eager_1b.js

@@ -29,6 +29,7 @@
  * @option -pcc
  * @option --lazy-compilation=false
  * @option -Dnashorn.persistent.code.cache=build/nashorn_code_cache
+ * @option -Dnashorn.options.allowEagerCompilationSilentOverride
  * @fork
  */
 

nashorn/test/script/basic/JDK-8078612_eager_2a.js

@@ -29,6 +29,7 @@
  * @option -pcc
  * @option --lazy-compilation=false
  * @option -Dnashorn.persistent.code.cache=build/nashorn_code_cache
+ * @option -Dnashorn.options.allowEagerCompilationSilentOverride
  * @fork
  */
 

nashorn/test/script/basic/JDK-8078612_eager_2b.js

@@ -29,6 +29,7 @@
  * @option -pcc
  * @option --lazy-compilation=false
  * @option -Dnashorn.persistent.code.cache=build/nashorn_code_cache
+ * @option -Dnashorn.options.allowEagerCompilationSilentOverride
  * @fork
  */