[Merge chakra-core/ChakraCore@06c67e8493] [1.6>1.7] [MERGE #3426 @Cel…

…lule] WASM: uint64 -> double edge case Merge pull request #3426 from Cellule:wasm/uint64_double Fix uint64 conversion to double in edge cases on x64
nodejs · Aug 10, 2017 · b8690b4 · b8690b4
1 parent 402e13f
commit b8690b4
Show file tree

Hide file tree

Showing 12 changed files with 76 additions and 31 deletions.
diff --git a/deps/chakrashim/core/lib/Backend/IR.cpp b/deps/chakrashim/core/lib/Backend/IR.cpp
@@ -1994,8 +1994,10 @@ Instr::New(Js::OpCode opcode, Opnd *dstOpnd, Func *func)
     Instr * instr;
 
     instr = Instr::New(opcode, func);
-    instr->SetDst(dstOpnd);
-
+    if (dstOpnd)
+    {
+        instr->SetDst(dstOpnd);
+    }
     return instr;
 }
 

diff --git a/deps/chakrashim/core/lib/Backend/LowerMDShared.cpp b/deps/chakrashim/core/lib/Backend/LowerMDShared.cpp
@@ -7676,29 +7676,42 @@ LowererMD::EmitInt64toFloat(IR::Opnd *dst, IR::Opnd *src, IR::Instr *instr)
         dst = IR::RegOpnd::New(TyFloat64, this->m_func);
     }
 
-    instr->InsertBefore(IR::Instr::New(Js::OpCode::CVTSI2SD, dst, src, this->m_func));
-    if (src->IsUnsigned())
+    const auto insertLegalize = [instr](IR::Instr* newInstr)
     {
-        IR::RegOpnd * highestBitOpnd = IR::RegOpnd::New(TyInt64, this->m_func);
-        IR::Instr* instrNew = IR::Instr::New(Js::OpCode::SHR, highestBitOpnd, src,
-        IR::IntConstOpnd::New(63, TyInt8, this->m_func, true), this->m_func);
-        instr->InsertBefore(instrNew);
-        Legalize(instrNew);
-        IR::RegOpnd * baseOpnd = IR::RegOpnd::New(TyMachPtr, this->m_func);
-
-        instrNew = IR::Instr::New(Js::OpCode::MOV, baseOpnd, IR::AddrOpnd::New(m_func->GetThreadContextInfo()->GetUInt64ConvertConstAddr(),
-            IR::AddrOpndKindDynamicMisc, this->m_func), this->m_func);
-
-        instr->InsertBefore(instrNew);
+        instr->InsertBefore(newInstr);
+        Legalize(newInstr);
+    };
 
-        instrNew = IR::Instr::New(Js::OpCode::ADDSD, dst, dst, IR::IndirOpnd::New(baseOpnd,
-            highestBitOpnd, IndirScale8, TyFloat64, this->m_func), this->m_func);
-        instr->InsertBefore(instrNew);
+    if (src->IsUnsigned())
+    {
+        insertLegalize(IR::Instr::New(Js::OpCode::TEST, nullptr, src, src, m_func));
+        IR::LabelInstr* msbSetLabel = IR::LabelInstr::New(Js::OpCode::Label, m_func);
+        IR::LabelInstr* doneLabel = IR::LabelInstr::New(Js::OpCode::Label, m_func);
+        insertLegalize(IR::BranchInstr::New(Js::OpCode::JSB, msbSetLabel, m_func));
+        // MSB not set, simple case
+        insertLegalize(IR::Instr::New(Js::OpCode::CVTSI2SD, dst, src, m_func));
+        insertLegalize(IR::BranchInstr::New(Js::OpCode::JMP, doneLabel, m_func));
+        insertLegalize(msbSetLabel);
+
+        IR::RegOpnd* halfOpnd = IR::RegOpnd::New(TyInt64, m_func);
+        IR::RegOpnd* lsbOpnd = IR::RegOpnd::New(TyInt64, m_func);
+        m_lowerer->InsertMove(halfOpnd, src, instr);
+        m_lowerer->InsertMove(lsbOpnd, src, instr);
+        insertLegalize(IR::Instr::New(Js::OpCode::SHR, halfOpnd, halfOpnd, IR::IntConstOpnd::New(1, TyInt8, m_func), m_func));
+        insertLegalize(IR::Instr::New(Js::OpCode::AND, lsbOpnd, lsbOpnd, IR::Int64ConstOpnd::New(1, TyInt64, m_func), m_func));
+        insertLegalize(IR::Instr::New(Js::OpCode::OR, halfOpnd, halfOpnd, lsbOpnd, m_func));
+        insertLegalize(IR::Instr::New(Js::OpCode::CVTSI2SD, dst, halfOpnd, m_func));
+        insertLegalize(IR::Instr::New(Js::OpCode::ADDSD, dst, dst, dst, m_func));
+        insertLegalize(doneLabel);
+    }
+    else
+    {
+        insertLegalize(IR::Instr::New(Js::OpCode::CVTSI2SD, dst, src, m_func));
     }
 
     if (origDst)
     {
-        instr->InsertBefore(IR::Instr::New(Js::OpCode::CVTSD2SS, origDst, dst, this->m_func));
+        insertLegalize(IR::Instr::New(Js::OpCode::CVTSD2SS, origDst, dst, m_func));
     }
 #endif
 }

