[MERGE #5686 @nhat-nguyen] Float type specialization for comparison instructions

nhat-nguyen · nhat-nguyen · commit 50eaeec6871f · 2018-09-13T14:23:16.000-07:00
Merge pull request #5686 from nhat-nguyen:floats Perform type specialization and update destination value to boolean for comparison instructions when either source is float
diff --git a/lib/Backend/GlobOpt.cpp b/lib/Backend/GlobOpt.cpp
@@ -10473,6 +10473,7 @@ GlobOpt::TypeSpecializeFloatBinary(IR::Instr *instr, Value *src1Val, Value *src2
     bool skipSrc1 = false;
     bool skipSrc2 = false;
     bool skipDst = false;
+    bool convertDstToBool = false;
 
     if (!this->DoFloatTypeSpec())
     {
@@ -10544,6 +10545,19 @@ GlobOpt::TypeSpecializeFloatBinary(IR::Instr *instr, Value *src1Val, Value *src2
             skipDst = true;
             break;
 
+        case Js::OpCode::CmEq_A:
+        case Js::OpCode::CmSrEq_A:
+        case Js::OpCode::CmNeq_A:
+        case Js::OpCode::CmSrNeq_A:
+        case Js::OpCode::CmLe_A:
+        case Js::OpCode::CmLt_A:
+        case Js::OpCode::CmGe_A:
+        case Js::OpCode::CmGt_A:
+        {
+            convertDstToBool = true;
+            break;
+        }
+        
         default:
             return false;
         }
@@ -10589,13 +10603,19 @@ GlobOpt::TypeSpecializeFloatBinary(IR::Instr *instr, Value *src1Val, Value *src2
     if (!skipDst)
     {
         dst = instr->GetDst();
-
         if (dst)
         {
-            *pDstVal = CreateDstUntransferredValue(ValueType::Float, instr, src1Val, src2Val);
-
-            AssertMsg(dst->IsRegOpnd(), "What else?");
-            this->ToFloat64Dst(instr, dst->AsRegOpnd(), this->currentBlock);
+            if (convertDstToBool)
+            {
+                *pDstVal = CreateDstUntransferredValue(ValueType::Boolean, instr, src1Val, src2Val);
+                ToVarRegOpnd(dst->AsRegOpnd(), currentBlock);
+            }
+            else
+            {
+                *pDstVal = CreateDstUntransferredValue(ValueType::Float, instr, src1Val, src2Val);
+                AssertMsg(dst->IsRegOpnd(), "What else?");
+                this->ToFloat64Dst(instr, dst->AsRegOpnd(), this->currentBlock);
+            }
         }
     }
 
diff --git a/lib/Backend/LowerMDShared.cpp b/lib/Backend/LowerMDShared.cpp
@@ -2653,9 +2653,7 @@ void LowererMD::GenerateFastCmXx(IR::Instr *instr)
     bool isFloatSrc = src1->IsFloat();
     bool isInt64Src = src1->IsInt64();
     Assert(!isFloatSrc || src2->IsFloat());
-    Assert(!isFloatSrc || isIntDst);
     Assert(!isInt64Src || src2->IsInt64());
-    Assert(!isInt64Src || isIntDst);
     Assert(!isFloatSrc || AutoSystemInfo::Data.SSE2Available());
     IR::Opnd *opnd;
     IR::Instr *newInstr;
@@ -2684,16 +2682,19 @@ void LowererMD::GenerateFastCmXx(IR::Instr *instr)
         done = instr;
     }
 
+    bool isNegOpt = instr->m_opcode == Js::OpCode::CmNeq_A || instr->m_opcode == Js::OpCode::CmSrNeq_A;
+    bool initDstToFalse = true;
     if (isIntDst)
     {
+        // Fast path for int src with destination type specialized to int
         // reg = MOV 0 will get peeped to XOR reg, reg which sets the flags.
         // Put the MOV before the CMP, but use a tmp if dst == src1/src2
         if (dst->IsEqual(src1) || dst->IsEqual(src2))
         {
             tmp = IR::RegOpnd::New(dst->GetType(), this->m_func);
         }
         // dst = MOV 0
-        if (isFloatSrc && instr->m_opcode == Js::OpCode::CmNeq_A)
+        if (isFloatSrc && isNegOpt)
         {
             opnd = IR::IntConstOpnd::New(1, TyInt32, this->m_func);
         }
@@ -2703,6 +2704,22 @@ void LowererMD::GenerateFastCmXx(IR::Instr *instr)
         }
         m_lowerer->InsertMove(tmp, opnd, done);
     }
+    else if (isFloatSrc)
+    {
+        // Fast path for float src when destination is a var
+        // Assign default value for destination in case either src is NaN
+        Assert(dst->IsVar());
+        if (isNegOpt)
+        {
+            opnd = this->m_lowerer->LoadLibraryValueOpnd(instr, LibraryValue::ValueTrue);
+        }
+        else
+        {
+            opnd = this->m_lowerer->LoadLibraryValueOpnd(instr, LibraryValue::ValueFalse);
+            initDstToFalse = false;
+        }
+        Lowerer::InsertMove(tmp, opnd, done);
+    }
 
     Js::OpCode cmpOp;
     if (isFloatSrc)
@@ -2733,7 +2750,9 @@ void LowererMD::GenerateFastCmXx(IR::Instr *instr)
         done->InsertBefore(newInstr);
     }
 
-    if (!isIntDst)
+    // For all cases where the operator is a comparison, we do not want to emit False value
+    // since it has already been generated in the if block before.
+    if (!isIntDst && initDstToFalse)
     {
         opnd = this->m_lowerer->LoadLibraryValueOpnd(instr, LibraryValue::ValueFalse);
         Lowerer::InsertMove(tmp, opnd, done);
@@ -2744,11 +2763,13 @@ void LowererMD::GenerateFastCmXx(IR::Instr *instr)
     {
     case Js::OpCode::CmEq_I4:
     case Js::OpCode::CmEq_A:
+    case Js::OpCode::CmSrEq_A:
         useCC = isIntDst ? Js::OpCode::SETE : Js::OpCode::CMOVE;
         break;
 
     case Js::OpCode::CmNeq_I4:
     case Js::OpCode::CmNeq_A:
+    case Js::OpCode::CmSrNeq_A:
         useCC = isIntDst ? Js::OpCode::SETNE : Js::OpCode::CMOVNE;
         break;
 
diff --git a/test/Basics/FloatComparison.js b/test/Basics/FloatComparison.js
@@ -0,0 +1,69 @@
+//-------------------------------------------------------------------------------------------------------
+// Copyright (C) Microsoft. All rights reserved.
+// Licensed under the MIT license. See LICENSE.txt file in the project root for full license information.
+//-------------------------------------------------------------------------------------------------------
+
+WScript.LoadScriptFile("..\\UnitTestFramework\\UnitTestFramework.js");
+
+// This tests the fast path for cmxx where either src is type specialized to float
+var tests = [
+    {
+        name: "NaN equality test",
+        body: function() {
+            assert.isTrue(NaN !== NaN);
+            assert.isTrue(NaN !== 0.5);
+            assert.isTrue(0.5 !== NaN);
+        
+            assert.isTrue(NaN != NaN);
+            assert.isTrue(NaN != 0.5);
+            assert.isTrue(0.5 != NaN);
+        
+            assert.isFalse(NaN === NaN);
+            assert.isFalse(NaN === 0.5);
+            assert.isFalse(0.5 === NaN);
+        
+            assert.isFalse(NaN == NaN);
+            assert.isFalse(NaN == 0.5);
+            assert.isFalse(0.5 == NaN);
+
+            assert.isFalse(NaN > 0.5);
+            assert.isFalse(NaN >= 0.5);
+            assert.isFalse(NaN < 0.5);
+            assert.isFalse(NaN <= 0.5);
+        }
+    },
+    {
+        name: "Type coercion test",
+        body: function() {
+            assert.isTrue('0.5' == 0.5);
+            assert.isFalse('0.5' === 0.5);
+            assert.isFalse('NaN' == NaN);
+            assert.isTrue('NaN' != NaN);
+        }
+    },
+    {
+        name: "int vs. float",
+        body: function() {
+            assert.isFalse(5 == 0.5);
+            assert.isTrue(5 != 0.5);
+            assert.isFalse(5 === 0.5);
+            assert.isTrue(5 !== 0.5);
+        }
+    },
+    {
+        name: "object vs. float",
+        body: function() {
+            assert.isFalse({} == 0.5);
+            assert.isFalse({} === 0.5);
+            assert.isTrue({} != 0.5);
+            assert.isTrue({} !== 0.5);
+        
+            assert.isFalse({} > 0.5);
+            assert.isFalse({} >= 0.5);
+            assert.isFalse({} < 0.5);
+            assert.isFalse({} <= 0.5);
+        }
+    }
+];
+
+testRunner.runTests(tests, { verbose: WScript.Arguments[0] != "summary" });
diff --git a/test/Basics/rlexe.xml b/test/Basics/rlexe.xml
@@ -337,4 +337,10 @@
       <tags>exclude_test,exclude_jshost</tags>
     </default>
   </test>
+  <test>
+    <default>
+      <files>FloatComparison.js</files>
+      <compile-flags>-args summary -endargs</compile-flags>
+    </default>
+  </test>
 </regress-exe>
