JIT: optimize some cases of unused structs (dotnet/coreclr#18819)

In some cases CSC will use `ldobj; pop` to null check a pointer to
struct. This change avoids copying the struct value for such constructs.

Codegen may still redundantly null check, if there are multiple such checks
in a method.

Fixes dotnet/coreclr#18710

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

diff --git a/src/coreclr/src/jit/importer.cpp b/src/coreclr/src/jit/importer.cpp
index 3ed4268..fa72bb1 100644 (file)
--- a/src/coreclr/src/jit/importer.cpp
+++ b/src/coreclr/src/jit/importer.cpp
@@ -12540,6 +12540,8 @@ void Compiler::impImportBlockCode(BasicBlock* block)
 
                     if (varTypeIsStruct(op1))
                     {
+                        JITDUMP("\n ... CEE_POP struct ...\n");
+                        DISPTREE(op1);
 #ifdef UNIX_AMD64_ABI
                         // Non-calls, such as obj or ret_expr, have to go through this.
                         // Calls with large struct return value have to go through this.
@@ -12549,7 +12551,20 @@ void Compiler::impImportBlockCode(BasicBlock* block)
                             op1->AsCall()->gtCallType == CT_HELPER)
 #endif // UNIX_AMD64_ABI
                         {
-                            op1 = impGetStructAddr(op1, clsHnd, (unsigned)CHECK_SPILL_ALL, false);
+                            // If the value being produced comes from loading
+                            // via an underlying address, just null check the address.
+                            if (op1->OperIs(GT_FIELD, GT_IND, GT_OBJ))
+                            {
+                                op1->ChangeOper(GT_NULLCHECK);
+                                op1->gtType = TYP_BYTE;
+                            }
+                            else
+                            {
+                                op1 = impGetStructAddr(op1, clsHnd, (unsigned)CHECK_SPILL_ALL, false);
+                            }
+
+                            JITDUMP("\n ... optimized to ...\n");
+                            DISPTREE(op1);
                         }
                     }
 

diff --git a/src/coreclr/tests/src/JIT/opt/Structs/structpop.cs b/src/coreclr/tests/src/JIT/opt/Structs/structpop.cs
new file mode 100644 (file)
index 0000000..7244d1f
--- /dev/null
+++ b/src/coreclr/tests/src/JIT/opt/Structs/structpop.cs
@@ -0,0 +1,83 @@
+// 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.
+
+// Optimization of pop with struct types
+// Codegen for TestByRef and TestByPtr should be similar
+//
+// See CoreClr#18710
+
+using System;
+using System.Runtime.CompilerServices;
+using System.Runtime.InteropServices;
+
+[StructLayout(LayoutKind.Sequential)]
+struct VT
+{
+    public static readonly int Size = Marshal.SizeOf<VT>();
+    
+    public int F1, F2, F3;
+    
+    [MethodImpl(MethodImplOptions.AggressiveInlining)]
+    public void Get(out int v1, out int v2, out int v3)
+    {
+        v1 = F1;
+        v2 = F2;
+        v3 = F3;
+    }
+}
+
+class P
+{
+    [MethodImpl(MethodImplOptions.NoInlining)]
+    unsafe static int TestMethodInlining(VT* pVT)
+    {
+        int v1, v2, v3;
+        pVT->Get(out v1, out v2, out v3);
+        return Do(v1, v2);
+    }
+    
+    [MethodImpl(MethodImplOptions.NoInlining)]
+    static int TestByRef(ref VT VTRef)
+    {
+        int v1, v2, v3;
+        v1 = VTRef.F1;
+        v2 = VTRef.F2;
+        v3 = VTRef.F3;
+        return Do(v1, v2);
+    }
+    
+    [MethodImpl(MethodImplOptions.NoInlining)]
+    unsafe static int TestByPtr(VT* pVT)
+    {
+        int v1, v2, v3;
+        v1 = pVT->F1;
+        v2 = pVT->F2;
+        v3 = pVT->F3;
+        return Do(v1, v2);
+    }
+    
+    unsafe static int Main(string[] args)
+    {
+        byte* pDataBytes = stackalloc byte[VT.Size];
+        VT* pVT = (VT*)pDataBytes;
+        pVT->F1 = 44;
+        pVT->F2 = 56;
+        pVT->F3 = 3;
+
+        int result = -200;
+        result += TestMethodInlining(pVT);
+        result += TestByRef(ref *pVT);
+        result += TestByPtr(pVT);
+
+        return result;
+    }
+    
+    [MethodImpl(MethodImplOptions.NoInlining)]
+    static int Do(int v1, int v2)
+    {
+        return v1 + v2;
+    }
+}
+
+

