From f312bd2fe7b16f3bbd3f65a53c7e0f727b8a397f Mon Sep 17 00:00:00 2001 From: Jakob Botsch Nielsen Date: Tue, 2 May 2023 17:36:48 +0200 Subject: [PATCH 1/2] JIT: Fix liveness for partial defs of parent locals MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Partial defs in liveness are modelled as full uses of all fields and then a full def of the entire local. The logic that handled fields directly got that right, but the logic that handled parent locals did not. For example, for IR like ``` ------------ BB18 [045..046), preds={BB16} succs={BB19} ***** BB18 STMT00096 ( INL10 @ 0x01F[E-] ... ??? ) <- INL04 @ ??? <- INLRT @ 0x045[E-] N003 ( 5, 4) [000403] -A------R-- ▌ ASG byref N002 ( 3, 2) [000402] D------N--- ├──▌ LCL_VAR byref V73 tmp45 N001 ( 1, 1) [000401] ----------- └──▌ LCL_VAR long V43 tmp15 ***** BB18 STMT00097 ( INL10 @ 0x026[E-] ... ??? ) <- INL04 @ ??? <- INLRT @ 0x045[E-] N003 ( 5, 4) [000407] -A------R-- ▌ ASG int N002 ( 3, 2) [000406] D------N--- ├──▌ LCL_VAR int V74 tmp46 N001 ( 1, 1) [000405] ----------- └──▌ LCL_VAR int V42 tmp14 ***** BB18 STMT00072 ( INL04 @ 0x073[--] ... ??? ) <- INLRT @ 0x045[E-] N007 ( 14, 14) [000627] -A--------- ▌ COMMA void N003 ( 7, 7) [000623] -A------R-- ├──▌ ASG byref N002 ( 3, 4) [000621] U------N--- │ ├──▌ LCL_FLD byref (P) V12 loc3 [+16] │ ├──▌ ref field V12._managedArray (fldOffset=0x0) -> V57 tmp29 │ ├──▌ long field V12._allocatedMemory (fldOffset=0x8) -> V58 tmp30 │ ├──▌ byref field V12._reference (fldOffset=0x10) -> V59 tmp31 │ ├──▌ int field V12._length (fldOffset=0x18) -> V60 tmp32 N001 ( 3, 2) [000622] ----------- │ └──▌ LCL_VAR byref V73 tmp45 N006 ( 7, 7) [000626] -A------R-- └──▌ ASG int N005 ( 3, 4) [000624] U------N--- ├──▌ LCL_FLD int (P) V12 loc3 [+24] ├──▌ ref field V12._managedArray (fldOffset=0x0) -> V57 tmp29 ├──▌ long field V12._allocatedMemory (fldOffset=0x8) -> V58 tmp30 ├──▌ byref field V12._reference (fldOffset=0x10) -> V59 tmp31 ├──▌ int field V12._length (fldOffset=0x18) -> V60 tmp32 N004 ( 3, 2) [000625] ----------- └──▌ LCL_VAR int V74 tmp46 ``` we would see ``` BB18 USE(6)={V58 V57 V59 V60 V42 V43 } DEF(2)={ V73 V74} ``` which is obviously incorrect as V57-V60 are all defined under this model. This would lead to an assert in SSA since SSA did treat this as a def. --- src/coreclr/jit/liveness.cpp | 27 ++++++++++++--------------- 1 file changed, 12 insertions(+), 15 deletions(-) diff --git a/src/coreclr/jit/liveness.cpp b/src/coreclr/jit/liveness.cpp index 0e415743f561db..f9b766393d9a32 100644 --- a/src/coreclr/jit/liveness.cpp +++ b/src/coreclr/jit/liveness.cpp @@ -97,24 +97,21 @@ void Compiler::fgMarkUseDef(GenTreeLclVarCommon* tree) for (unsigned i = varDsc->lvFieldLclStart; i < varDsc->lvFieldLclStart + varDsc->lvFieldCnt; ++i) { - noway_assert(lvaTable[i].lvIsStructField); - if (lvaTable[i].lvTracked) + if (!lvaTable[i].lvTracked) { - noway_assert(lvaTable[i].lvVarIndex < lvaTrackedCount); - VarSetOps::AddElemD(this, bitMask, lvaTable[i].lvVarIndex); + continue; } - } - // For pure defs (i.e. not an "update" def which is also a use), add to the (all) def set. - if (!isUse) - { - assert(isDef); - VarSetOps::UnionD(this, fgCurDefSet, bitMask); - } - else if (!VarSetOps::IsSubset(this, bitMask, fgCurDefSet)) - { - // Mark as used any struct fields that are not yet defined. - VarSetOps::UnionD(this, fgCurUseSet, bitMask); + unsigned varIndex = lvaTable[i].lvVarIndex; + if (isUse && !VarSetOps::IsMember(this, fgCurDefSet, varIndex)) + { + VarSetOps::AddElemD(this, fgCurUseSet, varIndex); + } + + if (isDef) + { + VarSetOps::AddElemD(this, fgCurDefSet, varIndex); + } } } } From e54e4b0dca40ef067306ca92cc4452cbed2c8053 Mon Sep 17 00:00:00 2001 From: Jakob Botsch Nielsen Date: Wed, 3 May 2023 10:40:00 +0200 Subject: [PATCH 2/2] Remove unused variable --- src/coreclr/jit/liveness.cpp | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/coreclr/jit/liveness.cpp b/src/coreclr/jit/liveness.cpp index f9b766393d9a32..f2c31907d4dab9 100644 --- a/src/coreclr/jit/liveness.cpp +++ b/src/coreclr/jit/liveness.cpp @@ -93,8 +93,6 @@ void Compiler::fgMarkUseDef(GenTreeLclVarCommon* tree) if (promotionType != PROMOTION_TYPE_NONE) { - VARSET_TP bitMask(VarSetOps::MakeEmpty(this)); - for (unsigned i = varDsc->lvFieldLclStart; i < varDsc->lvFieldLclStart + varDsc->lvFieldCnt; ++i) { if (!lvaTable[i].lvTracked)