Skip to content

Commit

Permalink
Fix lowering usage of an unset LSRA field. (#54731)
Browse files Browse the repository at this point in the history
* Add repro.

* fix the issue.

* delete a dead condition

* add a todo.

* Fix the failures.
  • Loading branch information
Sergey Andreenko committed Jun 28, 2021
1 parent 1c383a2 commit 94f3355
Show file tree
Hide file tree
Showing 7 changed files with 84 additions and 34 deletions.
2 changes: 2 additions & 0 deletions src/coreclr/jit/codegen.h
Original file line number Diff line number Diff line change
Expand Up @@ -863,8 +863,10 @@ XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX
// Generate code for a GT_BITCAST that is not contained.
void genCodeForBitCast(GenTreeOp* treeNode);

#if defined(TARGET_XARCH)
// Generate the instruction to move a value between register files
void genBitCast(var_types targetType, regNumber targetReg, var_types srcType, regNumber srcReg);
#endif // TARGET_XARCH

struct GenIntCastDesc
{
Expand Down
1 change: 1 addition & 0 deletions src/coreclr/jit/gentree.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -7396,6 +7396,7 @@ GenTree* Compiler::gtNewPutArgReg(var_types type, GenTree* arg, regNumber argReg
GenTree* Compiler::gtNewBitCastNode(var_types type, GenTree* arg)
{
assert(arg != nullptr);
assert(type != TYP_STRUCT);

GenTree* node = nullptr;
#if defined(TARGET_ARM)
Expand Down
1 change: 1 addition & 0 deletions src/coreclr/jit/lclvars.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3779,6 +3779,7 @@ var_types LclVarDsc::GetRegisterType(const GenTreeLclVarCommon* tree) const
{
if (lclVarType == TYP_STRUCT)
{
assert(!tree->OperIsLocalField() && "do not expect struct local fields.");
lclVarType = GetLayout()->GetRegisterType();
}
targetType = lclVarType;
Expand Down
68 changes: 34 additions & 34 deletions src/coreclr/jit/lower.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3073,10 +3073,11 @@ void Lowering::LowerStoreLocCommon(GenTreeLclVarCommon* lclStore)
DISPTREERANGE(BlockRange(), lclStore);
JITDUMP("\n");

GenTree* src = lclStore->gtGetOp1();
LclVarDsc* varDsc = comp->lvaGetDesc(lclStore);
bool srcIsMultiReg = src->IsMultiRegNode();
bool dstIsMultiReg = lclStore->IsMultiRegLclVar();
GenTree* src = lclStore->gtGetOp1();
LclVarDsc* varDsc = comp->lvaGetDesc(lclStore);

const bool srcIsMultiReg = src->IsMultiRegNode();
const bool dstIsMultiReg = lclStore->IsMultiRegLclVar();

if (!dstIsMultiReg && varTypeIsStruct(varDsc))
{
Expand All @@ -3098,25 +3099,7 @@ void Lowering::LowerStoreLocCommon(GenTreeLclVarCommon* lclStore)
}
}

if ((varTypeUsesFloatReg(lclStore) != varTypeUsesFloatReg(src)) && !lclStore->IsPhiDefn() &&
(src->TypeGet() != TYP_STRUCT))
{
if (m_lsra->isRegCandidate(varDsc))
{
GenTree* bitcast = comp->gtNewBitCastNode(lclStore->TypeGet(), src);
lclStore->gtOp1 = bitcast;
src = lclStore->gtGetOp1();
BlockRange().InsertBefore(lclStore, bitcast);
ContainCheckBitCast(bitcast);
}
else
{
// This is an actual store, we'll just retype it.
lclStore->gtType = src->TypeGet();
}
}

if (srcIsMultiReg || lclStore->IsMultiRegLclVar())
if (srcIsMultiReg || dstIsMultiReg)
{
const ReturnTypeDesc* retTypeDesc = nullptr;
if (src->OperIs(GT_CALL))
Expand All @@ -3125,22 +3108,24 @@ void Lowering::LowerStoreLocCommon(GenTreeLclVarCommon* lclStore)
}
CheckMultiRegLclVar(lclStore->AsLclVar(), retTypeDesc);
}

const var_types lclRegType = varDsc->GetRegisterType(lclStore);

if ((lclStore->TypeGet() == TYP_STRUCT) && !srcIsMultiReg)
{
bool convertToStoreObj;
if (src->OperGet() == GT_CALL)
{
GenTreeCall* call = src->AsCall();
const ClassLayout* layout = varDsc->GetLayout();
const var_types regType = layout->GetRegisterType();
GenTreeCall* call = src->AsCall();
const ClassLayout* layout = varDsc->GetLayout();

#ifdef DEBUG
const unsigned slotCount = layout->GetSlotCount();
#if defined(TARGET_XARCH) && !defined(UNIX_AMD64_ABI)
// Windows x64 doesn't have multireg returns,
// x86 uses it only for long return type, not for structs.
assert(slotCount == 1);
assert(regType != TYP_UNDEF);
assert(lclRegType != TYP_UNDEF);
#else // !TARGET_XARCH || UNIX_AMD64_ABI
if (!varDsc->lvIsHfa())
{
Expand All @@ -3153,15 +3138,15 @@ void Lowering::LowerStoreLocCommon(GenTreeLclVarCommon* lclStore)
unsigned size = layout->GetSize();
assert((size <= 8) || (size == 16));
bool isPowerOf2 = (((size - 1) & size) == 0);
bool isTypeDefined = (regType != TYP_UNDEF);
bool isTypeDefined = (lclRegType != TYP_UNDEF);
assert(isPowerOf2 == isTypeDefined);
}
}
#endif // !TARGET_XARCH || UNIX_AMD64_ABI
#endif // DEBUG

#if !defined(WINDOWS_AMD64_ABI)
if (!call->HasMultiRegRetVal() && (regType == TYP_UNDEF))
if (!call->HasMultiRegRetVal() && (lclRegType == TYP_UNDEF))
{
// If we have a single return register,
// but we can't retype it as a primitive type, we must spill it.
Expand All @@ -3182,9 +3167,8 @@ void Lowering::LowerStoreLocCommon(GenTreeLclVarCommon* lclStore)
else if (src->OperIs(GT_CNS_INT))
{
assert(src->IsIntegralConst(0) && "expected an INIT_VAL for non-zero init.");
var_types regType = varDsc->GetRegisterType();
#ifdef FEATURE_SIMD
if (varTypeIsSIMD(regType))
if (varTypeIsSIMD(lclRegType))
{
CorInfoType simdBaseJitType = comp->getBaseJitTypeOfSIMDLocal(lclStore);
if (simdBaseJitType == CORINFO_TYPE_UNDEF)
Expand All @@ -3193,7 +3177,7 @@ void Lowering::LowerStoreLocCommon(GenTreeLclVarCommon* lclStore)
simdBaseJitType = CORINFO_TYPE_FLOAT;
}
GenTreeSIMD* simdTree =
comp->gtNewSIMDNode(regType, src, SIMDIntrinsicInit, simdBaseJitType, varDsc->lvExactSize);
comp->gtNewSIMDNode(lclRegType, src, SIMDIntrinsicInit, simdBaseJitType, varDsc->lvExactSize);
BlockRange().InsertAfter(src, simdTree);
LowerSIMD(simdTree);
src = simdTree;
Expand Down Expand Up @@ -3245,6 +3229,20 @@ void Lowering::LowerStoreLocCommon(GenTreeLclVarCommon* lclStore)
}
}

// src and dst can be in registers, check if we need a bitcast.
if (!src->TypeIs(TYP_STRUCT) && (varTypeUsesFloatReg(lclRegType) != varTypeUsesFloatReg(src)))
{
assert(!srcIsMultiReg && !dstIsMultiReg);
assert(lclStore->OperIsLocalStore());
assert(lclRegType != TYP_UNDEF);

GenTree* bitcast = comp->gtNewBitCastNode(lclRegType, src);
lclStore->gtOp1 = bitcast;
src = lclStore->gtGetOp1();
BlockRange().InsertBefore(lclStore, bitcast);
ContainCheckBitCast(bitcast);
}

LowerStoreLoc(lclStore);
JITDUMP("lowering store lcl var/field (after):\n");
DISPTREERANGE(BlockRange(), lclStore);
Expand Down Expand Up @@ -6571,8 +6569,10 @@ void Lowering::ContainCheckBitCast(GenTree* node)
{
op1->SetContained();
}
LclVarDsc* varDsc = &comp->lvaTable[op1->AsLclVar()->GetLclNum()];
if (!m_lsra->isRegCandidate(varDsc))
const LclVarDsc* varDsc = comp->lvaGetDesc(op1->AsLclVar());
// TODO-Cleanup: we want to check if the local is already known not
// to be on reg, for example, because local enreg is disabled.
if (varDsc->lvDoNotEnregister)
{
op1->SetContained();
}
Expand Down
4 changes: 4 additions & 0 deletions src/coreclr/jit/lowerarmarch.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1549,6 +1549,9 @@ void Lowering::ContainCheckStoreLoc(GenTreeLclVarCommon* storeLoc) const
assert(storeLoc->OperIsLocalStore());
GenTree* op1 = storeLoc->gtGetOp1();

#if 0
// TODO-ARMARCH-CQ: support contained bitcast under STORE_LCL_VAR/FLD,
// currently codegen does not expect it.
if (op1->OperIs(GT_BITCAST))
{
// If we know that the source of the bitcast will be in a register, then we can make
Expand All @@ -1561,6 +1564,7 @@ void Lowering::ContainCheckStoreLoc(GenTreeLclVarCommon* storeLoc) const
return;
}
}
#endif

const LclVarDsc* varDsc = comp->lvaGetDesc(storeLoc);

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

namespace Runtime_54466
{
public class Test
{
static int Main()
{
return t(1, 1, 1, 1, Vector2.One, Vector2.One, Vector2.One, Vector2.One);
}


[MethodImplAttribute(MethodImplOptions.NoInlining)]
static int t(int a1, int a2, int a3, int a4, Vector2 x1, Vector2 x2, Vector2 x3, Vector2 x4)
{
if (x1 != Vector2.One)
{
Console.WriteLine("FAIL");
return 101;
}
return 100;
}
}

}


Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<OutputType>Exe</OutputType>
<DebugType>None</DebugType>
<Optimize>True</Optimize>
</PropertyGroup>
<ItemGroup>
<Compile Include="$(MSBuildProjectName).cs" />
</ItemGroup>
</Project>

0 comments on commit 94f3355

Please sign in to comment.