Skip to content

Commit

Permalink
Check that 'lvDoNotEnregister' is set as necessary. (#52802)
Browse files Browse the repository at this point in the history
* 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.
  • Loading branch information
Sergey Andreenko authored May 27, 2021
1 parent 62712ec commit edaa6d7
Show file tree
Hide file tree
Showing 8 changed files with 105 additions and 10 deletions.
1 change: 1 addition & 0 deletions src/coreclr/jit/compiler.h
Original file line number Diff line number Diff line change
Expand Up @@ -3433,6 +3433,7 @@ class Compiler
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
Expand Down
1 change: 1 addition & 0 deletions src/coreclr/jit/decomposelongs.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand Down
3 changes: 3 additions & 0 deletions src/coreclr/jit/lclvars.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
47 changes: 39 additions & 8 deletions src/coreclr/jit/lower.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down Expand Up @@ -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));
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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:
Expand All @@ -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;
}

Expand All @@ -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;
}
Expand Down
2 changes: 2 additions & 0 deletions src/coreclr/jit/morph.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand All @@ -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;
}
Expand Down
12 changes: 10 additions & 2 deletions src/coreclr/jit/rationalize.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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)
{
Expand All @@ -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;
Expand Down
36 changes: 36 additions & 0 deletions src/tests/JIT/Regression/JitBlue/Runtime_49780/Runtime_49780.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.
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();
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<OutputType>Exe</OutputType>
<CLRTestPriority>1</CLRTestPriority>
</PropertyGroup>
<PropertyGroup>
<DebugType>None</DebugType>
<Optimize>True</Optimize>
</PropertyGroup>
<ItemGroup>
<Compile Include="$(MSBuildProjectName).cs" />
</ItemGroup>
</Project>

0 comments on commit edaa6d7

Please sign in to comment.