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

Commit

Permalink
Don't map P-DEP SIMD12 local vars to SIMD16 on x64
Browse files Browse the repository at this point in the history
P-DEP local vars are logically independent locals, but physically embeded
in some structure with fixed layout. So they cannot be made larger.

We already had safeguards for ths in place for x86 so extend these to kick
in for x64 too.

Also update Lowering's checker to account for the fact that some SIMD12s
can persist in x64.

Fixes #12950.
  • Loading branch information
AndyAyersMS authored and RussKeldorph committed Aug 18, 2017
1 parent d99bf2d commit 0803236
Show file tree
Hide file tree
Showing 5 changed files with 90 additions and 12 deletions.
9 changes: 3 additions & 6 deletions src/jit/compiler.h
Original file line number Diff line number Diff line change
Expand Up @@ -2744,12 +2744,11 @@ class Compiler

#if defined(_TARGET_64BIT_)
assert(varDsc->lvSize() == 16);
return true;
#else // !defined(_TARGET_64BIT_)
#endif // defined(_TARGET_64BIT_)

// For 32-bit architectures, we make local variable SIMD12 types 16 bytes instead of just 12. lvSize()
// We make local variable SIMD12 types 16 bytes instead of just 12. lvSize()
// already does this calculation. However, we also need to prevent mapping types if the var is a
// depenendently promoted struct field, which must remain its exact size within its parent struct.
// dependently promoted struct field, which must remain its exact size within its parent struct.
// However, we don't know this until late, so we may have already pretended the field is bigger
// before that.
if ((varDsc->lvSize() == 16) && !lvaIsFieldOfDependentlyPromotedStruct(varDsc))
Expand All @@ -2760,8 +2759,6 @@ class Compiler
{
return false;
}

#endif // !defined(_TARGET_64BIT_)
}
#endif // defined(FEATURE_SIMD)

Expand Down
17 changes: 12 additions & 5 deletions src/jit/lower.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4656,9 +4656,10 @@ void Lowering::CheckCall(GenTreeCall* call)
// after lowering.
//
// Arguments:
// compiler - the compiler context.
// node - the node to check.
//
void Lowering::CheckNode(GenTree* node)
void Lowering::CheckNode(Compiler* compiler, GenTree* node)
{
switch (node->OperGet())
{
Expand All @@ -4668,13 +4669,19 @@ void Lowering::CheckNode(GenTree* node)

#ifdef FEATURE_SIMD
case GT_SIMD:
assert(node->TypeGet() != TYP_SIMD12);
break;
#ifdef _TARGET_64BIT_
case GT_LCL_VAR:
case GT_STORE_LCL_VAR:
{
unsigned lclNum = node->AsLclVarCommon()->GetLclNum();
LclVarDsc* lclVar = &compiler->lvaTable[lclNum];
assert(node->TypeGet() != TYP_SIMD12 || compiler->lvaIsFieldOfDependentlyPromotedStruct(lclVar));
}
break;
#endif // _TARGET_64BIT_
assert(node->TypeGet() != TYP_SIMD12);
break;
#endif
#endif // SIMD

default:
break;
Expand All @@ -4696,7 +4703,7 @@ bool Lowering::CheckBlock(Compiler* compiler, BasicBlock* block)
LIR::Range& blockRange = LIR::AsRange(block);
for (GenTree* node : blockRange)
{
CheckNode(node);
CheckNode(compiler, node);
}

assert(blockRange.CheckLIR(compiler));
Expand Down
2 changes: 1 addition & 1 deletion src/jit/lower.h
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ class Lowering : public Phase
#ifdef DEBUG
static void CheckCallArg(GenTree* arg);
static void CheckCall(GenTreeCall* call);
static void CheckNode(GenTree* node);
static void CheckNode(Compiler* compiler, GenTree* node);
static bool CheckBlock(Compiler* compiler, BasicBlock* block);
#endif // DEBUG

Expand Down
36 changes: 36 additions & 0 deletions tests/src/JIT/Regression/JitBlue/GitHub_12950/GitHub_12950.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
// 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.Numerics;

class Program
{
struct BoundingBoxTest
{
public Vector3 Min;
public Vector3 Max;

public override int GetHashCode()
{
return Min.GetHashCode() + Max.GetHashCode();
}
}

public static void Test()
{
var box = new BoundingBoxTest();
box.Min = Vector3.Min(box.Min, box.Min);
var hmm = box.GetHashCode();
}

static int Main(string[] args)
{
var someMemory = new int[1];
var someMoreMemory = new int[1];
Test();
someMoreMemory[someMemory[0]] = 100;
return someMoreMemory[0];
}
}
38 changes: 38 additions & 0 deletions tests/src/JIT/Regression/JitBlue/GitHub_12950/GitHub_12950.csproj
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
<?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 0803236

Please sign in to comment.