From: Sergey Andreenko Date: Thu, 27 May 2021 20:20:11 +0000 (-0700) Subject: Check that 'lvDoNotEnregister' is set as necessary. (#52802) X-Git-Tag: submit/tizen/20210909.063632~1117 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=edaa6d7e65ab3758088901d6758494dcd862c201;p=platform%2Fupstream%2Fdotnet%2Fruntime.git Check that 'lvDoNotEnregister' is set as necessary. (#52802) * add a repro test. * LclVar which addresses are taken should be marked as doNotEnreg. Check that we don't have independently promoted LCL_VAR that are references after lowering. Check that all LclVars that have ADDR() on top of them are marked as doNotEnreg. In the past when we did not enregister structs we were allocating them on the stack even without doNotEnreg set. --- diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index 2272aa1..9eb9843 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -3433,6 +3433,7 @@ public: DNER_NoRegVars, // opts.compFlags & CLFLG_REGVAR is not set DNER_MinOptsGC, // It is a GC Ref and we are compiling MinOpts #if !defined(TARGET_64BIT) + DNER_LongParamVar, // It is a long parameter. DNER_LongParamField, // It is a decomposed field of a long parameter. #endif #ifdef JIT32_GCENCODER diff --git a/src/coreclr/jit/decomposelongs.cpp b/src/coreclr/jit/decomposelongs.cpp index f72e0c4..f563b44 100644 --- a/src/coreclr/jit/decomposelongs.cpp +++ b/src/coreclr/jit/decomposelongs.cpp @@ -356,6 +356,7 @@ GenTree* DecomposeLongs::DecomposeLclVar(LIR::Use& use) } else { + m_compiler->lvaSetVarDoNotEnregister(varNum DEBUGARG(Compiler::DNER_LocalField)); loResult->SetOper(GT_LCL_FLD); loResult->AsLclFld()->SetLclOffs(0); loResult->AsLclFld()->SetFieldSeq(FieldSeqStore::NotAField()); diff --git a/src/coreclr/jit/lclvars.cpp b/src/coreclr/jit/lclvars.cpp index 16cf024..e517df3 100644 --- a/src/coreclr/jit/lclvars.cpp +++ b/src/coreclr/jit/lclvars.cpp @@ -2635,6 +2635,9 @@ void Compiler::lvaSetVarDoNotEnregister(unsigned varNum DEBUGARG(DoNotEnregister case DNER_LongParamField: JITDUMP("it is a decomposed field of a long parameter\n"); break; + case DNER_LongParamVar: + JITDUMP("it is a long parameter\n"); + break; #endif default: unreached(); diff --git a/src/coreclr/jit/lower.cpp b/src/coreclr/jit/lower.cpp index 242c371..559daa9 100644 --- a/src/coreclr/jit/lower.cpp +++ b/src/coreclr/jit/lower.cpp @@ -354,6 +354,16 @@ GenTree* Lowering::LowerNode(GenTree* node) node->gtGetOp1()->SetRegOptional(); break; + case GT_LCL_FLD_ADDR: + case GT_LCL_VAR_ADDR: + { + // TODO-Cleanup: this is definitely not the best place for this detection, + // but for now it is the easiest. Move it to morph. + const GenTreeLclVarCommon* lclAddr = node->AsLclVarCommon(); + comp->lvaSetVarDoNotEnregister(lclAddr->GetLclNum() DEBUGARG(Compiler::DNER_BlockOp)); + } + break; + default: break; } @@ -3208,7 +3218,9 @@ void Lowering::LowerStoreLocCommon(GenTreeLclVarCommon* lclStore) if (convertToStoreObj) { - GenTreeLclVar* addr = comp->gtNewLclVarAddrNode(lclStore->GetLclNum(), TYP_BYREF); + const unsigned lclNum = lclStore->GetLclNum(); + GenTreeLclVar* addr = comp->gtNewLclVarAddrNode(lclNum, TYP_BYREF); + comp->lvaSetVarDoNotEnregister(lclNum DEBUGARG(Compiler::DNER_BlockOp)); addr->gtFlags |= GTF_VAR_DEF; assert(!addr->IsPartialLclFld(comp)); @@ -3584,7 +3596,8 @@ void Lowering::LowerStoreSingleRegCallStruct(GenTreeBlk* store) GenTreeLclVar* Lowering::SpillStructCallResult(GenTreeCall* call) const { // TODO-1stClassStructs: we can support this in codegen for `GT_STORE_BLK` without new temps. - const unsigned spillNum = comp->lvaGrabTemp(true DEBUGARG("Return value temp for an odd struct return size")); + const unsigned spillNum = comp->lvaGrabTemp(true DEBUGARG("Return value temp for an odd struct return size")); + comp->lvaSetVarDoNotEnregister(spillNum DEBUGARG(Compiler::DNER_LocalField)); CORINFO_CLASS_HANDLE retClsHnd = call->gtRetClsHnd; comp->lvaSetStruct(spillNum, retClsHnd, false); GenTreeLclFld* spill = new (comp, GT_STORE_LCL_FLD) GenTreeLclFld(GT_STORE_LCL_FLD, call->gtType, spillNum, 0); @@ -6011,20 +6024,26 @@ void Lowering::CheckNode(Compiler* compiler, GenTree* node) case GT_HWINTRINSIC: assert(node->TypeGet() != TYP_SIMD12); break; -#ifdef TARGET_64BIT +#endif // FEATURE_SIMD + case GT_LCL_VAR: case GT_STORE_LCL_VAR: { - unsigned lclNum = node->AsLclVarCommon()->GetLclNum(); - LclVarDsc* lclVar = &compiler->lvaTable[lclNum]; + GenTreeLclVar* lclVar = node->AsLclVar(); + const unsigned lclNum = lclVar->GetLclNum(); + const LclVarDsc* varDsc = compiler->lvaGetDesc(lclNum); +#if defined(FEATURE_SIMD) && defined(TARGET_64BIT) if (node->TypeIs(TYP_SIMD12)) { - assert(compiler->lvaIsFieldOfDependentlyPromotedStruct(lclVar) || (lclVar->lvSize() == 12)); + assert(compiler->lvaIsFieldOfDependentlyPromotedStruct(varDsc) || (varDsc->lvSize() == 12)); + } +#endif // FEATURE_SIMD && TARGET_64BIT + if (varDsc->lvPromoted) + { + assert(varDsc->lvDoNotEnregister || varDsc->lvIsMultiRegRet); } } break; -#endif // TARGET_64BIT -#endif // SIMD case GT_LCL_VAR_ADDR: case GT_LCL_FLD_ADDR: @@ -6043,6 +6062,8 @@ void Lowering::CheckNode(Compiler* compiler, GenTree* node) assert(lclVarAddr->isContained() || !varDsc->lvTracked || varTypeIsStruct(varDsc)); // TODO: support this assert for uses, see https://github.com/dotnet/runtime/issues/51900. } + + assert(varDsc->lvDoNotEnregister); break; } @@ -6051,6 +6072,16 @@ void Lowering::CheckNode(Compiler* compiler, GenTree* node) assert(!"Should not see phi nodes after rationalize"); break; + case GT_LCL_FLD: + case GT_STORE_LCL_FLD: + { + GenTreeLclFld* lclFld = node->AsLclFld(); + const unsigned lclNum = lclFld->GetLclNum(); + const LclVarDsc* varDsc = compiler->lvaGetDesc(lclNum); + assert(varDsc->lvDoNotEnregister); + } + break; + default: break; } diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp index b6e1f80..546782c 100644 --- a/src/coreclr/jit/morph.cpp +++ b/src/coreclr/jit/morph.cpp @@ -3959,6 +3959,7 @@ GenTreeCall* Compiler::fgMorphArgs(GenTreeCall* call) { // Can't use the existing field's type, so use GT_LCL_FLD to swizzle // to a new type + lvaSetVarDoNotEnregister(lclNum DEBUGARG(DNER_LocalField)); argObj->ChangeOper(GT_LCL_FLD); argObj->gtType = structBaseType; } @@ -3983,6 +3984,7 @@ GenTreeCall* Compiler::fgMorphArgs(GenTreeCall* call) else if (genTypeSize(varDsc->TypeGet()) != genTypeSize(structBaseType)) { // Not a promoted struct, so just swizzle the type by using GT_LCL_FLD + lvaSetVarDoNotEnregister(lclNum DEBUGARG(DNER_LocalField)); argObj->ChangeOper(GT_LCL_FLD); argObj->gtType = structBaseType; } diff --git a/src/coreclr/jit/rationalize.cpp b/src/coreclr/jit/rationalize.cpp index 23aa85b..ea0e5fb 100644 --- a/src/coreclr/jit/rationalize.cpp +++ b/src/coreclr/jit/rationalize.cpp @@ -139,7 +139,11 @@ void Rationalizer::RewriteSIMDIndir(LIR::Use& use) // replace the expression by GT_LCL_VAR or GT_LCL_FLD. BlockRange().Remove(indir); - var_types lclType = comp->lvaGetDesc(addr->AsLclVar())->TypeGet(); + const GenTreeLclVar* lclAddr = addr->AsLclVar(); + const unsigned lclNum = lclAddr->GetLclNum(); + LclVarDsc* varDsc = comp->lvaGetDesc(lclNum); + + var_types lclType = varDsc->TypeGet(); if (lclType == simdType) { @@ -156,7 +160,11 @@ void Rationalizer::RewriteSIMDIndir(LIR::Use& use) addr->gtFlags |= GTF_VAR_USEASG; } - comp->lvaSetVarDoNotEnregister(addr->AsLclFld()->GetLclNum() DEBUGARG(Compiler::DNER_LocalField)); + comp->lvaSetVarDoNotEnregister(lclNum DEBUGARG(Compiler::DNER_LocalField)); + } + if (varDsc->lvPromotedStruct()) + { + comp->lvaSetVarDoNotEnregister(lclNum DEBUGARG(Compiler::DNER_IsStructArg)); } addr->gtType = simdType; diff --git a/src/tests/JIT/Regression/JitBlue/Runtime_49780/Runtime_49780.cs b/src/tests/JIT/Regression/JitBlue/Runtime_49780/Runtime_49780.cs new file mode 100644 index 0000000..e32fea4 --- /dev/null +++ b/src/tests/JIT/Regression/JitBlue/Runtime_49780/Runtime_49780.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. +using System; +using System.Runtime.CompilerServices; +using System.Numerics; +using System.Diagnostics; + +namespace Runtime_49489 +{ + public class Program + { + [MethodImpl(MethodImplOptions.NoInlining)] + static int Callee(float fltRef0, float fltRef1, float fltReg2, float fltReg3, float fltReg4, float fltReg5, float fltReg6, float fltReg7, Vector2 simd8) + { + const double eps = 1E-10; + Debug.Assert(Math.Abs(simd8.X) <= eps); + Debug.Assert(Math.Abs(simd8.Y - 1) <= eps); + return 100; + } + + [MethodImpl(MethodImplOptions.NoInlining)] + static int Caller() + { + Vector2 simd8; + simd8.X = 0; + simd8.Y = 1; + return Callee(0, 0, 0, 0, 0, 0, 0, 0, simd8); + + } + + static int Main(string[] args) + { + return Caller(); + } + } +} diff --git a/src/tests/JIT/Regression/JitBlue/Runtime_49780/Runtime_49780.csproj b/src/tests/JIT/Regression/JitBlue/Runtime_49780/Runtime_49780.csproj new file mode 100644 index 0000000..5d8fe22 --- /dev/null +++ b/src/tests/JIT/Regression/JitBlue/Runtime_49780/Runtime_49780.csproj @@ -0,0 +1,13 @@ + + + Exe + 1 + + + None + True + + + + +