diff --git a/deps/chakrashim/core/lib/Common/Common/NumberUtilities.cpp b/deps/chakrashim/core/lib/Common/Common/NumberUtilities.cpp
@@ -89,7 +89,6 @@ namespace Js
     __declspec(align(16)) double const NumberConstants::UIntConvertConst[2] = { 0, 4294967296.000000 };
     __declspec(align(16)) float const NumberConstants::MaskNegFloat[] = { -0.0f, -0.0f, -0.0f, -0.0f };
     __declspec(align(16)) double const NumberConstants::MaskNegDouble[] = { -0.0, -0.0 };
-    __declspec(align(16)) uint64 const NumberConstants::UInt64ConvertConst[2] = { 0, 0x43f0000000000000 };
 
 
     bool NumberUtilities::IsDigit(int ch)

diff --git a/deps/chakrashim/core/lib/Common/Common/NumberUtilities.h b/deps/chakrashim/core/lib/Common/Common/NumberUtilities.h
@@ -68,7 +68,6 @@ namespace Js
         static const BYTE SgnFloatBitCst[];
         static const BYTE SgnDoubleBitCst[];
         static double const UIntConvertConst[];
-        static uint64 const UInt64ConvertConst[];
 
         static double const MaskNegDouble[];
         static float const MaskNegFloat[];

diff --git a/deps/chakrashim/core/lib/Runtime/Base/ThreadContextInfo.cpp b/deps/chakrashim/core/lib/Runtime/Base/ThreadContextInfo.cpp
@@ -78,12 +78,6 @@ ThreadContextInfo::GetUIntConvertConstAddr() const
     return SHIFT_ADDR(this, &Js::JavascriptNumber::UIntConvertConst);
 }
 
-intptr_t
-ThreadContextInfo::GetUInt64ConvertConstAddr() const
-{
-    return SHIFT_ADDR(this, &Js::JavascriptNumber::UInt64ConvertConst);
-}
-
 intptr_t
 ThreadContextInfo::GetUint8ClampedArraySetItemAddr() const
 {

diff --git a/deps/chakrashim/core/lib/Runtime/Base/ThreadContextInfo.h b/deps/chakrashim/core/lib/Runtime/Base/ThreadContextInfo.h
@@ -51,7 +51,6 @@ class ThreadContextInfo
     intptr_t GetDoubleTwoTo31Addr() const;
 
     intptr_t GetUIntConvertConstAddr() const;
-    intptr_t GetUInt64ConvertConstAddr() const;
     intptr_t GetUint8ClampedArraySetItemAddr() const;
     intptr_t GetConstructorCacheDefaultInstanceAddr() const;
     intptr_t GetJavascriptObjectNewInstanceAddr() const;

diff --git a/deps/chakrashim/core/lib/Runtime/Language/Chakra.Runtime.Language.vcxproj b/deps/chakrashim/core/lib/Runtime/Language/Chakra.Runtime.Language.vcxproj
@@ -257,6 +257,9 @@
     <ClInclude Include="WebAssemblySource.h" />
   </ItemGroup>
   <ItemGroup>
+    <MASM Include="$(MSBuildThisFileDirectory)amd64\JavascriptConversionA.asm">
+      <ExcludedFromBuild Condition="'$(Platform)'!='x64'">true</ExcludedFromBuild>
+    </MASM>
     <MASM Include="$(MSBuildThisFileDirectory)amd64\JavascriptOperatorsA.asm">
       <ExcludedFromBuild Condition="'$(Platform)'!='x64'">true</ExcludedFromBuild>
     </MASM>

diff --git a/deps/chakrashim/core/lib/Runtime/Language/Chakra.Runtime.Language.vcxproj.filters b/deps/chakrashim/core/lib/Runtime/Language/Chakra.Runtime.Language.vcxproj.filters
@@ -157,6 +157,9 @@
     <MASM Include="$(MSBuildThisFileDirectory)amd64\JavascriptOperatorsA.asm">
       <Filter>amd64</Filter>
     </MASM>
+    <MASM Include="$(MSBuildThisFileDirectory)amd64\JavascriptConversionA.asm">
+      <Filter>amd64</Filter>
+    </MASM>
   </ItemGroup>
   <ItemGroup>
     <None Include="AsmJsEncoder.inl" />

diff --git a/deps/chakrashim/core/lib/Runtime/Language/JavascriptConversion.cpp b/deps/chakrashim/core/lib/Runtime/Language/JavascriptConversion.cpp
@@ -1544,10 +1544,13 @@ namespace Js
         return static_cast<double>(aValue);
     }
 
