Skip to content

Commit

Permalink
JIT: Avoid removing multi-use boxes
Browse files Browse the repository at this point in the history
There was an assumption that the local operand to a GT_BOX node is
single-use which was being violated when GDV clones these as arguments.
We now allow multi-uses of these locals by setting a flag when cloning
and then handle it in gtTryRemoveBoxUpstreamEffects.

There is also GTF_VAR_CLONED that would be set on the local itself, but
given that transformations can affect the operand local node arbitrarily
I went with another of these type of flags on GT_BOX instead.

Fix #72775
  • Loading branch information
jakobbotsch authored and github-actions committed Jul 28, 2022
1 parent 00f82ac commit 9588349
Show file tree
Hide file tree
Showing 3 changed files with 25 additions and 0 deletions.
5 changes: 5 additions & 0 deletions src/coreclr/jit/compiler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -9175,6 +9175,11 @@ void cTreeFlags(Compiler* comp, GenTree* tree)
{
chars += printf("[BOX_VALUE]");
}

if (tree->gtFlags & GTF_BOX_CLONED)
{
chars += printf("[BOX_CLONED]");
}
break;

case GT_CNS_INT:
Expand Down
9 changes: 9 additions & 0 deletions src/coreclr/jit/gentree.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -7935,6 +7935,8 @@ GenTree* Compiler::gtCloneExpr(
copy = new (this, GT_BOX)
GenTreeBox(tree->TypeGet(), tree->AsOp()->gtOp1, tree->AsBox()->gtAsgStmtWhenInlinedBoxValue,
tree->AsBox()->gtCopyStmtWhenInlinedBoxValue);
tree->AsBox()->SetCloned();
copy->AsBox()->SetCloned();
break;

case GT_INTRINSIC:
Expand Down Expand Up @@ -13834,6 +13836,13 @@ GenTree* Compiler::gtTryRemoveBoxUpstreamEffects(GenTree* op, BoxRemovalOptions
return nullptr;
}

// If this box is no longer single-use, bail.
if (box->WasCloned())
{
JITDUMP(" bailing; unsafe to remove box that has been cloned\n");
return nullptr;
}

// If we're eventually going to return the type handle, remember it now.
GenTree* boxTypeHandle = nullptr;
if ((options == BR_REMOVE_AND_NARROW_WANT_TYPE_HANDLE) || (options == BR_DONT_REMOVE_WANT_TYPE_HANDLE))
Expand Down
11 changes: 11 additions & 0 deletions src/coreclr/jit/gentree.h
Original file line number Diff line number Diff line change
Expand Up @@ -543,6 +543,7 @@ enum GenTreeFlags : unsigned int
GTF_QMARK_CAST_INSTOF = 0x80000000, // GT_QMARK -- Is this a top (not nested) level qmark created for
// castclass or instanceof?

GTF_BOX_CLONED = 0x40000000, // GT_BOX -- this box and its operand has been cloned, cannot assume it to be single-use anymore
GTF_BOX_VALUE = 0x80000000, // GT_BOX -- "box" is on a value type

GTF_ICON_HDL_MASK = 0xF0000000, // Bits used by handle types below
Expand Down Expand Up @@ -3638,6 +3639,16 @@ struct GenTreeBox : public GenTreeUnOp
{
}
#endif

bool WasCloned()
{
return (gtFlags & GTF_BOX_CLONED) != 0;
}

void SetCloned()
{
gtFlags |= GTF_BOX_CLONED;
}
};

/* gtField -- data member ref (GT_FIELD) */
Expand Down

0 comments on commit 9588349

Please sign in to comment.