Fix bad codegen for initblk with large fill value

When the initialization value of an init block is constant we attempt to
generate an inline expansion sequences to execute the stores. In order to
generate wider stores the constant value is expanded so that each byte
position holds the initial byte value. In some cases the constant on the
initblk may be a value not in the range of 0..255 and this constant
expansion will generate the wrong value. This change fixes the x64, x86,
and arm64 (comment only) expanses to use only the lower byte value of this
constant. The ARM32 backend already does this, and the other JIT32/JIT64
JITs shipping in Desktop also do this correctly.

Commit migrated from https://github.com/dotnet/coreclr/commit/2a487c287a732572bb8043fd9f3e17f32c855c35

diff --git a/src/coreclr/src/jit/lowerarm64.cpp b/src/coreclr/src/jit/lowerarm64.cpp
index 465800a..c51f7b0 100644 (file)
--- a/src/coreclr/src/jit/lowerarm64.cpp
+++ b/src/coreclr/src/jit/lowerarm64.cpp
@@ -1038,16 +1038,21 @@ Lowering::TreeNodeInfoInitBlockStore(GenTreeBlkOp* blkNode)
             && initVal->IsCnsIntOrI())
         {
             ssize_t size = blockSize->gtIntCon.gtIconVal;
-            // Replace the integer constant in initVal 
-            // to fill an 8-byte word with the fill value of the InitBlk
-            assert(initVal->gtIntCon.gtIconVal == (initVal->gtIntCon.gtIconVal & 0xFF));
+            // The fill value of an initblk is interpreted to hold a
+            // value of (unsigned int8) however a constant of any size
+            // may practically reside on the evaluation stack. So extract
+            // the lower byte out of the initVal constant and replicate
+            // it to a larger constant whose size is sufficient to support
+            // the largest width store of the desired inline expansion.
+
+            ssize_t fill = initVal->gtIntCon.gtIconVal & 0xFF;
             if (size < REGSIZE_BYTES)
             {
-                initVal->gtIntCon.gtIconVal = 0x01010101 * initVal->gtIntCon.gtIconVal;
+                initVal->gtIntCon.gtIconVal = 0x01010101 * fill;
             }
             else
             {
-                initVal->gtIntCon.gtIconVal = 0x0101010101010101LL * initVal->gtIntCon.gtIconVal;
+                initVal->gtIntCon.gtIconVal = 0x0101010101010101LL * fill;
                 initVal->gtType = TYP_LONG;
             }
 

diff --git a/src/coreclr/src/jit/lowerxarch.cpp b/src/coreclr/src/jit/lowerxarch.cpp
index 5a2e429..c9ea7d1 100644 (file)
--- a/src/coreclr/src/jit/lowerxarch.cpp
+++ b/src/coreclr/src/jit/lowerxarch.cpp
@@ -1769,21 +1769,26 @@ Lowering::TreeNodeInfoInitBlockStore(GenTreeBlkOp* blkNode)
             // Always favor unrolling vs rep stos.
             if (size <= INITBLK_UNROLL_LIMIT && initVal->IsCnsIntOrI())
             {
-                // Replace the integer constant in initVal 
-                // to fill an 8-byte word with the fill value of the InitBlk
-                assert(initVal->gtIntCon.gtIconVal == (initVal->gtIntCon.gtIconVal & 0xFF));
+                // The fill value of an initblk is interpreted to hold a
+                // value of (unsigned int8) however a constant of any size
+                // may practically reside on the evaluation stack. So extract
+                // the lower byte out of the initVal constant and replicate
+                // it to a larger constant whose size is sufficient to support
+                // the largest width store of the desired inline expansion.
+
+                ssize_t fill = initVal->gtIntCon.gtIconVal & 0xFF;
 #ifdef _TARGET_AMD64_
                 if (size < REGSIZE_BYTES)
                 {
-                    initVal->gtIntCon.gtIconVal = 0x01010101 * initVal->gtIntCon.gtIconVal;
+                    initVal->gtIntCon.gtIconVal = 0x01010101 * fill;
                 }
                 else
                 {
-                    initVal->gtIntCon.gtIconVal = 0x0101010101010101LL * initVal->gtIntCon.gtIconVal;
+                    initVal->gtIntCon.gtIconVal = 0x0101010101010101LL * fill;
                     initVal->gtType = TYP_LONG;
                 }
 #else // !_TARGET_AMD64_
-                initVal->gtIntCon.gtIconVal = 0x01010101 * initVal->gtIntCon.gtIconVal;
+                initVal->gtIntCon.gtIconVal = 0x01010101 * fill;
 #endif // !_TARGET_AMD64_
 
                 MakeSrcContained(blkNode, blockSize);

diff --git a/src/coreclr/tests/src/JIT/Regression/JitBlue/DevDiv_199169/DevDiv_199169.il b/src/coreclr/tests/src/JIT/Regression/JitBlue/DevDiv_199169/DevDiv_199169.il
new file mode 100644 (file)
index 0000000..482c695
--- /dev/null
+++ b/src/coreclr/tests/src/JIT/Regression/JitBlue/DevDiv_199169/DevDiv_199169.il
@@ -0,0 +1,64 @@
+// 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.
+
+.assembly extern legacy library mscorlib {}
+.assembly DevDiv_199169
+{
+}
+.module DevDiv_199169.exe
+
+.class public sequential ansi sealed beforefieldinit Blob
+       extends [mscorlib]System.ValueType
+{
+  .pack 0
+  .size 3
+}
+
+.field public static valuetype Blob bag
+
+.method assembly static void test() cil managed
+{
+  .maxstack  3
+  IL_0000:  ldsflda    valuetype Blob bag
+  IL_0005:  ldc.i4.s   -128 // this constant is outside 0..255
+  IL_0007:  ldc.i4.3
+  IL_0008:  unaligned. 1
+  IL_000b:  initblk
+  IL_000d:  ret
+}
+
+.method assembly static int32  main(string[] args) cil managed
+{
+  .entrypoint
+  .maxstack  6
+  .locals (int32 V_0)
+  IL_0000:  ldc.i4.0
+  IL_0001:  stloc.0
+  IL_0002:  call       void test()
+  IL_0007:  ldsflda    valuetype Blob bag
+  IL_000c:  ldc.i4.0
+  IL_0011:  add
+  IL_0012:  ldind.u1
+  IL_0013:  ldc.i4     0x80
+  IL_0018:  bne.un.s   IL_0048
+
+  IL_001a:  ldsflda    valuetype Blob bag
+  IL_001f:  ldc.i4.1
+  IL_0020:  add
+  IL_0021:  ldind.u1
+  IL_0022:  ldc.i4     0x80
+  IL_0027:  bne.un.s   IL_0048
+
+  IL_0029:  ldsflda    valuetype Blob bag
+  IL_002e:  ldc.i4.2
+  IL_0033:  add
+  IL_0034:  ldind.u1
+  IL_0035:  ldc.i4     0x80
+  IL_003a:  bne.un.s   IL_0048
+
+  IL_003c:  ldc.i4.s   100
+  IL_0041:  ret
+  IL_0048:  ldc.i4.s   101
+  IL_0050:  ret
+}

diff --git a/src/coreclr/tests/src/JIT/Regression/JitBlue/DevDiv_199169/DevDiv_199169.ilproj b/src/coreclr/tests/src/JIT/Regression/JitBlue/DevDiv_199169/DevDiv_199169.ilproj
new file mode 100644 (file)
index 0000000..db1d8ef
--- /dev/null
+++ b/src/coreclr/tests/src/JIT/Regression/JitBlue/DevDiv_199169/DevDiv_199169.ilproj
@@ -0,0 +1,44 @@
+<?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>
+    <AppDesignerFolder>Properties</AppDesignerFolder>
+    <FileAlignment>512</FileAlignment>
+    <ProjectTypeGuids>{786C830F-07A1-408B-BD7F-6EE04809D6DB};{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}</ProjectTypeGuids>
+    <ReferencePath>$(ProgramFiles)\Common Files\microsoft shared\VSTT\11.0\UITestExtensionPackages</ReferencePath>
+    <SolutionDir Condition="$(SolutionDir) == '' Or $(SolutionDir) == '*Undefined*'">..\..\</SolutionDir>
+    <NuGetPackageImportStamp>7a9bfb7d</NuGetPackageImportStamp>
+  </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="DevDiv_199169.il" />
+  </ItemGroup>
+  <ItemGroup>
+    <None Include="app.config" />
+  </ItemGroup>
+  <ItemGroup>
+    <Service Include="{82A7F48D-3B50-4B1E-B82E-3ADA8210C358}" />
+  </ItemGroup>
+  <Import Project="$([MSBuild]::GetDirectoryNameOfFileAbove($(MSBuildThisFileDirectory), dir.targets))\dir.targets" />
+  <PropertyGroup Condition=" '$(MsBuildProjectDirOverride)' != '' ">
+  </PropertyGroup> 
+</Project>
\ No newline at end of file

diff --git a/src/coreclr/tests/src/JIT/Regression/JitBlue/DevDiv_199169/app.config b/src/coreclr/tests/src/JIT/Regression/JitBlue/DevDiv_199169/app.config
new file mode 100644 (file)
index 0000000..8077c95
--- /dev/null
+++ b/src/coreclr/tests/src/JIT/Regression/JitBlue/DevDiv_199169/app.config
@@ -0,0 +1,27 @@
+<?xml version="1.0" encoding="utf-8"?>
+<configuration>
+  <runtime>
+    <assemblyBinding xmlns="urn:schemas-microsoft-com:asm.v1">
+      <dependentAssembly>
+        <assemblyIdentity name="System.Runtime" publicKeyToken="b03f5f7f11d50a3a" culture="neutral" />
+        <bindingRedirect oldVersion="0.0.0.0-4.0.20.0" newVersion="4.0.20.0" />
+      </dependentAssembly>
+      <dependentAssembly>
+        <assemblyIdentity name="System.Text.Encoding" publicKeyToken="b03f5f7f11d50a3a" culture="neutral" />
+        <bindingRedirect oldVersion="0.0.0.0-4.0.10.0" newVersion="4.0.10.0" />
+      </dependentAssembly>
+      <dependentAssembly>
+        <assemblyIdentity name="System.Threading.Tasks" publicKeyToken="b03f5f7f11d50a3a" culture="neutral" />
+        <bindingRedirect oldVersion="0.0.0.0-4.0.10.0" newVersion="4.0.10.0" />
+      </dependentAssembly>
+      <dependentAssembly>
+        <assemblyIdentity name="System.IO" publicKeyToken="b03f5f7f11d50a3a" culture="neutral" />
+        <bindingRedirect oldVersion="0.0.0.0-4.0.10.0" newVersion="4.0.10.0" />
+      </dependentAssembly>
+      <dependentAssembly>
+        <assemblyIdentity name="System.Reflection" publicKeyToken="b03f5f7f11d50a3a" culture="neutral" />
+        <bindingRedirect oldVersion="0.0.0.0-4.0.10.0" newVersion="4.0.10.0" />
+      </dependentAssembly>
+    </assemblyBinding>
+  </runtime>
+</configuration>
\ No newline at end of file