+    // Windows x64 version implemented in masm to work around precision limitation
+#if !defined(_WIN32 ) || !defined(_M_X64)
     double JavascriptConversion::ULongToDouble(unsigned __int64 aValue)
     {
         return static_cast<double>(aValue);
     }
+#endif
 
     float JavascriptConversion::LongToFloat(__int64 aValue)
     {

diff --git a/deps/chakrashim/core/lib/Runtime/Language/amd64/JavascriptConversionA.asm b/deps/chakrashim/core/lib/Runtime/Language/amd64/JavascriptConversionA.asm
@@ -0,0 +1,30 @@
+;-------------------------------------------------------------------------------------------------------
+; Copyright (C) Microsoft. All rights reserved.
+; Licensed under the MIT license. See LICENSE.txt file in the project root for full license information.
+;-------------------------------------------------------------------------------------------------------
+include ksamd64.inc
+
+        _TEXT SEGMENT
+
+align 16
+?ULongToDouble@JavascriptConversion@Js@@SAN_K@Z PROC FRAME
+        .endprolog
+
+        test rcx, rcx
+        js msbSet
+        cvtsi2sd xmm0, rcx
+        jmp doneULongToDouble
+    msbSet:
+        mov rdx, rcx
+        and rcx, 1 ; Save lsb
+        shr rdx, 1 ; divide by 2
+        or rcx, rdx ; put back lsb if it was set
+        cvtsi2sd xmm0, rcx ; do conversion
+        addsd xmm0, xmm0 ; xmm0 * 2
+    doneULongToDouble:
+        ret
+
+?ULongToDouble@JavascriptConversion@Js@@SAN_K@Z ENDP
+
+       _TEXT ENDS
+        end
diff --git a/deps/chakrashim/core/test/WasmSpec/baselines/conversions.baseline b/deps/chakrashim/core/test/WasmSpec/baselines/conversions.baseline
@@ -1 +1 @@
-396/396 tests passed.
+398/398 tests passed.
diff --git a/deps/chakrashim/core/test/WasmSpec/testsuite/core/conversions.wast b/deps/chakrashim/core/test/WasmSpec/testsuite/core/conversions.wast
@@ -321,8 +321,8 @@
 (assert_return (invoke "f64.convert_u_i64" (i64.const -9223372036854775808)) (f64.const 9223372036854775808))
 (assert_return (invoke "f64.convert_u_i64" (i64.const 0xffffffffffffffff)) (f64.const 18446744073709551616.0))
 (assert_return (invoke "f64.convert_u_i64" (i64.const 0x8000000000000400)) (f64.const 0x1.0000000000000p+63))
-;;(assert_return (invoke "f64.convert_u_i64" (i64.const 0x8000000000000401)) (f64.const 0x1.0000000000001p+63))
-;;(assert_return (invoke "f64.convert_u_i64" (i64.const 0x8000000000000402)) (f64.const 0x1.0000000000001p+63))
+(assert_return (invoke "f64.convert_u_i64" (i64.const 0x8000000000000401)) (f64.const 0x1.0000000000001p+63))
+(assert_return (invoke "f64.convert_u_i64" (i64.const 0x8000000000000402)) (f64.const 0x1.0000000000001p+63))
 (assert_return (invoke "f64.convert_u_i64" (i64.const 0xfffffffffffff400)) (f64.const 0x1.ffffffffffffep+63))
 (assert_return (invoke "f64.convert_u_i64" (i64.const 0xfffffffffffff401)) (f64.const 0x1.fffffffffffffp+63))
 (assert_return (invoke "f64.convert_u_i64" (i64.const 0xfffffffffffff402)) (f64.const 0x1.fffffffffffffp+63))