Attachment 371449 Details for Bug 198106 – Patch

[patch] Patch

bug-198106-20190605164616.patch (text/plain), 6.45 KB, created by Justin Michaud on 2019-06-05 16:46:17 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Justin Michaud

Created: 2019-06-05 16:46:17 PDT

Size: 6.45 KB

patch

obsolete

>Subversion Revision: 246077
>diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog
>index 7adba70160b2634d102c633a24401e406843c903..55bd2e2007e728ffd5aedcd5837915aad8f58200 100644
>--- a/Source/JavaScriptCore/ChangeLog
>+++ b/Source/JavaScriptCore/ChangeLog
>@@ -1,3 +1,15 @@
>+2019-06-05  Justin Michaud  <justin_michaud@apple.com>
>+
>+        WebAssembly: pow functions returns 0 when exponent 1.0 or -1.0
>+        https://bugs.webkit.org/show_bug.cgi?id=198106
>+
>+        Reviewed by Saam Barati.
>+
>+        Fix bug caused by using fcsel sX instead of fcsel dX on an f64 value in moveDoubleConditionally32.
>+
>+        * assembler/MacroAssemblerARM64.h:
>+        (JSC::MacroAssemblerARM64::moveDoubleConditionally32):
>+
> 2019-06-04  Tadeu Zagallo  <tzagallo@apple.com>
> 
>         Argument elimination should check transitive dependents for interference
>diff --git a/Source/JavaScriptCore/assembler/MacroAssemblerARM64.h b/Source/JavaScriptCore/assembler/MacroAssemblerARM64.h
>index eff8b71f39232277fbe3a6344869659374bf8621..4efd6f90d732da68f32345fa71682d82b86941f6 100644
>--- a/Source/JavaScriptCore/assembler/MacroAssemblerARM64.h
>+++ b/Source/JavaScriptCore/assembler/MacroAssemblerARM64.h
>@@ -2459,7 +2459,7 @@ public:
>     void moveDoubleConditionally32(RelationalCondition cond, RegisterID left, RegisterID right, FPRegisterID thenCase, FPRegisterID elseCase, FPRegisterID dest)
>     {
>         m_assembler.cmp<32>(left, right);
>-        m_assembler.fcsel<32>(dest, thenCase, elseCase, ARM64Condition(cond));
>+        m_assembler.fcsel<64>(dest, thenCase, elseCase, ARM64Condition(cond));
>     }
> 
>     void moveDoubleConditionally32(RelationalCondition cond, RegisterID left, TrustedImm32 right, FPRegisterID thenCase, FPRegisterID elseCase, FPRegisterID dest)
>diff --git a/Source/JavaScriptCore/assembler/testmasm.cpp b/Source/JavaScriptCore/assembler/testmasm.cpp
>index ac5c63291b3d8935b59b9fce40613754d9ddc1f1..cc25c94896ed37ce0f05773fd5b838e7e0e3950e 100644
>--- a/Source/JavaScriptCore/assembler/testmasm.cpp
>+++ b/Source/JavaScriptCore/assembler/testmasm.cpp
>@@ -935,6 +935,74 @@ void testByteSwap()
> #endif
> }
> 
>+void testMoveDoubleConditionally32()
>+{
>+#if CPU(X86_64) | CPU(ARM64)
>+    double arg1 = 0;
>+    double arg2 = 0;
>+    const double zero = -0;
>+
>+    const double chosenDouble = 6.00000059604644775390625;
>+    CHECK_EQ(static_cast<double>(static_cast<float>(chosenDouble)) == chosenDouble, false);
>+
>+    auto sel = compile([&] (CCallHelpers& jit) {
>+        jit.emitFunctionPrologue();
>+        jit.loadDouble(CCallHelpers::TrustedImmPtr(&zero), FPRInfo::returnValueFPR);
>+        jit.loadDouble(CCallHelpers::TrustedImmPtr(&arg1), FPRInfo::fpRegT1);
>+        jit.loadDouble(CCallHelpers::TrustedImmPtr(&arg2), FPRInfo::fpRegT2);
>+
>+        jit.move(MacroAssembler::TrustedImm32(-1), GPRInfo::regT0);
>+        jit.moveDoubleConditionally32(MacroAssembler::Equal, GPRInfo::regT0, GPRInfo::regT0, FPRInfo::fpRegT1, FPRInfo::fpRegT2, FPRInfo::returnValueFPR);
>+
>+        jit.emitFunctionEpilogue();
>+        jit.ret();
>+    });
>+
>+    arg1 = chosenDouble;
>+    arg2 = 43;
>+    CHECK_EQ(invoke<double>(sel), chosenDouble);
>+
>+    arg1 = 43;
>+    arg2 = chosenDouble;
>+    CHECK_EQ(invoke<double>(sel), 43.0);
>+
>+#endif
>+}
>+
>+void testMoveDoubleConditionally64()
>+{
>+#if CPU(X86_64) | CPU(ARM64)
>+    double arg1 = 0;
>+    double arg2 = 0;
>+    const double zero = -0;
>+
>+    const double chosenDouble = 6.00000059604644775390625;
>+    CHECK_EQ(static_cast<double>(static_cast<float>(chosenDouble)) == chosenDouble, false);
>+
>+    auto sel = compile([&] (CCallHelpers& jit) {
>+        jit.emitFunctionPrologue();
>+        jit.loadDouble(CCallHelpers::TrustedImmPtr(&zero), FPRInfo::returnValueFPR);
>+        jit.loadDouble(CCallHelpers::TrustedImmPtr(&arg1), FPRInfo::fpRegT1);
>+        jit.loadDouble(CCallHelpers::TrustedImmPtr(&arg2), FPRInfo::fpRegT2);
>+
>+        jit.move(MacroAssembler::TrustedImm64(-1), GPRInfo::regT0);
>+        jit.moveDoubleConditionally64(MacroAssembler::Equal, GPRInfo::regT0, GPRInfo::regT0, FPRInfo::fpRegT1, FPRInfo::fpRegT2, FPRInfo::returnValueFPR);
>+
>+        jit.emitFunctionEpilogue();
>+        jit.ret();
>+    });
>+
>+    arg1 = chosenDouble;
>+    arg2 = 43;
>+    CHECK_EQ(invoke<double>(sel), chosenDouble);
>+
>+    arg1 = 43;
>+    arg2 = chosenDouble;
>+    CHECK_EQ(invoke<double>(sel), 43.0);
>+
>+#endif
>+}
>+
> #define RUN(test) do {                          \
>         if (!shouldRun(#test))                  \
>             break;                              \
>@@ -1017,6 +1085,8 @@ void run(const char* filter)
> #endif // ENABLE(MASM_PROBE)
> 
>     RUN(testByteSwap());
>+    RUN(testMoveDoubleConditionally32());
>+    RUN(testMoveDoubleConditionally64());
> 
>     if (tasks.isEmpty())
>         usage();
>diff --git a/JSTests/ChangeLog b/JSTests/ChangeLog
>index 87b3b1924b383aab67020d4fc8998c2b00786810..189ecbd6daa2fb5c15be830536e594aced932251 100644
>--- a/JSTests/ChangeLog
>+++ b/JSTests/ChangeLog
>@@ -1,3 +1,14 @@
>+2019-06-05  Justin Michaud  <justin_michaud@apple.com>
>+
>+        WebAssembly: pow functions returns 0 when exponent 1.0 or -1.0
>+        https://bugs.webkit.org/show_bug.cgi?id=198106
>+
>+        Reviewed by Saam Barati.
>+
>+        * wasm/regress/selectf64.js: Added.
>+        * wasm/regress/selectf64.wasm: Added.
>+        * wasm/regress/selectf64.wat: Added.
>+
> 2019-06-04  Tadeu Zagallo  <tzagallo@apple.com>
> 
>         Argument elimination should check transitive dependents for interference
>diff --git a/JSTests/wasm/regress/selectf64.js b/JSTests/wasm/regress/selectf64.js
>new file mode 100644
>index 0000000000000000000000000000000000000000..dd496c989a5ae1fa3151eaf77b6d978d76013517
>--- /dev/null
>+++ b/JSTests/wasm/regress/selectf64.js
>@@ -0,0 +1,4 @@
>+import { test } from 'selectf64.wasm'
>+import * as assert from '../assert.js';
>+
>+assert.eq(test(), 43)
>diff --git a/JSTests/wasm/regress/selectf64.wasm b/JSTests/wasm/regress/selectf64.wasm
>new file mode 100644
>index 0000000000000000000000000000000000000000..30ffef581c020c6b000951839a53f6a57b588870
>GIT binary patch
>literal 59
>zcmZQbEY4+QU|?WmWlUgTtYKziWMJoDWGP84E@5Ecl3|o$Z~@8!fvW?A(ctRfSnpWp
>HCd~~1-%JQU
>
>literal 0
>HcmV?d00001
>
>diff --git a/JSTests/wasm/regress/selectf64.wat b/JSTests/wasm/regress/selectf64.wat
>new file mode 100644
>index 0000000000000000000000000000000000000000..489a5220288192c69de372b76a91cdc7b838be06
>--- /dev/null
>+++ b/JSTests/wasm/regress/selectf64.wat
>@@ -0,0 +1,9 @@
>+(module
>+  (func (export "test") (result f64)
>+    f64.const 42
>+    f64.const 43
>+    i32.const -1
>+    i32.const -2
>+    i32.eq
>+    select
>+))

Actions: View | Formatted Diff | Diff

Attachments on bug 198106: 370778 | 370780 | 370781 | 371421 | 371422 | 371423 | 371448 | 371449