Don't Free UpperVector (#23889)
authorCarol Eidt <carol.eidt@microsoft.com>
Fri, 12 Apr 2019 18:31:55 +0000 (11:31 -0700)
committerGitHub <noreply@github.com>
Fri, 12 Apr 2019 18:31:55 +0000 (11:31 -0700)
* Don't Free UpperVector

UpperVector regs are freed at their time of use, and shouldn't be freed when the last RefPosition is encountered.
Also, we need to specify all 3 operands for vinsertf128

Fix #23861
Fix #23904
Fix #23804

src/jit/lsra.cpp
src/jit/simdcodegenxarch.cpp
tests/src/JIT/Regression/JitBlue/GitHub_23861/GitHub_23861.cs [new file with mode: 0644]
tests/src/JIT/Regression/JitBlue/GitHub_23861/GitHub_23861.csproj [new file with mode: 0644]

index 9d4344f..8a5323c 100644 (file)
@@ -5774,36 +5774,41 @@ void LinearScan::allocateRegisters()
             // The interval could be dead if this is a user variable, or if the
             // node is being evaluated for side effects, or a call whose result
             // is not used, etc.
-            if (currentRefPosition->lastUse || currentRefPosition->nextRefPosition == nullptr)
+            // If this is an UpperVector we'll neither free it nor preference it
+            // (it will be freed when it is used).
+            if (!currentInterval->IsUpperVector())
             {
-                assert(currentRefPosition->isIntervalRef());
-
-                if (refType != RefTypeExpUse && currentRefPosition->nextRefPosition == nullptr)
+                if (currentRefPosition->lastUse || currentRefPosition->nextRefPosition == nullptr)
                 {
-                    if (currentRefPosition->delayRegFree)
+                    assert(currentRefPosition->isIntervalRef());
+
+                    if (refType != RefTypeExpUse && currentRefPosition->nextRefPosition == nullptr)
                     {
-                        delayRegsToFree |= assignedRegBit;
+                        if (currentRefPosition->delayRegFree)
+                        {
+                            delayRegsToFree |= assignedRegBit;
+
+                            INDEBUG(dumpLsraAllocationEvent(LSRA_EVENT_LAST_USE_DELAYED));
+                        }
+                        else
+                        {
+                            regsToFree |= assignedRegBit;
 
-                        INDEBUG(dumpLsraAllocationEvent(LSRA_EVENT_LAST_USE_DELAYED));
+                            INDEBUG(dumpLsraAllocationEvent(LSRA_EVENT_LAST_USE));
+                        }
                     }
                     else
                     {
-                        regsToFree |= assignedRegBit;
-
-                        INDEBUG(dumpLsraAllocationEvent(LSRA_EVENT_LAST_USE));
+                        currentInterval->isActive = false;
                     }
-                }
-                else
-                {
-                    currentInterval->isActive = false;
-                }
 
-                // Update the register preferences for the relatedInterval, if this is 'preferencedToDef'.
-                // Don't propagate to subsequent relatedIntervals; that will happen as they are allocated, and we
-                // don't know yet whether the register will be retained.
-                if (currentInterval->relatedInterval != nullptr)
-                {
-                    currentInterval->relatedInterval->updateRegisterPreferences(assignedRegBit);
+                    // Update the register preferences for the relatedInterval, if this is 'preferencedToDef'.
+                    // Don't propagate to subsequent relatedIntervals; that will happen as they are allocated, and we
+                    // don't know yet whether the register will be retained.
+                    if (currentInterval->relatedInterval != nullptr)
+                    {
+                        currentInterval->relatedInterval->updateRegisterPreferences(assignedRegBit);
+                    }
                 }
             }
 
index 6982a1b..a340fb0 100644 (file)
@@ -3089,7 +3089,7 @@ void CodeGen::genSIMDIntrinsicUpperRestore(GenTreeSIMD* simdNode)
     assert(srcReg != REG_NA);
     if (srcReg != REG_STK)
     {
-        getEmitter()->emitIns_R_R_I(INS_vinsertf128, EA_32BYTE, lclVarReg, srcReg, 0x01);
+        getEmitter()->emitIns_R_R_R_I(INS_vinsertf128, EA_32BYTE, lclVarReg, lclVarReg, srcReg, 0x01);
     }
     else
     {
@@ -3099,7 +3099,7 @@ void CodeGen::genSIMDIntrinsicUpperRestore(GenTreeSIMD* simdNode)
         assert(varDsc->lvOnFrame);
         // We will load this from the upper 16 bytes of this localVar's home.
         int offs = 16;
-        getEmitter()->emitIns_R_S_I(INS_vinsertf128, EA_32BYTE, lclVarReg, varNum, offs, 0x01);
+        getEmitter()->emitIns_R_R_S_I(INS_vinsertf128, EA_32BYTE, lclVarReg, lclVarReg, varNum, offs, 0x01);
     }
 }
 
