Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
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
47 changes: 40 additions & 7 deletions src/coreclr/jit/assertionprop.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1649,7 +1649,7 @@ AssertionIndex Compiler::optCreateJtrueAssertions(GenTree* op1, GenTree* op2, bo
return assertionIndex;
}

AssertionInfo Compiler::optCreateJTrueBoundsAssertion(GenTree* tree)
AssertionInfo Compiler::optCreateJTrueBoundsAssertion(GenTree* tree, bool createComplementary)
{
// These assertions are VN based, so not relevant for local prop
//
Expand Down Expand Up @@ -1705,7 +1705,10 @@ AssertionInfo Compiler::optCreateJTrueBoundsAssertion(GenTree* tree)
relopFunc = ValueNumStore::SwapRelop(relopFunc);
AssertionDsc dsc = AssertionDsc::CreateCompareCheckedBound(this, relopFunc, op2VN, op1VN, 0);
AssertionIndex idx = optAddAssertion(dsc);
optCreateComplementaryAssertion(idx);
if (createComplementary)
{
optCreateComplementaryAssertion(idx);
}
return idx;
}

Expand All @@ -1714,7 +1717,10 @@ AssertionInfo Compiler::optCreateJTrueBoundsAssertion(GenTree* tree)
{
AssertionDsc dsc = AssertionDsc::CreateCompareCheckedBound(this, relopFunc, op1VN, op2VN, 0);
AssertionIndex idx = optAddAssertion(dsc);
optCreateComplementaryAssertion(idx);
if (createComplementary)
{
optCreateComplementaryAssertion(idx);
}
return idx;
}

Expand All @@ -1727,7 +1733,10 @@ AssertionInfo Compiler::optCreateJTrueBoundsAssertion(GenTree* tree)
relopFunc = ValueNumStore::SwapRelop(relopFunc);
AssertionDsc dsc = AssertionDsc::CreateCompareCheckedBound(this, relopFunc, op2VN, checkedBnd, checkedBndCns);
AssertionIndex idx = optAddAssertion(dsc);
optCreateComplementaryAssertion(idx);
if (createComplementary)
{
optCreateComplementaryAssertion(idx);
}
return idx;
}

Expand All @@ -1736,7 +1745,10 @@ AssertionInfo Compiler::optCreateJTrueBoundsAssertion(GenTree* tree)
{
AssertionDsc dsc = AssertionDsc::CreateCompareCheckedBound(this, relopFunc, op1VN, checkedBnd, checkedBndCns);
AssertionIndex idx = optAddAssertion(dsc);
optCreateComplementaryAssertion(idx);
if (createComplementary)
{
optCreateComplementaryAssertion(idx);
}
return idx;
}

Expand All @@ -1745,6 +1757,14 @@ AssertionInfo Compiler::optCreateJTrueBoundsAssertion(GenTree* tree)
ValueNumStore::UnsignedCompareCheckedBoundInfo unsignedCompareBnd;
if (vnStore->IsVNUnsignedCompareCheckedBound(relopVN, &unsignedCompareBnd))
{
if (!createComplementary && (unsignedCompareBnd.cmpOper == VNF_GE_UN))
{
// The no-throw assertion only holds on the FALSE edge of "i >= bnd". A one-sided
// caller guarantees the relop is TRUE at the use site, so the assertion does not
// hold there - skip it.
return NO_ASSERTION_INDEX;
}

ValueNum idxVN = vnStore->VNNormalValue(unsignedCompareBnd.vnIdx);
ValueNum lenVN = vnStore->VNNormalValue(unsignedCompareBnd.vnBound);

Expand All @@ -1767,15 +1787,21 @@ AssertionInfo Compiler::optCreateJTrueBoundsAssertion(GenTree* tree)
relopFunc = ValueNumStore::SwapRelop(relopFunc);
AssertionDsc dsc = AssertionDsc::CreateConstantBound(this, relopFunc, op2VN, op1VN);
AssertionIndex idx = optAddAssertion(dsc);
optCreateComplementaryAssertion(idx);
if (createComplementary)
{
optCreateComplementaryAssertion(idx);
}
return idx;
}

if (vnStore->IsVNIntegralConstant(op2VN, &cns) && (!isUnsignedRelop || (cns > 0)))
{
AssertionDsc dsc = AssertionDsc::CreateConstantBound(this, relopFunc, op1VN, op2VN);
AssertionIndex idx = optAddAssertion(dsc);
optCreateComplementaryAssertion(idx);
if (createComplementary)
{
optCreateComplementaryAssertion(idx);
}
return idx;
}

