Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Check that 'lvDoNotEnregister' is set as necessary. #52802

Merged
merged 2 commits into from
May 27, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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>