Skip to content
This repository has been archived by the owner on Jan 23, 2023. It is now read-only.

Commit

Permalink
Handle SIMD8/LONG recasts for LCL_FLD
Browse files Browse the repository at this point in the history
Lowering needs to insert a `BITCAST` in the case of a `STORE_LCL_FLD` with mismatched `TYP_SIMD8`/`TYP_LONG` operands, just as for `STORE_LCL_VAR`.
  • Loading branch information
CarolEidt committed Apr 4, 2018
1 parent 757cb82 commit 2aeecbf
Show file tree
Hide file tree
Showing 3 changed files with 92 additions and 6 deletions.
12 changes: 6 additions & 6 deletions src/jit/lower.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -276,8 +276,12 @@ GenTree* Lowering::LowerNode(GenTree* node)
break;

case GT_STORE_LCL_VAR:
#if defined(_TARGET_AMD64_) && defined(FEATURE_SIMD)
WidenSIMD12IfNecessary(node->AsLclVarCommon());
__fallthrough;

case GT_STORE_LCL_FLD:
{
#if defined(_TARGET_AMD64_) && defined(FEATURE_SIMD)
GenTreeLclVarCommon* const store = node->AsLclVarCommon();
if ((store->TypeGet() == TYP_SIMD8) != (store->gtOp1->TypeGet() == TYP_SIMD8))
{
Expand All @@ -286,12 +290,7 @@ GenTree* Lowering::LowerNode(GenTree* node)
store->gtOp1 = bitcast;
BlockRange().InsertBefore(store, bitcast);
}
}
#endif // _TARGET_AMD64_
WidenSIMD12IfNecessary(node->AsLclVarCommon());
__fallthrough;

case GT_STORE_LCL_FLD:
// TODO-1stClassStructs: Once we remove the requirement that all struct stores
// are block stores (GT_STORE_BLK or GT_STORE_OBJ), here is where we would put the local
// store under a block store if codegen will require it.
Expand All @@ -306,6 +305,7 @@ GenTree* Lowering::LowerNode(GenTree* node)
}
LowerStoreLoc(node->AsLclVarCommon());
break;
}

#ifdef _TARGET_ARM64_
case GT_CMPXCHG:
Expand Down
49 changes: 49 additions & 0 deletions tests/src/JIT/Regression/JitBlue/DevDiv_590358/DevDiv_590358.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
using System;
using System.Numerics;

// In this test case, we have a struct S that contains a single Vector2 field (Vector),
// with an implicit conversion from an array of float to S.
// We inline a call to this op_Implicit, and then the constructor that it invokes.
// The op_Implicit RET_EXPR is TYP_LONG, since that's how the value is returned from the method.
// It is then stored to the Vector field which is TYP_SIMD8.
//
// Bug: The JIT had code to deal with this kind of retyping (which must be made explicit by
// Lowering, as the two types require different register types), when it involves locals
// (GT_LCL_VAR) but not fields of locals (GT_LCL_FLD).
//
// Perf Issue: What we wind up with is this: (V05 & V07 are both TYP_SIMD8, V01 is the struct type S
// V05 = *(TYP_SIMD8)numbers
// V07 = V05
// LCL_FLD(V01) = LCL_FLD(V07) // Both re-typed as TYP_LONG
// This generates:
// vmovsd xmm0, qword ptr [rax+16]
// vmovsd qword ptr[rsp + 28H], xmm0
// vmovsd xmm0, qword ptr[rsp + 28H]
// vmovsd qword ptr[rsp + 20H], xmm0
// vmovsd xmm0, qword ptr[rsp + 20H]
// vmovsd qword ptr[rsp + 30H], xmm0
//
// We should be able to elide these excessive copies and unnecessary retyping, producing close to this:
// vmovsd xmm0, qword ptr [rax+16]
// vmovsd qword ptr[rsp + 30H], xmm0

namespace Repro
{
class Program
{
struct S
{
public Vector2 Vector;
public S(float[] numbers)
{
Vector = new Vector2(numbers[0], numbers[1]);
}
public static implicit operator S(float[] numbers) => new S(numbers);
}
static void Main(string[] args)
{
S s = new float[] { 1.0f, 2.0f };
Console.WriteLine(s.Vector);
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
<?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></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>

0 comments on commit 2aeecbf

Please sign in to comment.