[Merge chakra-core/ChakraCore@872220a98a] [1.6>1.7] OS#13129251: Math…

….min/max should return an integer value when all of its params are integers

This change addresses a perf issue where non-inlined Math.min/max are always floating-point vars. In
the bug, this causes expensive bailouts in a loop that was setting to a Uin8ClampedArray. The fix is
to check whether all of the parameters are tagged integers, and, if so, return an int.
With a reduced repro of the scenario from the original page, there is a significant improvement,
where the same function takes 20% of the time it did before. Normal usage of Math.max with 3 int
parameters set to a var results in taking 65% of the time it did before. Normal usage of Math.max
with a float parameter showed a 1-5% regression, depending on where the first non-int parameter is
listed.

deps/chakrashim/core/lib/Runtime/Language/TaggedInt.h

@@ -44,6 +44,7 @@ namespace Js {
         static bool Is(Var aValue);
         static bool Is(intptr_t aValue);
         static bool IsPair(Var aLeft, Var aRight);
+        static bool OnlyContainsTaggedInt(Js::Arguments& args);
         static double ToDouble(Var aValue);
         static int32 ToInt32(Var aValue);
         static int32 ToInt32(intptr_t aValue);

deps/chakrashim/core/lib/Runtime/Language/TaggedInt.inl

@@ -206,7 +206,22 @@ namespace Js
     }
 #endif
 
+    // Returns true when the provided args only contains TaggedInts. Note that
+    // iteration starts from 1 (to account for 'this' at index 0).
+    bool inline TaggedInt::OnlyContainsTaggedInt(Js::Arguments& args)
+    {
+        bool onlyContainsInts = true;
+        for (uint idxArg = 1; idxArg < args.Info.Count; idxArg++)
+        {
+            if (!TaggedInt::Is(args[idxArg]))
+            {
+                onlyContainsInts = false;
+                break;
+            }
+        }
 
+        return onlyContainsInts;
+    }
 
     inline Var TaggedInt::Add(Var aLeft,Var aRight,ScriptContext* scriptContext)
 #ifdef DBG

deps/chakrashim/core/lib/Runtime/Library/MathLibrary.cpp

@@ -702,6 +702,7 @@ namespace Js
         ARGUMENTS(args, callInfo);
         AssertMsg(args.Info.Count > 0, "Should always have implicit 'this'");
         ScriptContext* scriptContext = function->GetScriptContext();
+        bool hasOnlyIntegerArgs = false;
 
         Assert(!(callInfo.Flags & CallFlags_New));
 
@@ -714,14 +715,31 @@ namespace Js
             double result = JavascriptConversion::ToNumber(args[1], scriptContext);
             return JavascriptNumber::ToVarNoCheck(result, scriptContext);
         }
-        else if (args.Info.Count == 3)
+        else
         {
-            if (TaggedInt::Is(args[1]) && TaggedInt::Is(args[2]))
+            hasOnlyIntegerArgs = TaggedInt::OnlyContainsTaggedInt(args);
+            if (hasOnlyIntegerArgs && args.Info.Count == 3)
             {
                 return TaggedInt::ToVarUnchecked(max(TaggedInt::ToInt32(args[1]), TaggedInt::ToInt32(args[2])));
             }
         }
 
+        if (hasOnlyIntegerArgs)
+        {
+            int32 current = TaggedInt::ToInt32(args[1]);
+            for (uint idxArg = 2; idxArg < args.Info.Count; idxArg++)
+            {
+                int32 compare = TaggedInt::ToInt32(args[idxArg]);
+                if (current < compare)
+                {
+                    current = compare;
+                }
+            }
+
+            return TaggedInt::ToVarUnchecked(current);
+        }
+        else
+        {
         double current = JavascriptConversion::ToNumber(args[1], scriptContext);
         if(JavascriptNumber::IsNan(current))
         {
@@ -735,7 +753,10 @@ namespace Js
             {
                 return scriptContext->GetLibrary()->GetNaN();
             }
-            if((JavascriptNumber::IsNegZero(current) && compare == 0) ||
+
+                // In C++, -0.0f == 0.0f; however, in ES, -0.0f < 0.0f. Thus, use additional library 
+                // call to test this comparison.
+                if ((compare == 0 && JavascriptNumber::IsNegZero(current)) ||
                 current < compare )
             {
                 current = compare;
@@ -744,6 +765,7 @@ namespace Js
 
         return JavascriptNumber::ToVarNoCheck(current, scriptContext);
     }
+    }
 
 
     ///----------------------------------------------------------------------------
@@ -760,6 +782,7 @@ namespace Js
         ARGUMENTS(args, callInfo);
         AssertMsg(args.Info.Count > 0, "Should always have implicit 'this'");
         ScriptContext* scriptContext = function->GetScriptContext();
+        bool hasOnlyIntegerArgs = false;
 
         Assert(!(callInfo.Flags & CallFlags_New));
 
@@ -772,14 +795,31 @@ namespace Js
             double result = JavascriptConversion::ToNumber(args[1], scriptContext);
             return JavascriptNumber::ToVarNoCheck(result, scriptContext);
         }
-        else if (args.Info.Count == 3)
+        else
         {
-            if (TaggedInt::Is(args[1]) && TaggedInt::Is(args[2]))
+            hasOnlyIntegerArgs = TaggedInt::OnlyContainsTaggedInt(args);
+            if (hasOnlyIntegerArgs && args.Info.Count == 3)
             {
                 return TaggedInt::ToVarUnchecked(min(TaggedInt::ToInt32(args[1]), TaggedInt::ToInt32(args[2])));
             }
         }
 
+        if (hasOnlyIntegerArgs)
+        {
+            int32 current = TaggedInt::ToInt32(args[1]);
+            for (uint idxArg = 2; idxArg < args.Info.Count; idxArg++)
+            {
+                int32 compare = TaggedInt::ToInt32(args[idxArg]);
+                if (current > compare)
+                {
+                    current = compare;
+                }
+            }
+
+            return TaggedInt::ToVarUnchecked(current);
+        }
+        else
+        {
         double current = JavascriptConversion::ToNumber(args[1], scriptContext);
         if(JavascriptNumber::IsNan(current))
         {
@@ -793,7 +833,10 @@ namespace Js
             {
                 return scriptContext->GetLibrary()->GetNaN();
             }
-            if((JavascriptNumber::IsNegZero(compare) && current == 0) ||
+
+                // In C++, -0.0f == 0.0f; however, in ES, -0.0f < 0.0f. Thus, use additional library 
+                // call to test this comparison.
+                if ((current == 0 && JavascriptNumber::IsNegZero(compare)) ||
                 current > compare )
             {
                 current = compare;
@@ -802,6 +845,7 @@ namespace Js
 
         return JavascriptNumber::ToVarNoCheck(current, scriptContext);
     }
+    }
 
 
     ///----------------------------------------------------------------------------