diff --git a/tests/src/JIT/Regression/JitBlue/GitHub_23861/GitHub_23861.cs b/tests/src/JIT/Regression/JitBlue/GitHub_23861/GitHub_23861.cs
new file mode 100644 (file)
index 0000000..2bd0d09
--- /dev/null
@@ -0,0 +1,69 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+// See the LICENSE file in the project root for more information.
+
+using System;
+using System.Runtime.CompilerServices;
+using System.Numerics;
+
+namespace GitHub_23861
+{
+    class Program
+    {
+        static int returnVal = 100;
+        static int Main(string[] args)
+        {
+            LessThanAllDouble();
+
+            if (returnVal == 100)
+            {
+                Console.WriteLine("Pass");                
+            }
+            else
+            {
+                Console.WriteLine("FAIL");
+            }
+            return returnVal;
+        }
+
+        public static void LessThanAllDouble() { TestVectorLessThanAll<double>(); }
+
+        private static void TestVectorLessThanAll<T>() where T : struct
+        {
+            T[] values1 = new T[Vector<T>.Count];
+            for (int g = 0; g < Vector<T>.Count; g++)
+            {
+                values1[g] = (T)(dynamic)g;
+            }
+            Vector<T> vec1 = new Vector<T>(values1);
+
+            T[] values2 = new T[Vector<T>.Count];
+            for (int g = 0; g < Vector<T>.Count; g++)
+            {
+                values2[g] = (T)(dynamic)(g + 25);
+            }
+            Vector<T> vec2 = new Vector<T>(values2);
+
+            if (!Vector.LessThanAll(vec1, vec2))
+            {
+                returnVal = -1;
+            }
+            if (!Vector.LessThanAll(Vector<T>.Zero, Vector<T>.One))
+            {
+                returnVal = -1;
+            }
+
+            T[] values3 = new T[Vector<T>.Count];
+            for (int g = 0; g < Vector<T>.Count; g++)
+            {
+                values3[g] = (g < Vector<T>.Count / 2) ? Vector<T>.Zero[0] : Vector<T>.One[0];
+            }
+            Vector<T> vec3 = new Vector<T>(values3);
+            if (Vector.LessThanAll(vec3, Vector<T>.One))
+            {
+                returnVal = -1;
+            }
+        }
+    }
+}
+
diff --git a/tests/src/JIT/Regression/JitBlue/GitHub_23861/GitHub_23861.csproj b/tests/src/JIT/Regression/JitBlue/GitHub_23861/GitHub_23861.csproj
new file mode 100644 (file)
index 0000000..e191e01
--- /dev/null
@@ -0,0 +1,34 @@
+<?xml version="1.0" encoding="utf-8"?>
+<Project ToolsVersion="12.0" DefaultTargets="Build" xmlns="http://schemas.microsoft.com/developer/msbuild/2003">
+  <Import Project="$([MSBuild]::GetDirectoryNameOfFileAbove($(MSBuildThisFileDirectory), dir.props))\dir.props" />
+  <PropertyGroup>
+    <Configuration Condition=" '$(Configuration)' == '' ">Debug</Configuration>
+    <Platform Condition=" '$(Platform)' == '' ">AnyCPU</Platform>
+    <AssemblyName>$(MSBuildProjectName)</AssemblyName>
+    <SchemaVersion>2.0</SchemaVersion>
+    <ProjectGuid>{95DFC527-4DC1-495E-97D7-E94EE1F7140D}</ProjectGuid>
+    <OutputType>Exe</OutputType>
+    <ProjectTypeGuids>{786C830F-07A1-408B-BD7F-6EE04809D6DB};{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}</ProjectTypeGuids>
+    <SolutionDir Condition="$(SolutionDir) == '' Or $(SolutionDir) == '*Undefined*'">..\..\</SolutionDir>
+  </PropertyGroup>
+  <!-- Default configurations to help VS understand the configurations -->
+  <PropertyGroup Condition=" '$(Configuration)|$(Platform)' == 'Debug|AnyCPU' "></PropertyGroup>
+  <PropertyGroup Condition=" '$(Configuration)|$(Platform)' == 'Release|AnyCPU' "></PropertyGroup>
+  <ItemGroup>
+    <CodeAnalysisDependentAssemblyPaths Condition=" '$(VS100COMNTOOLS)' != '' " Include="$(VS100COMNTOOLS)..\IDE\PrivateAssemblies">
+      <Visible>False</Visible>
+    </CodeAnalysisDependentAssemblyPaths>
+  </ItemGroup>
+  <PropertyGroup>
+    <DebugType>None</DebugType>
+    <Optimize>True</Optimize>
+  </PropertyGroup>
+  <ItemGroup>
+    <Compile Include="$(MSBuildProjectName).cs" />
+  </ItemGroup>
+  <ItemGroup>
+    <Service Include="{82A7F48D-3B50-4B1E-B82E-3ADA8210C358}" />
+  </ItemGroup>
+  <Import Project="$([MSBuild]::GetDirectoryNameOfFileAbove($(MSBuildThisFileDirectory), dir.targets))\dir.targets" />
+  <PropertyGroup Condition=" '$(MsBuildProjectDirOverride)' != '' "></PropertyGroup>
+</Project>