Expand Down Expand Up @@ -2163,6 +2189,13 @@ void Compiler::optAssertionGen(GenTree* tree)
assertionInfo = optAssertionGenJtrue(tree);
break;

case GT_ASSERTION:
// The relop wrapped by GT_ASSERTION is known to be true at this point.
// Generate the same assertion we would for `JTRUE(relop)` taking the true edge,
// but skip the complementary assertion since this is a one-sided fact.
assertionInfo = optCreateJTrueBoundsAssertion(tree, /* createComplementary */ false);
break;

default:
// All other gtOper node kinds, leave 'assertionIndex' = NO_ASSERTION_INDEX
break;
Expand Down
3 changes: 2 additions & 1 deletion src/coreclr/jit/compiler.h
Original file line number Diff line number Diff line change
Expand Up @@ -8726,7 +8726,7 @@ class Compiler
// Assertion Gen functions.
void optAssertionGen(GenTree* tree);
AssertionIndex optAssertionGenCast(GenTreeCast* cast);
AssertionInfo optCreateJTrueBoundsAssertion(GenTree* tree);
AssertionInfo optCreateJTrueBoundsAssertion(GenTree* tree, bool createComplementary = true);
AssertionInfo optAssertionGenJtrue(GenTree* tree);
AssertionIndex optCreateJtrueAssertions(GenTree* op1, GenTree* op2, bool equals);
AssertionIndex optFindComplementary(AssertionIndex assertionIndex);
Expand Down Expand Up @@ -12490,6 +12490,7 @@ class GenTreeVisitor
case GT_RUNTIMELOOKUP:
case GT_ARR_ADDR:
case GT_KEEPALIVE:
case GT_ASSERTION:
case GT_INC_SATURATE:
{
GenTreeUnOp* const unOp = node->AsUnOp();
Expand Down
1 change: 1 addition & 0 deletions src/coreclr/jit/compiler.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -4476,6 +4476,7 @@ GenTree::VisitResult GenTree::VisitOperandUses(TVisitor visitor)
case GT_PUTARG_STK:
case GT_RETURNTRAP:
case GT_KEEPALIVE:
case GT_ASSERTION:
case GT_INC_SATURATE:
case GT_RETURN_SUSPEND:
return visitor(&this->AsUnOp()->gtOp1);
Expand Down
2 changes: 2 additions & 0 deletions src/coreclr/jit/gentree.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6900,6 +6900,7 @@ bool GenTree::TryGetUse(GenTree* operand, GenTree*** pUse)
case GT_BSWAP:
case GT_BSWAP16:
case GT_KEEPALIVE:
case GT_ASSERTION:
case GT_INC_SATURATE:
if (operand == this->AsUnOp()->gtOp1)
{
Expand Down Expand Up @@ -10547,6 +10548,7 @@ GenTreeUseEdgeIterator::GenTreeUseEdgeIterator(GenTree* node)
case GT_BSWAP:
case GT_BSWAP16:
case GT_KEEPALIVE:
case GT_ASSERTION:
case GT_INC_SATURATE:
case GT_RETURNTRAP:
case GT_RETURN_SUSPEND:
Expand Down
1 change: 1 addition & 0 deletions src/coreclr/jit/gtlist.h
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ GTNODE(NEG , GenTreeOp ,0,0,GTK_UNOP)

GTNODE(INTRINSIC , GenTreeIntrinsic ,0,0,GTK_BINOP|GTK_EXOP)
GTNODE(KEEPALIVE , GenTree ,0,0,GTK_UNOP|GTK_NOVALUE) // keep operand alive, generate no code, produce no result
GTNODE(ASSERTION , GenTreeOp ,0,0,GTK_UNOP|GTK_NOVALUE|DBK_NOTLIR) // used to seed assertions
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
GTNODE(ASSERTION , GenTreeOp ,0,0,GTK_UNOP|GTK_NOVALUE|DBK_NOTLIR) // used to seed assertions
GTNODE(ASSERTION , GenTreeOp ,0,0,GTK_UNOP|GTK_NOVALUE|DBK_NOTLIR) // op1 is non-zero


GTNODE(CAST , GenTreeCast ,0,0,GTK_UNOP|GTK_EXOP) // conversion to another type
GTNODE(BITCAST , GenTreeOp ,0,1,GTK_UNOP) // reinterpretation of bits as another type
Expand Down
8 changes: 8 additions & 0 deletions src/coreclr/jit/inductionvariableopts.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,14 @@ PerLoopInfo::LoopInfo* PerLoopInfo::GetOrCreateInfo(FlowGraphNaturalLoop* loop)

for (Statement* stmt : block->NonPhiStatements())
{
// GT_ASSERTION is a marker that is dropped before LIR. Skip its uses
// here so they don't pin the IV and prevent strength reduction.
if (stmt->GetRootNode()->OperIs(GT_ASSERTION) &&
((stmt->GetRootNode()->gtGetOp1()->gtFlags & GTF_SIDE_EFFECT) == 0))
{
continue;
}
Comment on lines +161 to +167
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will essentially produce illegal IR -- if we strength reduce the locals contained in GT_ASSERTION's operand, then the assertion is no longer true. I think there are two ways to do it properly:

  1. Just delete relevant GT_ASSERTIONS when we strength reduce
  2. Replace expressions under GT_ASSERTIONS with a computation of the original IV

2 is probably not that simple.


for (GenTree* node : stmt->TreeList())
{
info.HasSuspensionPoint |= node->IsCall() && node->AsCall()->IsAsync();
Expand Down
1 change: 1 addition & 0 deletions src/coreclr/jit/liveness.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2567,6 +2567,7 @@ void Liveness<TLiveness>::ComputeLifeLIR(VARSET_TP& life, BasicBlock* block, VAR
case GT_IL_OFFSET:
case GT_RECORD_ASYNC_RESUME:
case GT_KEEPALIVE:
case GT_ASSERTION:
case GT_SWIFT_ERROR_RET:
case GT_GCPOLL:
case GT_WASM_JEXCEPT:
Expand Down
27 changes: 27 additions & 0 deletions src/coreclr/jit/optimizer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2126,6 +2126,33 @@ bool Compiler::optTryInvertWhileLoop(FlowGraphNaturalLoop* loop)
#endif // DEBUG

Metrics.LoopsInverted++;

// Synthesize a GT_ASSERTION at the start of `stayInLoopSucc` recording that the
// loop guard is known to be true on every entry into the loop body. This compensates
// for the cyclic value numbers that loop inversion creates on phi merges, which can
// otherwise prevent later phases (assertion prop, range check) from proving that
// the guard still holds inside the loop body.
//
auto isSuitableRelopOp = [this](GenTree* node) -> bool {
return node->IsInvariant() || (node->OperIs(GT_LCL_VAR) && !lvaVarAddrExposed(node->AsLclVar()->GetLclNum()));
};

GenTree* origRelop = condBlock->lastStmt()->GetRootNode()->gtGetOp1();
if (origRelop->OperIsCmpCompare() && isSuitableRelopOp(origRelop->gtGetOp1()) &&
isSuitableRelopOp(origRelop->gtGetOp2()))
{
GenTree* relopClone = gtCloneExpr(origRelop);
if (trueExits)
{
relopClone = gtReverseCond(relopClone);
}
relopClone->gtFlags &= ~GTF_RELOP_JMP_USED;
GenTree* assertion = gtNewOperNode(GT_ASSERTION, TYP_VOID, relopClone);
Statement* stmt = fgNewStmtFromTree(assertion, condBlock->lastStmt()->GetDebugInfo());
fgInsertStmtAtBeg(stayInLoopSucc, stmt);
JITDUMP("Inserted GT_ASSERTION at start of " FMT_BB " to record loop guard\n", stayInLoopSucc->bbNum);
}

return true;
}

Expand Down
21 changes: 21 additions & 0 deletions src/coreclr/jit/rationalize.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1726,6 +1726,27 @@ Compiler::fgWalkResult Rationalizer::RewriteNode(GenTree** useEdge, Compiler::Ge
}
break;

case GT_ASSERTION:
{
bool isClosed = false;
unsigned sideEffects = 0;
LIR::ReadOnlyRange range = BlockRange().GetTreeRange(node->gtGetOp1(), &isClosed, &sideEffects);
if (isClosed && ((sideEffects & GTF_SIDE_EFFECT) == 0))
{
BlockRange().Delete(m_compiler, m_block, std::move(range));
BlockRange().Remove(node);
return Compiler::WALK_CONTINUE;
}

BlockRange().Remove(node);
node = node->gtGetOp1();
if (node->IsValue())
{
node->SetUnusedValue();
}
break;
}

case GT_GCPOLL:
{
// GCPOLL is essentially a no-op, we used it as a hint for fgCreateGCPoll
Expand Down
1 change: 1 addition & 0 deletions src/coreclr/jit/valuenum.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -12956,6 +12956,7 @@ void Compiler::fgValueNumberTree(GenTree* tree)
case GT_RETFILT:
case GT_RETURN_SUSPEND:
case GT_NULLCHECK:
case GT_ASSERTION:
if (tree->gtGetOp1() != nullptr)
{
tree->gtVNPair = vnStore->VNPWithExc(vnStore->VNPForVoid(),
Expand Down
Loading