Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add and use a MakeSrcRegOptional that validates IsSafeToContainMem was called #77895

Merged
merged 4 commits into from
Nov 7, 2022
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 29 additions & 0 deletions src/coreclr/jit/lower.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ void Lowering::MakeSrcContained(GenTree* parentNode, GenTree* childNode) const
{
assert(!parentNode->OperIsLeaf());
assert(childNode->canBeContained());

childNode->SetContained();
assert(childNode->isContained());

Expand All @@ -60,6 +61,34 @@ void Lowering::MakeSrcContained(GenTree* parentNode, GenTree* childNode) const
#endif
}

//------------------------------------------------------------------------
// MakeSrcRegOptional: Make "childNode" a regOptional node
//
// Arguments:
// parentNode - is a non-leaf node that can regOptional its 'childNode'
// childNode - is an op that will now be regOptional to its parent.
//
void Lowering::MakeSrcRegOptional(GenTree* parentNode, GenTree* childNode) const
{
assert(!parentNode->OperIsLeaf());

childNode->SetRegOptional();
assert(childNode->IsRegOptional());

#ifdef DEBUG
// Verify caller of this method checked safety.
//
const bool isSafeToContainMem = IsSafeToContainMem(parentNode, childNode);

if (!isSafeToContainMem)
{
JITDUMP("** Unsafe regOptional of [%06u] in [%06u}\n", comp->dspTreeID(childNode),
comp->dspTreeID(parentNode));
assert(isSafeToContainMem);
}
#endif
}

//------------------------------------------------------------------------
// CheckImmedAndMakeContained: Checks if the 'childNode' is a containable immediate
// and, if so, makes it contained.
Expand Down
37 changes: 35 additions & 2 deletions src/coreclr/jit/lower.h
Original file line number Diff line number Diff line change
Expand Up @@ -288,6 +288,7 @@ class Lowering final : public Phase
const bool op2Legal = isSafeToMarkOp2 && (operatorSize == genTypeSize(op2->TypeGet()));

GenTree* regOptionalOperand = nullptr;

if (op1Legal)
{
regOptionalOperand = op2Legal ? PreferredRegOptionalOperand(tree) : op1;
Expand All @@ -296,9 +297,10 @@ class Lowering final : public Phase
{
regOptionalOperand = op2;
}

if (regOptionalOperand != nullptr)
{
regOptionalOperand->SetRegOptional();
MakeSrcRegOptional(tree, regOptionalOperand);
}
}
#endif // defined(TARGET_XARCH)
Expand Down Expand Up @@ -447,7 +449,7 @@ class Lowering final : public Phase
bool IsContainableBinaryOp(GenTree* parentNode, GenTree* childNode) const;
#endif // TARGET_ARM64

#ifdef FEATURE_HW_INTRINSICS
#if defined(FEATURE_HW_INTRINSICS)
// Tries to get a containable node for a given HWIntrinsic
bool TryGetContainableHWIntrinsicOp(GenTreeHWIntrinsic* containingNode,
GenTree** pNode,
Expand All @@ -465,6 +467,37 @@ class Lowering final : public Phase
// Makes 'childNode' contained in the 'parentNode'
void MakeSrcContained(GenTree* parentNode, GenTree* childNode) const;

// Makes 'childNode' regOptional in the 'parentNode'
void MakeSrcRegOptional(GenTree* parentNode, GenTree* childNode) const;

// Tries to make 'childNode' contained or regOptional in the 'parentNode'
inline void TryMakeSrcContainedOrRegOptional(GenTree* parentNode, GenTree* childNode) const
tannergooding marked this conversation as resolved.
Show resolved Hide resolved
{
#if defined(FEATURE_HW_INTRINSICS)
// HWIntrinsic nodes should use TryGetContainableHWIntrinsicOp and its relevant handling
assert(!parentNode->OperIsHWIntrinsic());
#endif
tannergooding marked this conversation as resolved.
Show resolved Hide resolved

if (!IsSafeToContainMem(parentNode, childNode))
{
return;
}

if (IsContainableMemoryOp(childNode))
{
MakeSrcContained(parentNode, childNode);
}
else
{
MakeSrcRegOptional(parentNode, childNode);
}
}

#if defined(FEATURE_HW_INTRINSICS)
// Tries to make 'childNode' contained or regOptional in the 'parentNode'
void TryMakeSrcContainedOrRegOptional(GenTreeHWIntrinsic* parentNode, GenTree* childNode) const;
#endif

// Checks and makes 'childNode' contained in the 'parentNode'
bool CheckImmedAndMakeContained(GenTree* parentNode, GenTree* childNode);

Expand Down
Loading