Skip to content

Commit

Permalink
JIT: Relax reg-optionality validity checks
Browse files Browse the repository at this point in the history
  • Loading branch information
@jakobbotsch
jakobbotsch committed Feb 4, 2023
1 parent 3ced968 commit cf0952b
Show file tree
Hide file tree
Showing 5 changed files with 147 additions and 108 deletions.
137 changes: 95 additions & 42 deletions src/coreclr/jit/lower.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -78,12 +78,12 @@ void Lowering::MakeSrcRegOptional(GenTree* parentNode, GenTree* childNode) const
#ifdef DEBUG
// Verify caller of this method checked safety.
//
const bool isSafeToContainMem = IsSafeToContainMem(parentNode, childNode);
const bool isSafeToMarkRegOptional = IsSafeToMarkRegOptional(parentNode, childNode);

if (!isSafeToContainMem)
if (!isSafeToMarkRegOptional)
{
JITDUMP("** Unsafe regOptional of [%06u] in [%06u}\n", comp->dspTreeID(childNode), comp->dspTreeID(parentNode));
assert(isSafeToContainMem);
assert(isSafeToMarkRegOptional);
}
#endif
}
Expand All @@ -100,16 +100,11 @@ void Lowering::TryMakeSrcContainedOrRegOptional(GenTree* parentNode, GenTree* ch
// HWIntrinsic nodes should use TryGetContainableHWIntrinsicOp and its relevant handling
assert(!parentNode->OperIsHWIntrinsic());

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

if (IsContainableMemoryOp(childNode))
if (IsContainableMemoryOp(childNode) && IsSafeToContainMem(parentNode, childNode))
{
MakeSrcContained(parentNode, childNode);
}
else
else if (IsSafeToMarkRegOptional(parentNode, childNode))
{
MakeSrcRegOptional(parentNode, childNode);
}
Expand Down Expand Up @@ -140,32 +135,35 @@ bool Lowering::CheckImmedAndMakeContained(GenTree* parentNode, GenTree* childNod
}

//------------------------------------------------------------------------
// IsSafeToContainMem: Checks for conflicts between childNode and parentNode,
// and returns 'true' iff memory operand childNode can be contained in parentNode.
// IsInvariantInRange: Check if a node is invariant in the specified range. In
// other words, can 'node' be moved to right before 'endExclusive' without its
// computation changing values?
//
// Arguments:
// parentNode - any non-leaf node
// childNode - some node that is an input to `parentNode`
// node - The node.
// endExclusive - The exclusive end of the range to check invariance for.
//
// Return value:
// true if it is safe to make childNode a contained memory operand.
// Returns:
// True if 'node' can be evaluated at any point between its current
// location and 'parentNode' without giving a different result; otherwise
// false.
//
bool Lowering::IsSafeToContainMem(GenTree* parentNode, GenTree* childNode) const
bool Lowering::IsInvariantInRange(GenTree* node, GenTree* endExclusive) const
{
// Quick early-out for unary cases
//
if (childNode->gtNext == parentNode)
if (node->gtNext == endExclusive)
{
return true;
}

m_scratchSideEffects.Clear();
m_scratchSideEffects.AddNode(comp, childNode);
m_scratchSideEffects.AddNode(comp, node);

for (GenTree* node = childNode->gtNext; node != parentNode; node = node->gtNext)
for (GenTree* cur = node->gtNext; cur != endExclusive; cur = cur->gtNext)
{
const bool strict = true;
if (m_scratchSideEffects.InterferesWith(comp, node, strict))
if (m_scratchSideEffects.InterferesWith(comp, cur, strict))
{
return false;
}
Expand All @@ -175,31 +173,33 @@ bool Lowering::IsSafeToContainMem(GenTree* parentNode, GenTree* childNode) const
}

//------------------------------------------------------------------------
// IsSafeToContainMem: Checks for conflicts between childNode and grandParentNode
// and returns 'true' iff memory operand childNode can be contained in ancestorNode
// IsInvariantInRange: Check if a node is invariant in the specified range,
// ignoring conflicts with one particular node.
//
// Arguments:
// grandParentNode - any non-leaf node
// parentNode - parent of `childNode` and an input to `grandParentNode`
// childNode - some node that is an input to `parentNode`
// node - The node.
// endExclusive - The exclusive end of the range to check invariance for.
// ignoreNode - The node to ignore interference checks with, for example
// because it will retain its relative order with 'node'.
//
// Return value:
// true if it is safe to make childNode a contained memory operand.
// Returns:
// True if 'node' can be evaluated at any point between its current location
// and 'endExclusive' without giving a different result; otherwise false.
//
bool Lowering::IsSafeToContainMem(GenTree* grandparentNode, GenTree* parentNode, GenTree* childNode) const
bool Lowering::IsInvariantInRange(GenTree* node, GenTree* endExclusive, GenTree* ignoreNode) const
{
m_scratchSideEffects.Clear();
m_scratchSideEffects.AddNode(comp, childNode);
m_scratchSideEffects.AddNode(comp, node);

for (GenTree* node = childNode->gtNext; node != grandparentNode; node = node->gtNext)
for (GenTree* cur = node->gtNext; cur != endExclusive; cur = cur->gtNext)
{
if (node == parentNode)
if (cur == ignoreNode)
{
continue;
}

const bool strict = true;
if (m_scratchSideEffects.InterferesWith(comp, node, strict))
if (m_scratchSideEffects.InterferesWith(comp, cur, strict))
{
return false;
}
Expand All @@ -208,6 +208,58 @@ bool Lowering::IsSafeToContainMem(GenTree* grandparentNode, GenTree* parentNode,
return true;
}

//------------------------------------------------------------------------
// IsSafeToContainMem: Checks for conflicts between childNode and parentNode,
// and returns 'true' iff memory operand childNode can be contained in parentNode.
//
// Arguments:
// parentNode - any non-leaf node
// childNode - some node that is an input to `parentNode`
//
// Return value:
// true if it is safe to make childNode a contained memory operand.
//
bool Lowering::IsSafeToContainMem(GenTree* parentNode, GenTree* childNode) const
{
return IsInvariantInRange(childNode, parentNode);
}

//------------------------------------------------------------------------
// IsSafeToContainMem: Checks for conflicts between childNode and grandParentNode
// and returns 'true' iff memory operand childNode can be contained in ancestorNode
//
// Arguments:
// grandParentNode - any non-leaf node
// parentNode - parent of `childNode` and an input to `grandParentNode`
// childNode - some node that is an input to `parentNode`
//
// Return value:
// true if it is safe to make childNode a contained memory operand.
//
bool Lowering::IsSafeToContainMem(GenTree* grandparentNode, GenTree* parentNode, GenTree* childNode) const
{
return IsInvariantInRange(childNode, grandparentNode, parentNode);
}

bool Lowering::IsSafeToMarkRegOptional(GenTree* parentNode, GenTree* childNode) const
{
if (!childNode->OperIs(GT_LCL_VAR))
{
// LIR edges never interfere.
return true;
}

LclVarDsc* dsc = comp->lvaGetDesc(childNode->AsLclVarCommon());
if (!dsc->IsAddressExposed())
{
// Safe by IR invariants (no assignments occur between parent and node).
return true;
}

// We expect this to have interference as otherwise we could have marked it contained.
return IsInvariantInRange(childNode, parentNode);
}

//------------------------------------------------------------------------
// LowerNode: this is the main entry point for Lowering.
//
Expand Down Expand Up @@ -2530,15 +2582,16 @@ void Lowering::LowerCFGCall(GenTreeCall* call)
}

//------------------------------------------------------------------------
// IsInvariantInRange: Check if a node is invariant in the specified range. In
// other words, can 'node' be moved to right before 'endExclusive' without its
// computation changing values?
// IsCFGCallArgInvariantInRange: A cheap version of IsInvariantInRange to check
// if a node is invariant in the specified range. In other words, can 'node' be
// moved to right before 'endExclusive' without its computation changing
// values?
//
// Arguments:
// node - The node.
// endExclusive - The exclusive end of the range to check invariance for.
//
bool Lowering::IsInvariantInRange(GenTree* node, GenTree* endExclusive)
bool Lowering::IsCFGCallArgInvariantInRange(GenTree* node, GenTree* endExclusive)
{
assert(node->Precedes(endExclusive));

Expand Down Expand Up @@ -2603,7 +2656,7 @@ void Lowering::MoveCFGCallArg(GenTreeCall* call, GenTree* node)
GenTree* operand = node->AsOp()->gtGetOp1();
JITDUMP("Checking if we can move operand of GT_PUTARG_* node:\n");
DISPTREE(operand);
if (((operand->gtFlags & GTF_ALL_EFFECT) == 0) && IsInvariantInRange(operand, call))
if (((operand->gtFlags & GTF_ALL_EFFECT) == 0) && IsCFGCallArgInvariantInRange(operand, call))
{
JITDUMP("...yes, moving to after validator call\n");
BlockRange().Remove(operand);
Expand Down Expand Up @@ -5490,7 +5543,7 @@ bool Lowering::TryCreateAddrMode(GenTree* addr, bool isContainable, GenTree* par
{
if (index->OperIs(GT_CAST) && (scale == 1) && (offset == 0) && varTypeIsByte(targetType))
{
if (IsSafeToContainMem(parent, index))
if (IsInvariantInRange(index, parent))
{
// Check containment safety against the parent node - this will ensure that LEA with the contained
// index will itself always be contained. We do not support uncontained LEAs with contained indices.
Expand All @@ -5513,7 +5566,7 @@ bool Lowering::TryCreateAddrMode(GenTree* addr, bool isContainable, GenTree* par
if (cast->CastOp()->TypeIs(TYP_INT) && cast->TypeIs(TYP_LONG) &&
(genTypeSize(targetType) == (1U << shiftBy)) && (scale == 1) && (offset == 0))
{
if (IsSafeToContainMem(parent, index))
if (IsInvariantInRange(index, parent))
{
// Check containment safety against the parent node - this will ensure that LEA with the contained
// index will itself always be contained. We do not support uncontained LEAs with contained indices.
Expand Down Expand Up @@ -7222,7 +7275,7 @@ void Lowering::LowerStoreIndirCommon(GenTreeStoreInd* ind)
#if defined(TARGET_ARM64)
// Verify containment safety before creating an LEA that must be contained.
//
const bool isContainable = IsSafeToContainMem(ind, ind->Addr());
const bool isContainable = IsInvariantInRange(ind->Addr(), ind);
#else
const bool isContainable = true;
#endif
Expand Down Expand Up @@ -7283,7 +7336,7 @@ void Lowering::LowerIndir(GenTreeIndir* ind)
#if defined(TARGET_ARM64)
// Verify containment safety before creating an LEA that must be contained.
//
const bool isContainable = (ind->Addr() != nullptr) && IsSafeToContainMem(ind, ind->Addr());
const bool isContainable = (ind->Addr() != nullptr) && IsInvariantInRange(ind->Addr(), ind);
#else
const bool isContainable = true;
#endif
Expand Down
8 changes: 7 additions & 1 deletion src/coreclr/jit/lower.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ class Lowering final : public Phase
void LowerBlock(BasicBlock* block);
GenTree* LowerNode(GenTree* node);

bool IsInvariantInRange(GenTree* node, GenTree* endExclusive);
bool IsCFGCallArgInvariantInRange(GenTree* node, GenTree* endExclusive);

// ------------------------------
// Call Lowering
Expand Down Expand Up @@ -500,13 +500,19 @@ class Lowering final : public Phase
// Checks and makes 'childNode' contained in the 'parentNode'
bool CheckImmedAndMakeContained(GenTree* parentNode, GenTree* childNode);

bool IsInvariantInRange(GenTree* node, GenTree* endExclusive) const;
bool IsInvariantInRange(GenTree* node, GenTree* endExclusive, GenTree* ignoreNode) const;

// Checks for memory conflicts in the instructions between childNode and parentNode, and returns true if childNode
// can be contained.
bool IsSafeToContainMem(GenTree* parentNode, GenTree* childNode) const;

// Similar to above, but allows bypassing a "transparent" parent.
bool IsSafeToContainMem(GenTree* grandparentNode, GenTree* parentNode, GenTree* childNode) const;

// Check if marking an operand of a node as reg-optional is safe.
bool IsSafeToMarkRegOptional(GenTree* parentNode, GenTree* node) const;

inline LIR::Range& BlockRange() const
{
return LIR::AsRange(m_block);
Expand Down
Loading

0 comments on commit cf0952b

Please sign in to comment.