Do not treat certain calls as intrinsics for RyuJIT/x86.
authorPat Gavlin <pagavlin@microsoft.com>
Mon, 21 Nov 2016 21:25:49 +0000 (13:25 -0800)
committerPat Gavlin <pagavlin@microsoft.com>
Mon, 21 Nov 2016 21:25:49 +0000 (13:25 -0800)
On x86 RyuJIT, importing intrinsics that are implemented as user calls
can cause incorrect calculation of the depth of the stack if these
intrinsics are used as arguments to another call. This causes bad code
generation for certain EH constructs. Instead of implementing these
intrinsics as intrinsics, simply import them as calls.

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

src/coreclr/src/jit/importer.cpp
src/coreclr/tests/src/JIT/Regression/JitBlue/DevDiv_280127/DevDiv_280127.cs [new file with mode: 0644]
src/coreclr/tests/src/JIT/Regression/JitBlue/DevDiv_280127/DevDiv_280127.csproj [new file with mode: 0644]

index 1802125..46ecf03 100644 (file)
@@ -3344,9 +3344,9 @@ GenTreePtr Compiler::impIntrinsic(CORINFO_CLASS_HANDLE  clsHnd,
 
             op1 = nullptr;
 
-#ifdef LEGACY_BACKEND
+#if defined(LEGACY_BACKEND)
             if (IsTargetIntrinsic(intrinsicID))
-#else
+#elif !defined(_TARGET_X86_)
             // Intrinsics that are not implemented directly by target instructions will
             // be re-materialized as users calls in rationalizer. For prefixed tail calls,
             // don't do this optimization, because
@@ -3354,6 +3354,11 @@ GenTreePtr Compiler::impIntrinsic(CORINFO_CLASS_HANDLE  clsHnd,
             //  b) It will be non-trivial task or too late to re-materialize a surviving
             //     tail prefixed GT_INTRINSIC as tail call in rationalizer.
             if (!IsIntrinsicImplementedByUserCall(intrinsicID) || !tailCall)
+#else
+            // On x86 RyuJIT, importing intrinsics that are implemented as user calls can cause incorrect calculation
+            // of the depth of the stack if these intrinsics are used as arguments to another call. This causes bad
+            // code generation for certain EH constructs.
+            if (!IsIntrinsicImplementedByUserCall(intrinsicID))
 #endif
             {
                 switch (sig->numArgs)
diff --git a/src/coreclr/tests/src/JIT/Regression/JitBlue/DevDiv_280127/DevDiv_280127.cs b/src/coreclr/tests/src/JIT/Regression/JitBlue/DevDiv_280127/DevDiv_280127.cs
new file mode 100644 (file)
index 0000000..3f8270f
--- /dev/null
@@ -0,0 +1,37 @@
+// 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;
+
+// The original repro for this test was an assertion after code generation that the actual maximum depth of the stack
+// was less than or identical to the estimated depth of the stack as calculated during morph. The calculation was
+// incorrect when a math intrinsic was used as an argument to a function with on-stack parameters (e.g. the call to
+// `M` on line 18).
+
+static class C
+{
+    struct S
+    {
+        int a, b, c, d;
+    }
+
+    [MethodImpl(MethodImplOptions.NoInlining)]
+    static int N(S s, float d)
+    {
+        return 100;
+    }
+
+    [MethodImpl(MethodImplOptions.NoInlining)]
+    static int M(double d)
+    {
+        N(new S(), (float)(Math.Atan2(d, 2.0) * 180 / Math.PI));
+        return 100;
+    }
+
+    static int Main()
+    {
+        return M(2.0);
+    }
+}
diff --git a/src/coreclr/tests/src/JIT/Regression/JitBlue/DevDiv_280127/DevDiv_280127.csproj b/src/coreclr/tests/src/JIT/Regression/JitBlue/DevDiv_280127/DevDiv_280127.csproj
new file mode 100644 (file)
index 0000000..ec9776d
--- /dev/null
@@ -0,0 +1,46 @@
+<?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></DebugType>
+    <Optimize>True</Optimize>
+  </PropertyGroup>
+  <ItemGroup>
+    <Compile Include="DevDiv_280127.cs" />
+  </ItemGroup>
+  <ItemGroup>
+    <Service Include="{82A7F48D-3B50-4B1E-B82E-3ADA8210C358}" />
+  </ItemGroup>
+  <PropertyGroup>
+    <ProjectJson>$(JitPackagesConfigFileDirectory)minimal\project.json</ProjectJson>
+    <ProjectLockJson>$(JitPackagesConfigFileDirectory)minimal\project.lock.json</ProjectLockJson>
+  </PropertyGroup>
+  <Import Project="$([MSBuild]::GetDirectoryNameOfFileAbove($(MSBuildThisFileDirectory), dir.targets))\dir.targets" />
+  <PropertyGroup Condition=" '$(MsBuildProjectDirOverride)' != '' ">
+  </PropertyGroup> 
+</Project>