[JSC] OSR exit to LLInt is broken on MIPS
authorticaiolima@gmail.com <ticaiolima@gmail.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 20 Nov 2019 23:12:41 +0000 (23:12 +0000)
committerticaiolima@gmail.com <ticaiolima@gmail.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 20 Nov 2019 23:12:41 +0000 (23:12 +0000)
https://bugs.webkit.org/show_bug.cgi?id=203737

Reviewed by Yusuke Suzuki.

JSTests:

Unskipping broken tests due to OSR to LLInt bug.

* microbenchmarks/call-spread-call.js:
* microbenchmarks/throw.js:
* stress/allocation-sinking-hints-are-valid-ssa-2.js:
* stress/allocation-sinking-hints-are-valid-ssa.js:
* stress/arith-profile-for-negate-can-see-non-number-due-to-dfg-osr-exit-profiling.js:
* stress/arrowfunction-lexical-bind-supercall-4.js:
* stress/arrowfunction-tdz-3.js:
* stress/function-constructor-semantics.js:
* stress/global-import-function-should-return-a-promise-when-clearing-exceptions.js:
* stress/stress-cleared-calllinkinfo.js:
* stress/typedarray-configure-index.js:
* stress/v8-deltablue-strict.js:

PerformanceTests/SunSpider:

* tests/v8-v6/v8-deltablue.js:

Source/JavaScriptCore:

This patch is adjusting the OSR to LLInt mechanism to MIPS. When we
use PIC on this architecture, we need to properly configure `$gp`
at some places to be able to access global variables. This is required
on LLInt to access Global Offset Table (got). According to MIPS ABI,
the `$gp` can be recalculated during function prologue using caller
register `$t9`. We also emit such instructions (we can see this as
`OFFLINE_ASM_CPLOAD` macro) immediately after a non-local label on
LLInt. With the introduction of OSR to LLInt mechanism, we now have
return location labels that are reached from `ret` LLInt instructions.
Such return locations are used to properly return to LLInt execution
whenever an OSR exits from inlined call on DFG or FTL to LLInt. When
OSR is materializing LLInt stack frames for inlined functions (or
accessors), it sets return address to its return location label.
This means that for such labels, we need to adjust `$gp`
using `$ra` instead of `$t9`, given that LLInt `ret` operation uses
`jr $ra` to jump the execution to there.
To implement this, we changed `mipsAddPICCode` to emit code
using the correct register required to recalculate `$gp`.
We also changed `callTargetFunction` to use the stubs as return
location points, since the declaration of global labels will emmit
`OFFLINE_ASM_CPLOAD($ra)` and we don't want to execute it during
normal LLInt execution.

* llint/LowLevelInterpreter.asm:
* offlineasm/mips.rb:

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@252713 268f45cc-cd09-0410-ab3c-d52691b4dbfc

18 files changed:
JSTests/ChangeLog
JSTests/microbenchmarks/call-spread-call.js
JSTests/microbenchmarks/throw.js
JSTests/stress/allocation-sinking-hints-are-valid-ssa-2.js
JSTests/stress/allocation-sinking-hints-are-valid-ssa.js
JSTests/stress/arith-profile-for-negate-can-see-non-number-due-to-dfg-osr-exit-profiling.js
JSTests/stress/arrowfunction-lexical-bind-supercall-4.js
JSTests/stress/arrowfunction-tdz-3.js
JSTests/stress/function-constructor-semantics.js
JSTests/stress/global-import-function-should-return-a-promise-when-clearing-exceptions.js
JSTests/stress/stress-cleared-calllinkinfo.js
JSTests/stress/typedarray-configure-index.js
JSTests/stress/v8-deltablue-strict.js
PerformanceTests/SunSpider/ChangeLog
PerformanceTests/SunSpider/tests/v8-v6/v8-deltablue.js
Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/llint/LowLevelInterpreter.asm
Source/JavaScriptCore/offlineasm/mips.rb

index 7a065b1..923c9e1 100644 (file)
@@ -1,3 +1,25 @@
+2019-11-20  Caio Lima  <ticaiolima@gmail.com>
+
+        [JSC] OSR exit to LLInt is broken on MIPS
+        https://bugs.webkit.org/show_bug.cgi?id=203737
+
+        Reviewed by Yusuke Suzuki.
+
+        Unskipping broken tests due to OSR to LLInt bug.
+
+        * microbenchmarks/call-spread-call.js:
+        * microbenchmarks/throw.js:
+        * stress/allocation-sinking-hints-are-valid-ssa-2.js:
+        * stress/allocation-sinking-hints-are-valid-ssa.js:
+        * stress/arith-profile-for-negate-can-see-non-number-due-to-dfg-osr-exit-profiling.js:
+        * stress/arrowfunction-lexical-bind-supercall-4.js:
+        * stress/arrowfunction-tdz-3.js:
+        * stress/function-constructor-semantics.js:
+        * stress/global-import-function-should-return-a-promise-when-clearing-exceptions.js:
+        * stress/stress-cleared-calllinkinfo.js:
+        * stress/typedarray-configure-index.js:
+        * stress/v8-deltablue-strict.js:
+
 2019-11-20  Mark Lam  <mark.lam@apple.com>
 
         Flaky JSC test: stress/stack-overflow-in-yarr-byteCompile.js.dfg-eager.
