Bug 1382650 part 3 - Clean up Ion eager compilation code. r=nbp a=pascalc
authorJan de Mooij <jdemooij@mozilla.com>
Wed, 27 Mar 2019 12:18:59 +0000
changeset 526039 7d87558da81f21659d5ba11df5564142f88e4703
parent 526038 2e6bccd461f45dc46e328e2ad0254df2faa20736
child 526040 d4a898d579ec1f10696774f3678ad97db02b0ae4
push id2032
push userffxbld-merge
push dateMon, 13 May 2019 09:36:57 +0000
treeherdermozilla-release@455c1065dcbe [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersnbp, pascalc
bugs1382650
milestone67.0
Bug 1382650 part 3 - Clean up Ion eager compilation code. r=nbp a=pascalc There's a lot of complexity around setting/unsetting the eagerCompilation flag. It's simpler to determine this based on the warm-up threshold being 0. The patch also fixes some jit-tests where this patch would result in a change in behavior. Differential Revision: https://phabricator.services.mozilla.com/D24155
js/src/jit-test/tests/cacheir/bug1500255.js
js/src/jit-test/tests/cacheir/bug1526872.js
js/src/jit-test/tests/debug/bug1263899.js
js/src/jit-test/tests/debug/bug1302432.js
js/src/jit-test/tests/ion/bug1526840.js
js/src/jit/Ion.cpp
js/src/jit/IonOptimizationLevels.cpp
js/src/jit/JitOptions.cpp
js/src/jit/JitOptions.h
js/src/jit/Lowering.cpp
js/src/jsapi.cpp
js/src/shell/js.cpp
--- a/js/src/jit-test/tests/cacheir/bug1500255.js
+++ b/js/src/jit-test/tests/cacheir/bug1500255.js
@@ -1,10 +1,11 @@
 
 setJitCompilerOption("offthread-compilation.enable", 0);
+setJitCompilerOption("baseline.warmup.trigger", 0);
 setJitCompilerOption("ion.warmup.trigger", 0);
 
 foo();
 
 function foo() {
     Array.prototype.__proto__ = null;
     Array.prototype[1] = 'bar';
 }
--- a/js/src/jit-test/tests/cacheir/bug1526872.js
+++ b/js/src/jit-test/tests/cacheir/bug1526872.js
@@ -1,10 +1,11 @@
 
 setJitCompilerOption("offthread-compilation.enable", 0);
+setJitCompilerOption("baseline.warmup.trigger", 0);
 setJitCompilerOption("ion.warmup.trigger", 0);
 
 for (var i = 0; i < 5; i++) {
     assertEq(foo(1n), false);
 }
 
 function foo(x) {
     return x == null || x == undefined;
--- a/js/src/jit-test/tests/debug/bug1263899.js
+++ b/js/src/jit-test/tests/debug/bug1263899.js
@@ -1,15 +1,16 @@
 try {
   evaluate(` 
     function runTestCase() $ERROR()
     function $ERROR() {
       throw Error
     }
     Object.defineProperty(this, "x", { value: 0 });
+    setJitCompilerOption("baseline.warmup.trigger", 0);
     setJitCompilerOption("ion.warmup.trigger", 0)
   `)
   evaluate(`function f() {} f(x)`)
   runTestCase()
 } catch (exc) {}
 evaluate(`
   g = newGlobal({newCompartment: true})
   g.parent = this
--- a/js/src/jit-test/tests/debug/bug1302432.js
+++ b/js/src/jit-test/tests/debug/bug1302432.js
@@ -1,8 +1,9 @@
+setJitCompilerOption("baseline.warmup.trigger", 0);
 setJitCompilerOption('ion.warmup.trigger', 0);
 gczeal(7, 1);
 var dbgGlobal = newGlobal({newCompartment: true});
 var dbg = new dbgGlobal.Debugger();
 dbg.addDebuggee(this);
 function f(x, await = () => Array.isArray(revocable.proxy), ...get) {
     dbg.getNewestFrame().older.eval("print(a)");
 }
--- a/js/src/jit-test/tests/ion/bug1526840.js
+++ b/js/src/jit-test/tests/ion/bug1526840.js
@@ -1,10 +1,11 @@
 
 setJitCompilerOption("offthread-compilation.enable", 0);
+setJitCompilerOption("baseline.warmup.trigger", 0);
 setJitCompilerOption("ion.warmup.trigger", 0);
 
 for (let j = 0; j < 2; ++j) {
     let z = j ? 0n : 1;
     if (z) {
         z = 0;
     } else {
         z = 0;
--- a/js/src/jit/Ion.cpp
+++ b/js/src/jit/Ion.cpp
@@ -2347,17 +2347,17 @@ MethodStatus jit::CanEnterIon(JSContext*
       TrackAndSpewIonAbort(cx, script, "too many args");
       ForbidCompilation(cx, script);
       return Method_CantCompile;
     }
   }
 
   // If --ion-eager is used, compile with Baseline first, so that we
   // can directly enter IonMonkey.
-  if (JitOptions.eagerCompilation && !script->hasBaselineScript()) {
+  if (JitOptions.eagerIonCompilation() && !script->hasBaselineScript()) {
     MethodStatus status = CanEnterBaselineMethod(cx, state);
     if (status != Method_Compiled) {
       return status;
     }
   }
 
   MOZ_ASSERT(!script->isIonCompilingOffThread());
   MOZ_ASSERT(script->canIonCompile());
--- a/js/src/jit/IonOptimizationLevels.cpp
+++ b/js/src/jit/IonOptimizationLevels.cpp
@@ -86,17 +86,17 @@ uint32_t OptimizationInfo::compilerWarmU
   }
 
   uint32_t numLocalsAndArgs = NumLocalsAndArgs(script);
   if (numLocalsAndArgs > MAX_MAIN_THREAD_LOCALS_AND_ARGS) {
     warmUpThreshold *=
         (numLocalsAndArgs / double(MAX_MAIN_THREAD_LOCALS_AND_ARGS));
   }
 
-  if (!pc || JitOptions.eagerCompilation) {
+  if (!pc || JitOptions.eagerIonCompilation()) {
     return warmUpThreshold;
   }
 
   // It's more efficient to enter outer loops, rather than inner loops, via OSR.
   // To accomplish this, we use a slightly higher threshold for inner loops.
   // Note that the loop depth is always > 0 so we will prefer non-OSR over OSR.
   uint32_t loopDepth = LoopEntryDepthHint(pc);
   MOZ_ASSERT(loopDepth > 0);
--- a/js/src/jit/JitOptions.cpp
+++ b/js/src/jit/JitOptions.cpp
@@ -131,19 +131,16 @@ DefaultJitOptions::DefaultJitOptions() {
   SET_DEFAULT(disableSincos, false);
 #else
   SET_DEFAULT(disableSincos, true);
 #endif
 
   // Toggles whether sink code motion is globally disabled.
   SET_DEFAULT(disableSink, true);
 
-  // Whether functions are compiled immediately.
-  SET_DEFAULT(eagerCompilation, false);
-
   // Whether IonBuilder should prefer IC generation above specialized MIR.
   SET_DEFAULT(forceInlineCaches, false);
 
   // Toggles whether large scripts are rejected.
   SET_DEFAULT(limitScriptSize, true);
 
   // Toggles whether functions may be entered at loop headers.
   SET_DEFAULT(osr, true);
@@ -262,38 +259,24 @@ DefaultJitOptions::DefaultJitOptions() {
 }
 
 bool DefaultJitOptions::isSmallFunction(JSScript* script) const {
   return script->length() <= smallFunctionMaxBytecodeLength_;
 }
 
 void DefaultJitOptions::enableGvn(bool enable) { disableGvn = !enable; }
 
-void DefaultJitOptions::setEagerCompilation() {
-  eagerCompilation = true;
+void DefaultJitOptions::setEagerIonCompilation() {
   baselineWarmUpThreshold = 0;
   normalIonWarmUpThreshold = 0;
 }
 
 void DefaultJitOptions::setCompilerWarmUpThreshold(uint32_t warmUpThreshold) {
   normalIonWarmUpThreshold = warmUpThreshold;
-
-  // Undo eager compilation
-  if (eagerCompilation && warmUpThreshold != 0) {
-    jit::DefaultJitOptions defaultValues;
-    eagerCompilation = false;
-    baselineWarmUpThreshold = defaultValues.baselineWarmUpThreshold;
-  }
 }
 
 void DefaultJitOptions::resetCompilerWarmUpThreshold() {
   jit::DefaultJitOptions defaultValues;
   normalIonWarmUpThreshold = defaultValues.normalIonWarmUpThreshold;
-
-  // Undo eager compilation
-  if (eagerCompilation) {
-    eagerCompilation = false;
-    baselineWarmUpThreshold = defaultValues.baselineWarmUpThreshold;
-  }
 }
 
 }  // namespace jit
 }  // namespace js
--- a/js/src/jit/JitOptions.h
+++ b/js/src/jit/JitOptions.h
@@ -60,17 +60,16 @@ struct DefaultJitOptions {
   bool disableInstructionReordering;
   bool disableRangeAnalysis;
   bool disableRecoverIns;
   bool disableScalarReplacement;
   bool disableCacheIR;
   bool disableCacheIRBinaryArith;
   bool disableSincos;
   bool disableSink;
-  bool eagerCompilation;
   bool forceInlineCaches;
   bool fullDebugChecks;
   bool limitScriptSize;
   bool osr;
   bool wasmFoldOffsets;
   bool wasmDelayTier2;
 #ifdef JS_TRACE_LOGGING
   bool enableTraceLogger;
@@ -109,20 +108,24 @@ struct DefaultJitOptions {
   bool spectreValueMasking;
   bool spectreJitToCxxCalls;
 
   // The options below affect the rest of the VM, and not just the JIT.
   bool disableUnboxedObjects;
 
   DefaultJitOptions();
   bool isSmallFunction(JSScript* script) const;
-  void setEagerCompilation();
+  void setEagerIonCompilation();
   void setCompilerWarmUpThreshold(uint32_t warmUpThreshold);
   void resetCompilerWarmUpThreshold();
   void enableGvn(bool val);
+
+  bool eagerIonCompilation() const {
+    return normalIonWarmUpThreshold == 0;
+  }
 };
 
 extern DefaultJitOptions JitOptions;
 
 }  // namespace jit
 }  // namespace js
 
 #endif /* jit_JitOptions_h */
--- a/js/src/jit/Lowering.cpp
+++ b/js/src/jit/Lowering.cpp
@@ -537,17 +537,17 @@ void LIRGenerator::visitEncodeSnapshot(M
   assignSnapshot(lir, Bailout_Inevitable);
   add(lir, mir);
 }
 
 void LIRGenerator::visitAssertFloat32(MAssertFloat32* assertion) {
   MIRType type = assertion->input()->type();
   DebugOnly<bool> checkIsFloat32 = assertion->mustBeFloat32();
 
-  if (type != MIRType::Value && !JitOptions.eagerCompilation) {
+  if (type != MIRType::Value && !JitOptions.eagerIonCompilation()) {
     MOZ_ASSERT_IF(checkIsFloat32, type == MIRType::Float32);
     MOZ_ASSERT_IF(!checkIsFloat32, type != MIRType::Float32);
   }
 }
 
 void LIRGenerator::visitAssertRecoveredOnBailout(
     MAssertRecoveredOnBailout* assertion) {
   MOZ_CRASH("AssertRecoveredOnBailout nodes are always recovered on bailouts.");
--- a/js/src/jsapi.cpp
+++ b/js/src/jsapi.cpp
@@ -5476,19 +5476,16 @@ JS_PUBLIC_API void JS_SetGlobalJitCompil
       jit::JitOptions.baselineWarmUpThreshold = value;
       break;
     case JSJITCOMPILER_ION_WARMUP_TRIGGER:
       if (value == uint32_t(-1)) {
         jit::JitOptions.resetCompilerWarmUpThreshold();
         break;
       }
       jit::JitOptions.setCompilerWarmUpThreshold(value);
-      if (value == 0) {
-        jit::JitOptions.setEagerCompilation();
-      }
       break;
     case JSJITCOMPILER_ION_GVN_ENABLE:
       if (value == 0) {
         jit::JitOptions.enableGvn(false);
         JitSpew(js::jit::JitSpew_IonScripts, "Disable ion's GVN");
       } else {
         jit::JitOptions.enableGvn(true);
         JitSpew(js::jit::JitSpew_IonScripts, "Enable ion's GVN");
--- a/js/src/shell/js.cpp
+++ b/js/src/shell/js.cpp
@@ -6558,17 +6558,17 @@ static bool IsLatin1(JSContext* cx, unsi
 }
 
 static bool UnboxedObjectsEnabled(JSContext* cx, unsigned argc, Value* vp) {
   // Note: this also returns |false| if we're using --ion-eager or if the
   // JITs are disabled, since that affects how unboxed objects are used.
 
   CallArgs args = CallArgsFromVp(argc, vp);
   args.rval().setBoolean(!jit::JitOptions.disableUnboxedObjects &&
-                         !jit::JitOptions.eagerCompilation &&
+                         !jit::JitOptions.eagerIonCompilation() &&
                          jit::IsIonEnabled(cx));
   return true;
 }
 
 static bool IsUnboxedObject(JSContext* cx, unsigned argc, Value* vp) {
   CallArgs args = CallArgsFromVp(argc, vp);
   args.rval().setBoolean(args.get(0).isObject() &&
                          args[0].toObject().is<UnboxedPlainObject>());
@@ -10394,17 +10394,17 @@ static bool SetContextOptions(JSContext*
   if (const char* str = op.getStringOption("ion-regalloc")) {
     jit::JitOptions.forcedRegisterAllocator = jit::LookupRegisterAllocator(str);
     if (!jit::JitOptions.forcedRegisterAllocator.isSome()) {
       return OptionFailure("ion-regalloc", str);
     }
   }
 
   if (op.getBoolOption("ion-eager")) {
-    jit::JitOptions.setEagerCompilation();
+    jit::JitOptions.setEagerIonCompilation();
   }
 
   offthreadCompilation = true;
   if (const char* str = op.getStringOption("ion-offthread-compile")) {
     if (strcmp(str, "off") == 0) {
       offthreadCompilation = false;
     } else if (strcmp(str, "on") != 0) {
       return OptionFailure("ion-offthread-compile", str);