Skip to content

Commit

Permalink
JIT: Fix liveness for partial defs of parent locals (#85654)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
jakobbotsch authored May 3, 2023
1 parent fc22ee9 commit edd6d63
Showing 1 changed file with 12 additions and 17 deletions.
29 changes: 12 additions & 17 deletions src/coreclr/jit/liveness.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -93,28 +93,23 @@ 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)
{
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);
}
}
}
}
Expand Down

0 comments on commit edd6d63

Please sign in to comment.