From 0803236057635e94bd2dc000897b4b157e9ef75c Mon Sep 17 00:00:00 2001 From: Andy Ayers Date: Mon, 24 Jul 2017 07:54:44 -0700 Subject: [PATCH] Don't map P-DEP SIMD12 local vars to SIMD16 on x64 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. --- src/jit/compiler.h | 9 ++--- src/jit/lower.cpp | 17 ++++++--- src/jit/lower.h | 2 +- .../JitBlue/GitHub_12950/GitHub_12950.cs | 36 ++++++++++++++++++ .../JitBlue/GitHub_12950/GitHub_12950.csproj | 38 +++++++++++++++++++ 5 files changed, 90 insertions(+), 12 deletions(-) create mode 100644 tests/src/JIT/Regression/JitBlue/GitHub_12950/GitHub_12950.cs create mode 100644 tests/src/JIT/Regression/JitBlue/GitHub_12950/GitHub_12950.csproj diff --git a/src/jit/compiler.h b/src/jit/compiler.h index 5bff8ddc1f38..e1d9e6551b4f 100644 --- a/src/jit/compiler.h +++ b/src/jit/compiler.h @@ -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)) @@ -2760,8 +2759,6 @@ class Compiler { return false; } - -#endif // !defined(_TARGET_64BIT_) } #endif // defined(FEATURE_SIMD) diff --git a/src/jit/lower.cpp b/src/jit/lower.cpp index 9cab5eecf23b..7df96e35dd01 100644 --- a/src/jit/lower.cpp +++ b/src/jit/lower.cpp @@ -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()) { @@ -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; @@ -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)); diff --git a/src/jit/lower.h b/src/jit/lower.h index 5a55d2d69f2e..c5d8c2fdf7f0 100644 --- a/src/jit/lower.h +++ b/src/jit/lower.h @@ -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 diff --git a/tests/src/JIT/Regression/JitBlue/GitHub_12950/GitHub_12950.cs b/tests/src/JIT/Regression/JitBlue/GitHub_12950/GitHub_12950.cs new file mode 100644 index 000000000000..73abf003a1ba --- /dev/null +++ b/tests/src/JIT/Regression/JitBlue/GitHub_12950/GitHub_12950.cs @@ -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]; + } +} diff --git a/tests/src/JIT/Regression/JitBlue/GitHub_12950/GitHub_12950.csproj b/tests/src/JIT/Regression/JitBlue/GitHub_12950/GitHub_12950.csproj new file mode 100644 index 000000000000..2cafa4cedf5d --- /dev/null +++ b/tests/src/JIT/Regression/JitBlue/GitHub_12950/GitHub_12950.csproj @@ -0,0 +1,38 @@ + + + + + Debug + AnyCPU + $(MSBuildProjectName) + 2.0 + {95DFC527-4DC1-495E-97D7-E94EE1F7140D} + Exe + {786C830F-07A1-408B-BD7F-6EE04809D6DB};{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC} + ..\..\ + + + + + + + + + + False + + + + + True + + + + + + + + + + +