Skip to content

Commit 94f3355

Browse files
author
Sergey Andreenko
authored
Fix lowering usage of an unset LSRA field. (#54731)
* Add repro. * fix the issue. * delete a dead condition * add a todo. * Fix the failures.
1 parent 1c383a2 commit 94f3355

File tree

7 files changed

+84
-34
lines changed

7 files changed

+84
-34
lines changed

‎src/coreclr/jit/codegen.h‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -863,8 +863,10 @@ XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX
863863
// Generate code for a GT_BITCAST that is not contained.
864864
void genCodeForBitCast(GenTreeOp* treeNode);
865865

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

869871
struct GenIntCastDesc
870872
{

‎src/coreclr/jit/gentree.cpp‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7396,6 +7396,7 @@ GenTree* Compiler::gtNewPutArgReg(var_types type, GenTree* arg, regNumber argReg
73967396
GenTree* Compiler::gtNewBitCastNode(var_types type, GenTree* arg)
73977397
{
73987398
assert(arg != nullptr);
7399+
assert(type != TYP_STRUCT);
73997400

74007401
GenTree* node = nullptr;
74017402
#if defined(TARGET_ARM)

‎src/coreclr/jit/lclvars.cpp‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3779,6 +3779,7 @@ var_types LclVarDsc::GetRegisterType(const GenTreeLclVarCommon* tree) const
37793779
{
37803780
if (lclVarType == TYP_STRUCT)
37813781
{
3782+
assert(!tree->OperIsLocalField() && "do not expect struct local fields.");
37823783
lclVarType = GetLayout()->GetRegisterType();
37833784
}
37843785
targetType = lclVarType;

‎src/coreclr/jit/lower.cpp‎

Lines changed: 34 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -3073,10 +3073,11 @@ void Lowering::LowerStoreLocCommon(GenTreeLclVarCommon* lclStore)
30733073
DISPTREERANGE(BlockRange(), lclStore);
30743074
JITDUMP("\n");
30753075

3076-
GenTree* src = lclStore->gtGetOp1();
3077-
LclVarDsc* varDsc = comp->lvaGetDesc(lclStore);
3078-
bool srcIsMultiReg = src->IsMultiRegNode();
3079-
bool dstIsMultiReg = lclStore->IsMultiRegLclVar();
3076+
GenTree* src = lclStore->gtGetOp1();
3077+
LclVarDsc* varDsc = comp->lvaGetDesc(lclStore);
3078+
3079+
const bool srcIsMultiReg = src->IsMultiRegNode();
3080+
const bool dstIsMultiReg = lclStore->IsMultiRegLclVar();
30803081

30813082
if (!dstIsMultiReg && varTypeIsStruct(varDsc))
30823083
{
@@ -3098,25 +3099,7 @@ void Lowering::LowerStoreLocCommon(GenTreeLclVarCommon* lclStore)
30983099
}
30993100
}
31003101

3101-
if ((varTypeUsesFloatReg(lclStore) != varTypeUsesFloatReg(src)) && !lclStore->IsPhiDefn() &&
3102-
(src->TypeGet() != TYP_STRUCT))
3103-
{
3104-
if (m_lsra->isRegCandidate(varDsc))
3105-
{
3106-
GenTree* bitcast = comp->gtNewBitCastNode(lclStore->TypeGet(), src);
3107-
lclStore->gtOp1 = bitcast;
3108-
src = lclStore->gtGetOp1();
3109-
BlockRange().InsertBefore(lclStore, bitcast);
3110-
ContainCheckBitCast(bitcast);
3111-
}
3112-
else
3113-
{
3114-
// This is an actual store, we'll just retype it.
3115-
lclStore->gtType = src->TypeGet();
3116-
}
3117-
}
3118-
3119-
if (srcIsMultiReg || lclStore->IsMultiRegLclVar())
3102+
if (srcIsMultiReg || dstIsMultiReg)
31203103
{
31213104
const ReturnTypeDesc* retTypeDesc = nullptr;
31223105
if (src->OperIs(GT_CALL))
@@ -3125,22 +3108,24 @@ void Lowering::LowerStoreLocCommon(GenTreeLclVarCommon* lclStore)
31253108
}
31263109
CheckMultiRegLclVar(lclStore->AsLclVar(), retTypeDesc);
31273110
}
3111+
3112+
const var_types lclRegType = varDsc->GetRegisterType(lclStore);
3113+
31283114
if ((lclStore->TypeGet() == TYP_STRUCT) && !srcIsMultiReg)
31293115
{
31303116
bool convertToStoreObj;
31313117
if (src->OperGet() == GT_CALL)
31323118
{
3133-
GenTreeCall* call = src->AsCall();
3134-
const ClassLayout* layout = varDsc->GetLayout();
3135-
const var_types regType = layout->GetRegisterType();
3119+
GenTreeCall* call = src->AsCall();
3120+
const ClassLayout* layout = varDsc->GetLayout();
31363121

31373122
#ifdef DEBUG
31383123
const unsigned slotCount = layout->GetSlotCount();
31393124
#if defined(TARGET_XARCH) && !defined(UNIX_AMD64_ABI)
31403125
// Windows x64 doesn't have multireg returns,
31413126
// x86 uses it only for long return type, not for structs.
31423127
assert(slotCount == 1);
3143-
assert(regType != TYP_UNDEF);
3128+
assert(lclRegType != TYP_UNDEF);
31443129
#else // !TARGET_XARCH || UNIX_AMD64_ABI
31453130
if (!varDsc->lvIsHfa())
31463131
{
@@ -3153,15 +3138,15 @@ void Lowering::LowerStoreLocCommon(GenTreeLclVarCommon* lclStore)
31533138
unsigned size = layout->GetSize();
31543139
assert((size <= 8) || (size == 16));
31553140
bool isPowerOf2 = (((size - 1) & size) == 0);
3156-
bool isTypeDefined = (regType != TYP_UNDEF);
3141+
bool isTypeDefined = (lclRegType != TYP_UNDEF);
31573142
assert(isPowerOf2 == isTypeDefined);
31583143
}
31593144
}
31603145
#endif // !TARGET_XARCH || UNIX_AMD64_ABI
31613146
#endif // DEBUG
31623147

31633148
#if !defined(WINDOWS_AMD64_ABI)
3164-
if (!call->HasMultiRegRetVal() && (regType == TYP_UNDEF))
3149+
if (!call->HasMultiRegRetVal() && (lclRegType == TYP_UNDEF))
31653150
{
31663151
// If we have a single return register,
31673152
// but we can't retype it as a primitive type, we must spill it.
@@ -3182,9 +3167,8 @@ void Lowering::LowerStoreLocCommon(GenTreeLclVarCommon* lclStore)
31823167
else if (src->OperIs(GT_CNS_INT))
31833168
{
31843169
assert(src->IsIntegralConst(0) && "expected an INIT_VAL for non-zero init.");
3185-
var_types regType = varDsc->GetRegisterType();
31863170
#ifdef FEATURE_SIMD
3187-
if (varTypeIsSIMD(regType))
3171+
if (varTypeIsSIMD(lclRegType))
31883172
{
31893173
CorInfoType simdBaseJitType = comp->getBaseJitTypeOfSIMDLocal(lclStore);
31903174
if (simdBaseJitType == CORINFO_TYPE_UNDEF)
@@ -3193,7 +3177,7 @@ void Lowering::LowerStoreLocCommon(GenTreeLclVarCommon* lclStore)
31933177
simdBaseJitType = CORINFO_TYPE_FLOAT;
31943178
}
31953179
GenTreeSIMD* simdTree =
3196-
comp->gtNewSIMDNode(regType, src, SIMDIntrinsicInit, simdBaseJitType, varDsc->lvExactSize);
3180+
comp->gtNewSIMDNode(lclRegType, src, SIMDIntrinsicInit, simdBaseJitType, varDsc->lvExactSize);
31973181
BlockRange().InsertAfter(src, simdTree);
31983182
LowerSIMD(simdTree);
31993183
src = simdTree;
@@ -3245,6 +3229,20 @@ void Lowering::LowerStoreLocCommon(GenTreeLclVarCommon* lclStore)
32453229
}
32463230
}
32473231

3232+
// src and dst can be in registers, check if we need a bitcast.
3233+
if (!src->TypeIs(TYP_STRUCT) && (varTypeUsesFloatReg(lclRegType) != varTypeUsesFloatReg(src)))
3234+
{
3235+
assert(!srcIsMultiReg && !dstIsMultiReg);
3236+
assert(lclStore->OperIsLocalStore());
3237+
assert(lclRegType != TYP_UNDEF);
3238+
3239+
GenTree* bitcast = comp->gtNewBitCastNode(lclRegType, src);
3240+
lclStore->gtOp1 = bitcast;
3241+
src = lclStore->gtGetOp1();
3242+
BlockRange().InsertBefore(lclStore, bitcast);
3243+
ContainCheckBitCast(bitcast);
3244+
}
3245+
32483246
LowerStoreLoc(lclStore);
32493247
JITDUMP("lowering store lcl var/field (after):\n");
32503248
DISPTREERANGE(BlockRange(), lclStore);
@@ -6571,8 +6569,10 @@ void Lowering::ContainCheckBitCast(GenTree* node)
65716569
{
65726570
op1->SetContained();
65736571
}
6574-
LclVarDsc* varDsc = &comp->lvaTable[op1->AsLclVar()->GetLclNum()];
6575-
if (!m_lsra->isRegCandidate(varDsc))
6572+
const LclVarDsc* varDsc = comp->lvaGetDesc(op1->AsLclVar());
6573+
// TODO-Cleanup: we want to check if the local is already known not
6574+
// to be on reg, for example, because local enreg is disabled.
6575+
if (varDsc->lvDoNotEnregister)
65766576
{
65776577
op1->SetContained();
65786578
}

‎src/coreclr/jit/lowerarmarch.cpp‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1549,6 +1549,9 @@ void Lowering::ContainCheckStoreLoc(GenTreeLclVarCommon* storeLoc) const
15491549
assert(storeLoc->OperIsLocalStore());
15501550
GenTree* op1 = storeLoc->gtGetOp1();
15511551

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

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

‎src/tests/JIT/Regression/JitBlue/Runtime_54466/Runtime_54466.cs‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
// Licensed to the .NET Foundation under one or more agreements.
2+
// The .NET Foundation licenses this file to you under the MIT license.
3+
4+
using System;
5+
using System.Numerics;
6+
using System.Runtime.CompilerServices;
7+
8+
namespace Runtime_54466
9+
{
10+
public class Test
11+
{
12+
static int Main()
13+
{
14+
return t(1, 1, 1, 1, Vector2.One, Vector2.One, Vector2.One, Vector2.One);
15+
}
16+
17+
18+
[MethodImplAttribute(MethodImplOptions.NoInlining)]
19+
static int t(int a1, int a2, int a3, int a4, Vector2 x1, Vector2 x2, Vector2 x3, Vector2 x4)
20+
{
21+
if (x1 != Vector2.One)
22+
{
23+
Console.WriteLine("FAIL");
24+
return 101;
25+
}
26+
return 100;
27+
}
28+
}
29+
30+
}
31+
32+

‎src/tests/JIT/Regression/JitBlue/Runtime_54466/Runtime_54466.csproj‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
<Project Sdk="Microsoft.NET.Sdk">
2+
<PropertyGroup>
3+
<OutputType>Exe</OutputType>
4+
<DebugType>None</DebugType>
5+
<Optimize>True</Optimize>
6+
</PropertyGroup>
7+
<ItemGroup>
8+
<Compile Include="$(MSBuildProjectName).cs" />
9+
</ItemGroup>
10+
</Project>

0 commit comments

Comments
 (0)