Przeglądaj źródła

fix bugs with non-canonical NaNs in asm.js

In asm.js, we don't canonicalize NaNs. We don't have vars in asm.js, so this is
ok. But we DO need to make sure to check for NaN if we are creating a
JavascriptNumber from it.
However, we were not doing this, which was an issue for argouts in asm.js->js
calls as well as return values from asm.js.
It was also an issue in one helper call on x64, where we were converting to var
before making the call.
This was being done because there was no support for x64 calling convention.
But with asm.js, I implemented that support, so I updated the helper call to
use the standard API. Other places were also hacking around this limitation, so
I updated them as well.
Michael Holman 10 lat temu
rodzic
commit
f8f3236ab5

+ 0 - 17
lib/Backend/Lower.cpp

@@ -8126,14 +8126,7 @@ Lowerer::LowerStElemI(IR::Instr * instr, Js::PropertyOperationFlags flags, bool
 
     if (srcType == TyFloat64)
     {
-        // We don't support the X64 floating-point calling convention. So put this parameter on the end
-        // and save directly to the stack slot.
-#if _M_X64
-        IR::Opnd *argOpnd = IR::SymOpnd::New(m_func->m_symTable->GetArgSlotSym(5), TyFloat64, m_func);
-        m_lowererMD.CreateAssign(argOpnd, src1, instr);
-#else
         m_lowererMD.LoadDoubleHelperArgument(instr, src1);
-#endif
     }
     m_lowererMD.LoadHelperArgument(instr,
         IR::IntConstOpnd::New(static_cast<IntConstType>(flags), IRType::TyInt32, m_func, true));
@@ -10327,17 +10320,7 @@ Lowerer::GenerateHelperToArrayPushFastPath(IR::Instr * instr, IR::LabelInstr * b
         Assert(arrayHelperOpnd->GetValueType().IsLikelyNativeFloatArray());
         helperMethod = IR::HelperArray_NativeFloatPush;
 
-    //Currently, X64 floating-point calling convention is not supported. Hence store the
-    // float value explicitly in RegXMM2 (RegXMM0 and RegXMM1 will be filled with ScriptContext and Var respectively)
-#if _M_X64
-        IR::RegOpnd* regXMM2 = IR::RegOpnd::New(nullptr, (RegNum)RegXMM2, TyMachDouble, this->m_func);
-        regXMM2->m_isCallArg = true;
-        IR::Instr * movInstr = IR::Instr::New(Js::OpCode::MOVSD, regXMM2, elementHelperOpnd, this->m_func);
-        instr->InsertBefore(movInstr);
-#else
         m_lowererMD.LoadDoubleHelperArgument(instr, elementHelperOpnd);
-#endif
-
     }
     else
     {

+ 62 - 46
lib/Backend/LowerMDShared.cpp

@@ -5972,11 +5972,57 @@ LowererMD::SaveDoubleToVar(IR::RegOpnd * dstOpnd, IR::RegOpnd *opndFloat, IR::In
 #else
 
     // s1 = MOVD opndFloat
+    IR::RegOpnd *s1 = IR::RegOpnd::New(TyMachReg, m_func);
+    IR::Instr *movd = IR::Instr::New(Js::OpCode::MOVD, s1, opndFloat, m_func);
+    instrInsert->InsertBefore(movd);
+
+    if (m_func->GetJnFunction()->GetIsAsmjsMode())
+    {
+        // s1 = MOVD src
+        // tmp = NOT s1
+        // tmp = AND tmp, 0x7FF0000000000000ull
+        // test tmp, tmp
+        // je helper
+        // jmp done
+        // helper:
+        // tmp2 = AND s1, 0x000FFFFFFFFFFFFFull
+        // test tmp2, tmp2
+        // je done
+        // s1 = JavascriptNumber::k_Nan
+        // done:
+
+        IR::RegOpnd *tmp = IR::RegOpnd::New(TyMachReg, m_func);
+        IR::Instr * newInstr = IR::Instr::New(Js::OpCode::NOT, tmp, s1, m_func);
+        instrInsert->InsertBefore(newInstr);
+        LowererMD::MakeDstEquSrc1(newInstr);
+
+        newInstr = IR::Instr::New(Js::OpCode::AND, tmp, tmp, IR::AddrOpnd::New((Js::Var)0x7FF0000000000000, IR::AddrOpndKindConstantVar, m_func, true), m_func);
+        instrInsert->InsertBefore(newInstr);
+        LowererMD::Legalize(newInstr);
+
+        IR::LabelInstr* helper = Lowerer::InsertLabel(true, instrInsert);
+
+        Lowerer::InsertTestBranch(tmp, tmp, Js::OpCode::BrEq_A, helper, helper);
+
+        IR::LabelInstr* done = Lowerer::InsertLabel(isHelper, instrInsert);
+
+        Lowerer::InsertBranch(Js::OpCode::Br, done, helper);
+
+        IR::RegOpnd *tmp2 = IR::RegOpnd::New(TyMachReg, m_func);
+
+        newInstr = IR::Instr::New(Js::OpCode::AND, tmp2, s1, IR::AddrOpnd::New((Js::Var)0x000FFFFFFFFFFFFFull, IR::AddrOpndKindConstantVar, m_func, true), m_func);
+        done->InsertBefore(newInstr);
+        LowererMD::Legalize(newInstr);
+
+        Lowerer::InsertTestBranch(tmp2, tmp2, Js::OpCode::BrEq_A, done, done);
+
+        IR::Opnd * opndNaN = IR::AddrOpnd::New((Js::Var)Js::JavascriptNumber::k_Nan, IR::AddrOpndKindConstantVar, m_func, true);
+        Lowerer::InsertMove(s1, opndNaN, done);
+    }
+
     // s1 = XOR s1, FloatTag_Value
     // dst = s1
-
-    IR::RegOpnd *s1 = IR::RegOpnd::New(TyMachReg, this->m_func);
-    IR::Instr *movd = IR::Instr::New(Js::OpCode::MOVD, s1, opndFloat, this->m_func);
+    
     IR::Instr *setTag = IR::Instr::New(Js::OpCode::XOR,
                                        s1,
                                        s1,
@@ -5987,7 +6033,6 @@ LowererMD::SaveDoubleToVar(IR::RegOpnd * dstOpnd, IR::RegOpnd *opndFloat, IR::In
                                        this->m_func);
     IR::Instr *movDst = IR::Instr::New(Js::OpCode::MOV, dstOpnd, s1, this->m_func);
 
-    instrInsert->InsertBefore(movd);
     instrInsert->InsertBefore(setTag);
     instrInsert->InsertBefore(movDst);
     LowererMD::Legalize(setTag);
@@ -7705,6 +7750,11 @@ LowererMD::InsertConvertFloat64ToInt32(const RoundMode roundMode, IR::Opnd *cons
 void
 LowererMD::EmitFloatToInt(IR::Opnd *dst, IR::Opnd *src, IR::Instr *instrInsert)
 {
+#ifdef _M_IX86
+    // We should only generate this if sse2 is available
+    Assert(AutoSystemInfo::Data.SSE2Available());
+#endif
+
     IR::LabelInstr *labelDone = IR::LabelInstr::New(Js::OpCode::Label, this->m_func);
     IR::LabelInstr *labelHelper = IR::LabelInstr::New(Js::OpCode::Label, this->m_func, true);
     IR::Instr *instr;
@@ -7714,37 +7764,20 @@ LowererMD::EmitFloatToInt(IR::Opnd *dst, IR::Opnd *src, IR::Instr *instrInsert)
     // $Helper
     instrInsert->InsertBefore(labelHelper);
 
-#ifdef _M_X64
-    // On x64, we can simply pass the var, this way we don't have to worry having to
-    // pass a double in a param reg
-
-    // s1 = MOVD src
-    IR::RegOpnd *s1 = IR::RegOpnd::New(TyMachReg, this->m_func);
-    instr = IR::Instr::New(Js::OpCode::MOVD, s1, src, this->m_func);
-    instrInsert->InsertBefore(instr);
-
-    // s1 = XOR s1, FloatTag_Value
-    instr = IR::Instr::New(Js::OpCode::XOR, s1, s1,
-                           IR::AddrOpnd::New((Js::Var)Js::FloatTag_Value, IR::AddrOpndKindConstantVar, this->m_func, /* dontEncode = */ true),
-                           this->m_func);
-    instrInsert->InsertBefore(instr);
-    LowererMD::Legalize(instr);
-
-    // dst = ToInt32_Full(s1, scriptContext);
-    m_lowerer->LoadScriptContext(instrInsert);
-    LoadHelperArgument(instrInsert, s1);
+    IR::Opnd * arg = src;
+    if (src->IsFloat32())
+    {
+        arg = IR::RegOpnd::New(TyFloat64, m_func);
 
-    instr = IR::Instr::New(Js::OpCode::CALL, dst, this->m_func);
-    instrInsert->InsertBefore(instr);
-    this->ChangeToHelperCall(instr, IR::HelperConv_ToInt32_Full);
-#else
+        EmitFloat32ToFloat64(arg, src, instrInsert);
+    }
     // dst = ToInt32Core(src);
-    LoadDoubleHelperArgument(instrInsert, src);
+    LoadDoubleHelperArgument(instrInsert, arg);
 
     instr = IR::Instr::New(Js::OpCode::CALL, dst, this->m_func);
     instrInsert->InsertBefore(instr);
     this->ChangeToHelperCall(instr, IR::HelperConv_ToInt32Core);
-#endif
+
     // $Done
     instrInsert->InsertBefore(labelDone);
 }
@@ -9018,28 +9051,11 @@ IR::Opnd* LowererMD::IsOpndNegZero(IR::Opnd* opnd, IR::Instr* instr)
 {
     IR::Opnd * isNegZero = IR::RegOpnd::New(TyInt32, this->m_func);
 
-#if defined(_M_IX86)
     LoadDoubleHelperArgument(instr, opnd);
     IR::Instr * helperCallInstr = IR::Instr::New(Js::OpCode::CALL, isNegZero, this->m_func);
     instr->InsertBefore(helperCallInstr);
     this->ChangeToHelperCall(helperCallInstr, IR::HelperIsNegZero);
 
-#else
-    IR::RegOpnd* regXMM0 = IR::RegOpnd::New(nullptr, (RegNum)FIRST_FLOAT_ARG_REG, TyMachDouble, this->m_func);
-    regXMM0->m_isCallArg = true;
-    IR::Instr * movInstr = IR::Instr::New(Js::OpCode::MOVSD, regXMM0, opnd, this->m_func);
-    instr->InsertBefore(movInstr);
-
-    IR::RegOpnd* reg1 = IR::RegOpnd::New(TyMachReg, this->m_func);
-    IR::AddrOpnd* helperAddr = IR::AddrOpnd::New((Js::Var)IR::GetMethodOriginalAddress(IR::HelperIsNegZero), IR::AddrOpndKind::AddrOpndKindDynamicMisc, this->m_func);
-    IR::Instr* mov = IR::Instr::New(Js::OpCode::MOV, reg1, helperAddr, this->m_func);
-    instr->InsertBefore(mov);
-
-    IR::Instr *helperCallInstr = IR::Instr::New(Js::OpCode::CALL, isNegZero, reg1, this->m_func);
-    instr->InsertBefore(helperCallInstr);
-
-#endif
-
     return isNegZero;
 }
 

+ 3 - 3
lib/Runtime/Language/AsmJSModule.cpp

@@ -2221,10 +2221,10 @@ namespace Js
                 switch (asmSlot->varType)
                 {
                 case AsmJsVarType::Double:
-                    value = JavascriptNumber::New(asmDoubleVars[asmSlot->location], scriptContext);
+                    value = JavascriptNumber::NewWithCheck(asmDoubleVars[asmSlot->location], scriptContext);
                     break;
                 case AsmJsVarType::Float:
-                    value = JavascriptNumber::New(asmFloatVars[asmSlot->location], scriptContext);
+                    value = JavascriptNumber::NewWithCheck(asmFloatVars[asmSlot->location], scriptContext);
                     break;
                 case AsmJsVarType::Int:
                     value = JavascriptNumber::ToVar(asmIntVars[asmSlot->location], scriptContext);
@@ -2272,7 +2272,7 @@ namespace Js
                 value = asmFuncs[asmSlot->location];
                 break;
             case AsmJsSymbol::MathConstant:
-                value = JavascriptNumber::New(asmSlot->mathConstVal, scriptContext);
+                value = JavascriptNumber::NewWithCheck(asmSlot->mathConstVal, scriptContext);
                 break;
             case AsmJsSymbol::ArrayView:
             {

+ 4 - 4
lib/Runtime/Language/AsmJSUtils.cpp

@@ -327,11 +327,11 @@ namespace Js
             break;
         }
         case AsmJsRetType::Double:{
-            returnValue = JavascriptNumber::New(doubleRetVal, func->GetScriptContext());
+            returnValue = JavascriptNumber::NewWithCheck(doubleRetVal, func->GetScriptContext());
             break;
         }
         case AsmJsRetType::Float:{
-            returnValue = JavascriptNumber::New(floatRetVal, func->GetScriptContext());
+            returnValue = JavascriptNumber::NewWithCheck(floatRetVal, func->GetScriptContext());
             break;
         }
         case AsmJsRetType::Float32x4:
@@ -515,7 +515,7 @@ namespace Js
                 call ecx
                 movsd dval, xmm0
             }
-            returnValue = JavascriptNumber::New(dval, func->GetScriptContext());
+            returnValue = JavascriptNumber::NewWithCheck(dval, func->GetScriptContext());
             break;
         }
         case AsmJsRetType::Float:{
@@ -530,7 +530,7 @@ namespace Js
                 call ecx
                 movss fval, xmm0
             }
-            returnValue = JavascriptNumber::New((double)fval, func->GetScriptContext());
+            returnValue = JavascriptNumber::NewWithCheck((double)fval, func->GetScriptContext());
             break;
         }
         case AsmJsRetType::Int32x4:

+ 1 - 1
lib/Runtime/Language/InterpreterStackFrame.cpp

@@ -2091,7 +2091,7 @@ namespace Js
     inline void InterpreterStackFrame::OP_SetOutAsmDb( RegSlot outRegisterID, double val )
     {
         Assert( m_outParams + outRegisterID < m_outSp );
-        m_outParams[outRegisterID] = JavascriptNumber::New( val, scriptContext );
+        m_outParams[outRegisterID] = JavascriptNumber::NewWithCheck( val, scriptContext );
     }
 
     inline void InterpreterStackFrame::OP_SetOutAsmInt( RegSlot outRegisterID, int val )

+ 1 - 1
lib/Runtime/Language/i386/AsmJSJitTemplate.cpp

@@ -3145,7 +3145,7 @@ namespace Js
             size += SUB::EncodeInstruction<int>( buffer, InstrParamsRegImm<int8>( RegESP, 8 ) );
             size += MOVSD::EncodeInstruction<double>( buffer, InstrParamsAddrReg( RegESP, 0, regVariable ) );
 
-            size += MOV::EncodeInstruction<int>( buffer, InstrParamsRegImm<int32>( RegEAX, (int32)(Var(*)(double,ScriptContext*))JavascriptNumber::New) );
+            size += MOV::EncodeInstruction<int>( buffer, InstrParamsRegImm<int32>( RegEAX, (int32)(Var(*)(double,ScriptContext*))JavascriptNumber::NewWithCheck) );
             size += CALL::EncodeInstruction<int>( buffer, InstrParamsReg( RegEAX ) );
 
             size += MOV::EncodeInstruction<int>( buffer, InstrParamsAddrReg( RegESP, argIndex << 2, RegEAX ) );

+ 5 - 0
test/AsmJs/nanbug.baseline

@@ -0,0 +1,5 @@
+Successfully compiled asm.js code
+NaN
+0
+NaN
+0

+ 33 - 0
test/AsmJs/nanbug.js

@@ -0,0 +1,33 @@
+//-------------------------------------------------------------------------------------------------------
+// Copyright (C) Microsoft. All rights reserved.
+// Licensed under the MIT license. See LICENSE.txt file in the project root for full license information.
+//-------------------------------------------------------------------------------------------------------
+
+function AsmModule(stdlib,foreign,buffer) {
+    "use asm";
+    var HEAP32 =new stdlib.Float32Array(buffer);
+    var fround = stdlib.Math.fround;
+    var c = foreign.fun2;
+    function f() {
+        var a = fround(0);
+        var b = 0.;
+        a = fround(HEAP32[0]);
+        b = +a;
+        c(b);
+        return (~~b)|0;
+    }
+    
+    return {
+        f : f
+    };
+}
+
+var global = {Math:Math,Int8Array:Int8Array,Int16Array:Int16Array,Int32Array:Int32Array,Uint8Array:Uint8Array,Uint16Array:Uint16Array,Uint32Array:Uint32Array,Float32Array:Float32Array,Float64Array:Float64Array,Infinity:Infinity, NaN:NaN}
+var env = {fun1:function(x1,x2,x3,x4,x5,x6,x7,x8){print(x1,x2,x3,x4,x5,x6,x7,x8);}, fun2:function(x){print(x);},x:155,i2:658,d1:68.25,d2:3.14156,f1:48.1523,f2:14896.2514}
+var buffer = new ArrayBuffer(1<<20);
+var view = new Int32Array(buffer);
+view[0] = 0xffffffff
+var asmModule = new AsmModule(global,env,buffer);
+
+print(asmModule.f(Number.MAX_VALUE));
+print(asmModule.f(Number.MAX_VALUE));

+ 14 - 0
test/AsmJs/rlexe.xml

@@ -493,6 +493,20 @@
       <compile-flags>-testtrace:asmjs -simdjs -maic:0</compile-flags>
     </default>
   </test>
+  <test>
+    <default>
+      <files>nanbug.js</files>
+      <baseline>nanbug.baseline</baseline>
+      <compile-flags>-testtrace:asmjs -simdjs -maic:0</compile-flags>
+    </default>
+  </test>
+  <test>
+    <default>
+      <files>nanbug.js</files>
+      <baseline>nanbug.baseline</baseline>
+      <compile-flags>-testtrace:asmjs -simdjs</compile-flags>
+    </default>
+  </test>
   <test>
     <default>
       <files>switchbug.js</files>