index 923fcd4..d2ae2e6 100644 (file)
@@ -1,4 +1,4 @@
-//@ skip if $model == "Apple Watch Series 3" or $architecture == "mips" # added by mark-jsc-stress-test.py
+//@ skip if $model == "Apple Watch Series 3" # added by mark-jsc-stress-test.py
 
 Function.prototype.c = Function.prototype.call;
 
index c758e46..46078c1 100644 (file)
@@ -1,4 +1,4 @@
-//@ skip if $model == "Apple Watch Series 3" or $architecture == "mips" # added by mark-jsc-stress-test.py
+//@ skip if $model == "Apple Watch Series 3" # added by mark-jsc-stress-test.py
 function foo()
 {
     throw new Error();
index 12cc36c..8738443 100644 (file)
@@ -1,5 +1,3 @@
-//@ skip if $architecture == "mips"
-
 function main() {
     const arr = [0];
     function executor(resolve, ...reject) {
index 12cc36c..8738443 100644 (file)
@@ -1,5 +1,3 @@
-//@ skip if $architecture == "mips"
-
 function main() {
     const arr = [0];
     function executor(resolve, ...reject) {
index ea31d64..fb887c2 100644 (file)
@@ -1,5 +1,3 @@
-//@ skip if $architecture == "mips"
-
 var testCase = function (actual, expected, message) {
     if (actual !== expected) {
         throw message + ". Expected '" + expected + "', but was '" + actual + "'";
index 178cc16..ab98309 100644 (file)
@@ -1,5 +1,3 @@
-//@ skip if $architecture == "mips"
-
 var testCase = function (actual, expected, message) {
   if (actual !== expected) {
     throw message + ". Expected '" + expected + "', but was '" + actual + "'";
index ad196e9..8b35be4 100644 (file)
@@ -1,5 +1,3 @@
-//@ skip if $architecture == "mips"
-
 function assert(b) {
     if (!b)
         throw new Error("Bad");
index 6477085..5e52031 100644 (file)
@@ -1,5 +1,3 @@
-// Skip on mips until https://bugs.webkit.org/show_bug.cgi?id=203737 is fixed.
-//@ skip if $architecture == "mips"
 //@ requireOptions("--maxPerThreadStackUsage=300000", "--exceptionStackTraceLimit=0", "--defaultErrorStackTraceLimit=0")
 
 function bar(v) {
index 65ceec0..1b61d59 100644 (file)
@@ -1,4 +1,4 @@
-//@ if $architecture == "mips" then skip else slow! end
+//@ slow!
 function runNearStackLimit(f) {
     function t() {
         try {
index e5e15d0..e82de0b 100644 (file)
@@ -1,5 +1,3 @@
-//@ skip if $architecture == "mips"
-
 typedArrays = [Int8Array, Uint8Array, Uint8ClampedArray, Int16Array, Uint16Array, Int32Array, Uint32Array, Float32Array, Float64Array];
 
 function assert(cond) {
index e6edc7e..8badbe9 100644 (file)
@@ -1,5 +1,3 @@
-//@ skip if $architecture == "mips"
-
 "use strict";
 
 // Copyright 2008 the V8 project authors. All rights reserved.
index eecb5e9..ae37337 100644 (file)
@@ -1,3 +1,12 @@
+2019-11-20  Caio Lima  <ticaiolima@gmail.com>
+
+        [JSC] OSR exit to LLInt is broken on MIPS
+        https://bugs.webkit.org/show_bug.cgi?id=203737
+
+        Reviewed by Yusuke Suzuki.
+
+        * tests/v8-v6/v8-deltablue.js:
+
 2019-10-31  Caio Lima  <ticaiolima@gmail.com>
 
         Temporary skip broken tests on MIPS that is broken due to OSR exit to LLInt
index 577c55a..5c3e233 100644 (file)
@@ -1,5 +1,3 @@
-//@ skip if $architecture == "mips"
-
 // Copyright 2008 the V8 project authors. All rights reserved.
 // Copyright 1996 John Maloney and Mario Wolczko.
 
index 5cf00a1..2f78dff 100644 (file)
@@ -1,3 +1,36 @@
+2019-11-20  Caio Lima  <ticaiolima@gmail.com>
+
+        [JSC] OSR exit to LLInt is broken on MIPS
+        https://bugs.webkit.org/show_bug.cgi?id=203737
+
+        Reviewed by Yusuke Suzuki.
+
+        This patch is adjusting the OSR to LLInt mechanism to MIPS. When we
+        use PIC on this architecture, we need to properly configure `$gp`
+        at some places to be able to access global variables. This is required
+        on LLInt to access Global Offset Table (got). According to MIPS ABI,
+        the `$gp` can be recalculated during function prologue using caller
+        register `$t9`. We also emit such instructions (we can see this as 
+        `OFFLINE_ASM_CPLOAD` macro) immediately after a non-local label on
+        LLInt. With the introduction of OSR to LLInt mechanism, we now have
+        return location labels that are reached from `ret` LLInt instructions.
+        Such return locations are used to properly return to LLInt execution
+        whenever an OSR exits from inlined call on DFG or FTL to LLInt. When
+        OSR is materializing LLInt stack frames for inlined functions (or
+        accessors), it sets return address to its return location label.
+        This means that for such labels, we need to adjust `$gp`
+        using `$ra` instead of `$t9`, given that LLInt `ret` operation uses
+        `jr $ra` to jump the execution to there.
+        To implement this, we changed `mipsAddPICCode` to emit code
+        using the correct register required to recalculate `$gp`.
+        We also changed `callTargetFunction` to use the stubs as return
+        location points, since the declaration of global labels will emmit
+        `OFFLINE_ASM_CPLOAD($ra)` and we don't want to execute it during
+        normal LLInt execution.
+
+        * llint/LowLevelInterpreter.asm:
+        * offlineasm/mips.rb:
+
 2019-11-20  Robin Morisset  <rmorisset@apple.com>
 
         Fix load<16> on ARM64
index b2ab791..e1734dc 100644 (file)
@@ -994,9 +994,11 @@ macro callTargetFunction(opcodeName, size, opcodeStruct, dispatch, callee, callP
         call callee, callPtrTag
     end
 
-    if ARMv7
-        # Only required in ARMv7 since only here defineOSRExitReturnLabel
-        # inserts the global label words
+    if ARMv7 or MIPS
+        # It is required in ARMv7 and MIPs because global label definitions
+        # for those architectures generates a set of instructions
+        # that can clobber LLInt execution, resulting in unexpected
+        # crashes.
         restoreStackPointerAfterCall()
         dispatchAfterCall(size, opcodeStruct, dispatch)
     end
index 5a532d8..6324730 100644 (file)
@@ -99,6 +99,7 @@ MIPS_ZERO_REG = SpecialRegister.new("$zero")
 MIPS_GP_REG = SpecialRegister.new("$gp")
 MIPS_GPSAVE_REG = SpecialRegister.new("$s4")
 MIPS_CALL_REG = SpecialRegister.new("$t9")
+MIPS_RETURN_ADDRESS_REG = SpecialRegister.new("$ra")
 MIPS_TEMP_FPRS = [SpecialRegister.new("$f16")]
 MIPS_SCRATCH_FPR = SpecialRegister.new("$f18")
 
@@ -679,14 +680,15 @@ def mipsAddPICCode(list)
     list.each {
         | node |
         myList << node
-        if node.is_a? Label and node.name !~ /^.*_return_location(?:_(?:wide16|wide32))?$/
-            # Only generate a pichdr if the label is not a return location (OSR Exit) - a cpload 
-            # (generated by pichdr) after this type of label would cause the a gp register
-            # corruption. The cpload only sets gp correctly if t9 contains the address of the start
-            # of cpload, which is only the case when we do a `jr t9` (on a OSR Exit).
-            # This regular expression needs to stay in sync with the labels
-            # generated in macro defineOSRExitReturnLabel defined llint/LowLevelInterpreter.asm 
-            myList << Instruction.new(node.codeOrigin, "pichdr", [])
+        if node.is_a? Label
+            if node.name =~ /^.*_return_location(?:_(?:wide16|wide32))?$/
+                # We need to have a special case for return location labels because they are always
+                # reached from a `ret` instruction. In this case, we need to proper reconfigure `$gp`
+                # using `$ra` instead of using `$t9`.
+                myList << Instruction.new(node.codeOrigin, "pichdr", [MIPS_RETURN_ADDRESS_REG])
+            else
+                myList << Instruction.new(node.codeOrigin, "pichdr", [MIPS_CALL_REG])
+            end
         end
     }
     myList
@@ -1067,7 +1069,7 @@ class Instruction
         when "sltu", "sltub"
             $asm.puts "sltu #{operands[0].mipsOperand}, #{operands[1].mipsOperand}, #{operands[2].mipsOperand}"
         when "pichdr"
-            $asm.putStr("OFFLINE_ASM_CPLOAD(#{MIPS_CALL_REG.mipsOperand})")
+            $asm.putStr("OFFLINE_ASM_CPLOAD(#{operands[0].mipsOperand})")
         when "memfence"
             $asm.puts "sync"
         else