diff --git a/src/coreclr/tests/src/JIT/opt/Structs/structpop.csproj b/src/coreclr/tests/src/JIT/opt/Structs/structpop.csproj
new file mode 100644 (file)
index 0000000..c955a50
--- /dev/null
+++ b/src/coreclr/tests/src/JIT/opt/Structs/structpop.csproj
@@ -0,0 +1,39 @@
+<?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  .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>
+    <AllowUnsafeBlocks>True</AllowUnsafeBlocks>
+  </PropertyGroup>
+  <ItemGroup>
+    <Compile Include="structpop.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>
\ No newline at end of file

diff --git a/src/coreclr/tests/src/JIT/opt/Structs/structpop2.cs b/src/coreclr/tests/src/JIT/opt/Structs/structpop2.cs
new file mode 100644 (file)
index 0000000..6a3b270
--- /dev/null
+++ b/src/coreclr/tests/src/JIT/opt/Structs/structpop2.cs
@@ -0,0 +1,63 @@
+// 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.
+
+// Optimization of pop with struct types
+// Codegen for TestByPtr should be minimal
+//
+// See CoreClr#18710
+
+using System;
+using System.Runtime.CompilerServices;
+using System.Runtime.InteropServices;
+
+[StructLayout(LayoutKind.Sequential)]
+struct VT
+{
+    public static readonly int Size = Marshal.SizeOf<VT>();
+
+    public int F1, F2, F3, F4, F5, F6, F7, F8;
+}
+
+class P
+{
+    [MethodImpl(MethodImplOptions.NoInlining)]
+    static int Do(int v1)
+    {
+        Console.WriteLine("v1={0}", v1);
+        return v1;
+    }
+    
+    [MethodImpl(MethodImplOptions.NoInlining)]
+    unsafe static int TestByPtr(VT* pVT)
+    {
+        int v1, v2, v3, v4, v5, v6, v7, v8;
+        v1 = pVT->F1;
+        v2 = pVT->F2;
+        v3 = pVT->F3;
+        v4 = pVT->F4;
+        v5 = pVT->F5;
+        v6 = pVT->F6;
+        v7 = pVT->F7;
+        v8 = pVT->F8;
+        return Do(v1);
+    }
+    
+    unsafe static int Main(string[] args)
+    {
+        byte* pDataBytes = stackalloc byte[VT.Size];
+        VT* pVT = (VT*)pDataBytes;
+        pVT->F1 = 1;
+        pVT->F2 = 2;
+        pVT->F3 = 3;
+        pVT->F4 = 4;
+        pVT->F5 = 5;
+        pVT->F6 = 6;
+        pVT->F7 = 7;
+        pVT->F8 = 8;
+        int result = TestByPtr(pVT);
+        return result + 99;
+    }
+}
+
+

diff --git a/src/coreclr/tests/src/JIT/opt/Structs/structpop2.csproj b/src/coreclr/tests/src/JIT/opt/Structs/structpop2.csproj
new file mode 100644 (file)
index 0000000..37cc1ad
--- /dev/null
+++ b/src/coreclr/tests/src/JIT/opt/Structs/structpop2.csproj
@@ -0,0 +1,39 @@
+<?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  .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>
+    <AllowUnsafeBlocks>True</AllowUnsafeBlocks>
+  </PropertyGroup>
+  <ItemGroup>
+    <Compile Include="structpop2.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>
\ No newline at end of file