From 8578b39fa50624df0124ca03d6e4bf61336b582b Mon Sep 17 00:00:00 2001 From: EgorBo Date: Thu, 30 Jan 2025 00:21:34 +0100 Subject: [PATCH 01/11] nonnull assertions --- src/coreclr/jit/assertionprop.cpp | 78 +++++++++++++++++++++++++++++++ src/coreclr/jit/compiler.h | 2 + 2 files changed, 80 insertions(+) diff --git a/src/coreclr/jit/assertionprop.cpp b/src/coreclr/jit/assertionprop.cpp index 12f0ed85e03c66..96d4f4b4444091 100644 --- a/src/coreclr/jit/assertionprop.cpp +++ b/src/coreclr/jit/assertionprop.cpp @@ -4367,6 +4367,64 @@ GenTree* Compiler::optAssertionProp_RelOp(ASSERT_VALARG_TP assertions, GenTree* return optAssertionPropLocal_RelOp(assertions, tree, stmt); } +bool Compiler::optAssertionPropPhiDefNotNull(BasicBlock* defBlock, GenTreePhi* phi) +{ + // enumerate phi's args + for (GenTreePhi::Use& use : phi->Uses()) + { + if (!use.GetNode()->OperIs(GT_PHI_ARG)) + { + return false; + } + + GenTreePhiArg* arg = use.GetNode()->AsPhiArg(); + BasicBlock* pred = arg->gtPredBB; + + const ValueNum phiArgVN = vnStore->VNConservativeNormalValue(arg->gtVNPair); + bool isKnownNonNull = vnStore->IsKnownNonNull(phiArgVN); + if (!isKnownNonNull) + { + ASSERT_TP assertions = nullptr; + if (pred->KindIs(BBJ_COND) && pred->FalseTargetIs(defBlock)) + { + assertions = pred->bbAssertionOut; + } + else if ((pred->KindIs(BBJ_ALWAYS) && pred->TargetIs(defBlock)) || + (pred->KindIs(BBJ_COND) && pred->TrueTargetIs(defBlock))) + { + if (bbJtrueAssertionOut != nullptr) + { + assertions = bbJtrueAssertionOut[pred->bbNum]; + } + } + + if (assertions == nullptr) + { + return false; + } + + bool found = false; + BitVecOps::Iter iter(apTraits, assertions); + unsigned index = 0; + while (iter.NextElem(&index)) + { + AssertionDsc* curAssertion = optGetAssertion(GetAssertionIndex(index)); + if (curAssertion->CanPropNonNull() && curAssertion->op1.vn == phiArgVN) + { + found = true; + break; + } + } + + if (!found) + { + return false; + } + } + } + return true; +} + //------------------------------------------------------------------------ // optAssertionProp: try and optimize a relop via assertion propagation // @@ -4463,6 +4521,26 @@ GenTree* Compiler::optAssertionPropGlobal_RelOp(ASSERT_VALARG_TP assertions, Gen return nullptr; } + if (!optLocalAssertionProp && op2->IsIntegralConst(0) && varTypeIsGC(op1) && op1->OperIs(GT_LCL_VAR) && + newTree->OperIs(GT_EQ, GT_NE)) + { + unsigned lclNum = op1->AsLclVarCommon()->GetLclNum(); + unsigned ssaNum = op1->AsLclVarCommon()->GetSsaNum(); + + LclSsaVarDsc* ssaDef = lvaGetDesc(lclNum)->GetPerSsaData(ssaNum); + GenTreeLclVarCommon* node = ssaDef->GetDefNode(); + if (node->OperIs(GT_STORE_LCL_VAR) && node->Data()->OperIs(GT_PHI)) + { + if (optAssertionPropPhiDefNotNull(ssaDef->GetBlock(), ssaDef->GetDefNode()->AsPhi())) + { + newTree = tree->OperIs(GT_EQ) ? gtNewIconNode(0) : gtNewIconNode(1); + newTree = gtWrapWithSideEffects(newTree, tree, GTF_ALL_EFFECT); + DISPTREE(newTree); + return optAssertionProp_Update(newTree, tree, stmt); + } + } + } + // Find an equal or not equal assertion involving "op1" and "op2". index = optGlobalAssertionIsEqualOrNotEqual(assertions, op1, op2); diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index 4a639c7e639f9e..52064d285248fb 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -8257,6 +8257,8 @@ class Compiler bool optNonNullAssertionProp_Ind(ASSERT_VALARG_TP assertions, GenTree* indir); bool optWriteBarrierAssertionProp_StoreInd(ASSERT_VALARG_TP assertions, GenTreeStoreInd* indir); + bool Compiler::optAssertionPropPhiDefNotNull(BasicBlock* defBlock, GenTreePhi* phi); + void optAssertionProp_RangeProperties(ASSERT_VALARG_TP assertions, GenTree* tree, bool* isKnownNonZero, From 16601c43b0dc67574e6332f4f6cd8dd0ff6b7904 Mon Sep 17 00:00:00 2001 From: EgorBo Date: Thu, 30 Jan 2025 00:36:24 +0100 Subject: [PATCH 02/11] fix build --- src/coreclr/jit/assertionprop.cpp | 6 +++--- src/coreclr/jit/compiler.h | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/coreclr/jit/assertionprop.cpp b/src/coreclr/jit/assertionprop.cpp index 96d4f4b4444091..11c54315409680 100644 --- a/src/coreclr/jit/assertionprop.cpp +++ b/src/coreclr/jit/assertionprop.cpp @@ -4522,16 +4522,16 @@ GenTree* Compiler::optAssertionPropGlobal_RelOp(ASSERT_VALARG_TP assertions, Gen } if (!optLocalAssertionProp && op2->IsIntegralConst(0) && varTypeIsGC(op1) && op1->OperIs(GT_LCL_VAR) && - newTree->OperIs(GT_EQ, GT_NE)) + newTree->OperIs(GT_EQ, GT_NE) && op1->AsLclVarCommon()->HasSsaName()) { unsigned lclNum = op1->AsLclVarCommon()->GetLclNum(); unsigned ssaNum = op1->AsLclVarCommon()->GetSsaNum(); LclSsaVarDsc* ssaDef = lvaGetDesc(lclNum)->GetPerSsaData(ssaNum); GenTreeLclVarCommon* node = ssaDef->GetDefNode(); - if (node->OperIs(GT_STORE_LCL_VAR) && node->Data()->OperIs(GT_PHI)) + if (node != nullptr && node->OperIs(GT_STORE_LCL_VAR) && node->Data()->OperIs(GT_PHI)) { - if (optAssertionPropPhiDefNotNull(ssaDef->GetBlock(), ssaDef->GetDefNode()->AsPhi())) + if (optAssertionPropPhiDefNotNull(ssaDef->GetBlock(), node->Data()->AsPhi())) { newTree = tree->OperIs(GT_EQ) ? gtNewIconNode(0) : gtNewIconNode(1); newTree = gtWrapWithSideEffects(newTree, tree, GTF_ALL_EFFECT); diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index 52064d285248fb..69751597ced4ee 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -8257,7 +8257,7 @@ class Compiler bool optNonNullAssertionProp_Ind(ASSERT_VALARG_TP assertions, GenTree* indir); bool optWriteBarrierAssertionProp_StoreInd(ASSERT_VALARG_TP assertions, GenTreeStoreInd* indir); - bool Compiler::optAssertionPropPhiDefNotNull(BasicBlock* defBlock, GenTreePhi* phi); + bool optAssertionPropPhiDefNotNull(BasicBlock* defBlock, GenTreePhi* phi); void optAssertionProp_RangeProperties(ASSERT_VALARG_TP assertions, GenTree* tree, From 1aa4aeb8778f7aa096ad38f134cb8156f074a229 Mon Sep 17 00:00:00 2001 From: EgorBo Date: Thu, 30 Jan 2025 01:11:35 +0100 Subject: [PATCH 03/11] clean up --- src/coreclr/jit/assertionprop.cpp | 51 +++++++++++++++++++------------ src/coreclr/jit/compiler.h | 2 +- 2 files changed, 32 insertions(+), 21 deletions(-) diff --git a/src/coreclr/jit/assertionprop.cpp b/src/coreclr/jit/assertionprop.cpp index 11c54315409680..5f8b6e590d1dfb 100644 --- a/src/coreclr/jit/assertionprop.cpp +++ b/src/coreclr/jit/assertionprop.cpp @@ -4367,9 +4367,12 @@ GenTree* Compiler::optAssertionProp_RelOp(ASSERT_VALARG_TP assertions, GenTree* return optAssertionPropLocal_RelOp(assertions, tree, stmt); } -bool Compiler::optAssertionPropPhiDefNotNull(BasicBlock* defBlock, GenTreePhi* phi) +bool Compiler::optAssertionPropPhiDefNotNull(BasicBlock* block, GenTreePhi* phi) { - // enumerate phi's args + assert(phi != nullptr); + assert(block != nullptr); + + // Enumerate phi's args for (GenTreePhi::Use& use : phi->Uses()) { if (!use.GetNode()->OperIs(GT_PHI_ARG)) @@ -4377,20 +4380,21 @@ bool Compiler::optAssertionPropPhiDefNotNull(BasicBlock* defBlock, GenTreePhi* p return false; } - GenTreePhiArg* arg = use.GetNode()->AsPhiArg(); - BasicBlock* pred = arg->gtPredBB; + GenTreePhiArg* arg = use.GetNode()->AsPhiArg(); + BasicBlock* pred = arg->gtPredBB; + const ValueNum phiArgVN = vnStore->VNConservativeNormalValue(arg->gtVNPair); - const ValueNum phiArgVN = vnStore->VNConservativeNormalValue(arg->gtVNPair); - bool isKnownNonNull = vnStore->IsKnownNonNull(phiArgVN); - if (!isKnownNonNull) + // If this PhiArg is not known to be non-null, then maybe assertions can help. + if (!vnStore->IsKnownNonNull(phiArgVN)) { - ASSERT_TP assertions = nullptr; - if (pred->KindIs(BBJ_COND) && pred->FalseTargetIs(defBlock)) + // Find the outgoing assertion for the predecessor block that targets this block. + ASSERT_TP assertions = BitVecOps::UninitVal(); + if (pred->KindIs(BBJ_COND) && pred->FalseTargetIs(block)) { assertions = pred->bbAssertionOut; } - else if ((pred->KindIs(BBJ_ALWAYS) && pred->TargetIs(defBlock)) || - (pred->KindIs(BBJ_COND) && pred->TrueTargetIs(defBlock))) + else if ((pred->KindIs(BBJ_ALWAYS) && pred->TargetIs(block)) || + (pred->KindIs(BBJ_COND) && pred->TrueTargetIs(block))) { if (bbJtrueAssertionOut != nullptr) { @@ -4398,12 +4402,13 @@ bool Compiler::optAssertionPropPhiDefNotNull(BasicBlock* defBlock, GenTreePhi* p } } - if (assertions == nullptr) + if (BitVecOps::MayBeUninit(assertions) || BitVecOps::IsEmpty(apTraits, assertions)) { return false; } - bool found = false; + // Iterate over the assertions and check if there is a "phiArgVN != null" assertion. + bool notNull = false; BitVecOps::Iter iter(apTraits, assertions); unsigned index = 0; while (iter.NextElem(&index)) @@ -4411,13 +4416,14 @@ bool Compiler::optAssertionPropPhiDefNotNull(BasicBlock* defBlock, GenTreePhi* p AssertionDsc* curAssertion = optGetAssertion(GetAssertionIndex(index)); if (curAssertion->CanPropNonNull() && curAssertion->op1.vn == phiArgVN) { - found = true; + notNull = true; break; } } - if (!found) + if (!notNull) { + // We can't prove that the phiArg is non-null - bail out. return false; } } @@ -4438,6 +4444,8 @@ bool Compiler::optAssertionPropPhiDefNotNull(BasicBlock* defBlock, GenTreePhi* p // GenTree* Compiler::optAssertionPropGlobal_RelOp(ASSERT_VALARG_TP assertions, GenTree* tree, Statement* stmt) { + assert(!optLocalAssertionProp); + GenTree* newTree = tree; GenTree* op1 = tree->AsOp()->gtOp1; GenTree* op2 = tree->AsOp()->gtOp2; @@ -4521,18 +4529,21 @@ GenTree* Compiler::optAssertionPropGlobal_RelOp(ASSERT_VALARG_TP assertions, Gen return nullptr; } - if (!optLocalAssertionProp && op2->IsIntegralConst(0) && varTypeIsGC(op1) && op1->OperIs(GT_LCL_VAR) && - newTree->OperIs(GT_EQ, GT_NE) && op1->AsLclVarCommon()->HasSsaName()) + // See if we have "PHI ==/!= null" tree. In that case, we can use optAssertionPropPhiDefNotNull + // to iterate over all the phi args and check if they are non-null. If all of them are non-null, + // then we can fold this tree to true/false. + if (op2->IsIntegralConst(0) && varTypeIsGC(op1) && op1->OperIs(GT_LCL_VAR) && op1->AsLclVarCommon()->HasSsaName()) { - unsigned lclNum = op1->AsLclVarCommon()->GetLclNum(); - unsigned ssaNum = op1->AsLclVarCommon()->GetSsaNum(); - + // Get the definition of the local variable via SSA, we're looking for a phi definition. + unsigned lclNum = op1->AsLclVarCommon()->GetLclNum(); + unsigned ssaNum = op1->AsLclVarCommon()->GetSsaNum(); LclSsaVarDsc* ssaDef = lvaGetDesc(lclNum)->GetPerSsaData(ssaNum); GenTreeLclVarCommon* node = ssaDef->GetDefNode(); if (node != nullptr && node->OperIs(GT_STORE_LCL_VAR) && node->Data()->OperIs(GT_PHI)) { if (optAssertionPropPhiDefNotNull(ssaDef->GetBlock(), node->Data()->AsPhi())) { + assert(newTree->OperIs(GT_EQ, GT_NE)); newTree = tree->OperIs(GT_EQ) ? gtNewIconNode(0) : gtNewIconNode(1); newTree = gtWrapWithSideEffects(newTree, tree, GTF_ALL_EFFECT); DISPTREE(newTree); diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index 69751597ced4ee..6c79876cb331a0 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -8257,7 +8257,7 @@ class Compiler bool optNonNullAssertionProp_Ind(ASSERT_VALARG_TP assertions, GenTree* indir); bool optWriteBarrierAssertionProp_StoreInd(ASSERT_VALARG_TP assertions, GenTreeStoreInd* indir); - bool optAssertionPropPhiDefNotNull(BasicBlock* defBlock, GenTreePhi* phi); + bool optAssertionPropPhiDefNotNull(BasicBlock* block, GenTreePhi* phi); void optAssertionProp_RangeProperties(ASSERT_VALARG_TP assertions, GenTree* tree, From 787203ae48da612888b84b8c8c70a9f76af694a3 Mon Sep 17 00:00:00 2001 From: EgorBo Date: Thu, 30 Jan 2025 02:04:04 +0100 Subject: [PATCH 04/11] clean up --- src/coreclr/jit/assertionprop.cpp | 97 ++++++++++++++++++------------- src/coreclr/jit/compiler.h | 2 + src/coreclr/jit/rangecheck.cpp | 21 +------ 3 files changed, 59 insertions(+), 61 deletions(-) diff --git a/src/coreclr/jit/assertionprop.cpp b/src/coreclr/jit/assertionprop.cpp index 5f8b6e590d1dfb..e5872e33f42ff9 100644 --- a/src/coreclr/jit/assertionprop.cpp +++ b/src/coreclr/jit/assertionprop.cpp @@ -4372,60 +4372,55 @@ bool Compiler::optAssertionPropPhiDefNotNull(BasicBlock* block, GenTreePhi* phi) assert(phi != nullptr); assert(block != nullptr); + // NOTE: We might want to use VNVisitReachingVNs instead here, but we need to + // extend it to propagate BasicBlock preds for phi args. + // For now, we will iterate over the physical PhiArgs on the Phi node. + + JITDUMP("Checking if phi [%06u] is non-null\n", phi->gtTreeID); + // Enumerate phi's args for (GenTreePhi::Use& use : phi->Uses()) { - if (!use.GetNode()->OperIs(GT_PHI_ARG)) + GenTreePhiArg* phiArg = use.GetNode()->AsPhiArg(); + BasicBlock* argPredBb = phiArg->gtPredBB; + const ValueNum phiArgVN = vnStore->VNConservativeNormalValue(phiArg->gtVNPair); + + // Fast path - if VN tells us this particular phiArg is non-null, then we go to the next phiArg. + if (vnStore->IsKnownNonNull(phiArgVN)) { - return false; + continue; } - GenTreePhiArg* arg = use.GetNode()->AsPhiArg(); - BasicBlock* pred = arg->gtPredBB; - const ValueNum phiArgVN = vnStore->VNConservativeNormalValue(arg->gtVNPair); - // If this PhiArg is not known to be non-null, then maybe assertions can help. - if (!vnStore->IsKnownNonNull(phiArgVN)) + // + // Find the outgoing assertion for the predecessor block that targets this block. + ASSERT_TP assertions = optGetEdgeAssertions(argPredBb, block); + if (BitVecOps::MayBeUninit(assertions) || BitVecOps::IsEmpty(apTraits, assertions)) { - // Find the outgoing assertion for the predecessor block that targets this block. - ASSERT_TP assertions = BitVecOps::UninitVal(); - if (pred->KindIs(BBJ_COND) && pred->FalseTargetIs(block)) - { - assertions = pred->bbAssertionOut; - } - else if ((pred->KindIs(BBJ_ALWAYS) && pred->TargetIs(block)) || - (pred->KindIs(BBJ_COND) && pred->TrueTargetIs(block))) - { - if (bbJtrueAssertionOut != nullptr) - { - assertions = bbJtrueAssertionOut[pred->bbNum]; - } - } + return false; + } - if (BitVecOps::MayBeUninit(assertions) || BitVecOps::IsEmpty(apTraits, assertions)) - { - return false; - } + JITDUMP("Looking at assertions from pred " FMT_BB " ", argPredBb->bbNum); + optDumpAssertionIndices(assertions, "\n"); - // Iterate over the assertions and check if there is a "phiArgVN != null" assertion. - bool notNull = false; - BitVecOps::Iter iter(apTraits, assertions); - unsigned index = 0; - while (iter.NextElem(&index)) + // Iterate over the assertions and check if there is a "phiArgVN != null" assertion. + bool notNull = false; + BitVecOps::Iter iter(apTraits, assertions); + unsigned index = 0; + while (iter.NextElem(&index)) + { + AssertionDsc* curAssertion = optGetAssertion(GetAssertionIndex(index)); + if (curAssertion->CanPropNonNull() && curAssertion->op1.vn == phiArgVN) { - AssertionDsc* curAssertion = optGetAssertion(GetAssertionIndex(index)); - if (curAssertion->CanPropNonNull() && curAssertion->op1.vn == phiArgVN) - { - notNull = true; - break; - } + notNull = true; + break; } + } - if (!notNull) - { - // We can't prove that the phiArg is non-null - bail out. - return false; - } + if (!notNull) + { + // We can't prove that the phiArg is non-null - bail out. + return false; } } return true; @@ -4532,7 +4527,8 @@ GenTree* Compiler::optAssertionPropGlobal_RelOp(ASSERT_VALARG_TP assertions, Gen // See if we have "PHI ==/!= null" tree. In that case, we can use optAssertionPropPhiDefNotNull // to iterate over all the phi args and check if they are non-null. If all of them are non-null, // then we can fold this tree to true/false. - if (op2->IsIntegralConst(0) && varTypeIsGC(op1) && op1->OperIs(GT_LCL_VAR) && op1->AsLclVarCommon()->HasSsaName()) + if (op2->IsIntegralConst(0) && op1->TypeIs(TYP_REF) && op1->OperIs(GT_LCL_VAR) && + op1->AsLclVarCommon()->HasSsaName()) { // Get the definition of the local variable via SSA, we're looking for a phi definition. unsigned lclNum = op1->AsLclVarCommon()->GetLclNum(); @@ -5899,6 +5895,23 @@ ASSERT_VALRET_TP Compiler::optGetVnMappedAssertions(ValueNum vn) return BitVecOps::UninitVal(); } +ASSERT_VALRET_TP Compiler::optGetEdgeAssertions(const BasicBlock* pred, const BasicBlock* block) const +{ + if (pred->KindIs(BBJ_COND) && pred->FalseTargetIs(block)) + { + return pred->bbAssertionOut; + } + + if ((pred->KindIs(BBJ_ALWAYS) && pred->TargetIs(block)) || (pred->KindIs(BBJ_COND) && pred->TrueTargetIs(block))) + { + if (bbJtrueAssertionOut != nullptr) + { + return bbJtrueAssertionOut[pred->bbNum]; + } + } + return BitVecOps::UninitVal(); +} + /***************************************************************************** * * Given a const assertion this method computes the set of implied assertions diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index 6c79876cb331a0..073e1e204a3f43 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -8278,6 +8278,8 @@ class Compiler void optDebugCheckAssertions(AssertionIndex AssertionIndex); #endif + ASSERT_VALRET_TP optGetEdgeAssertions(const BasicBlock* pred, const BasicBlock* block) const; + static void optDumpAssertionIndices(const char* header, ASSERT_TP assertions, const char* footer = nullptr); static void optDumpAssertionIndices(ASSERT_TP assertions, const char* footer = nullptr); diff --git a/src/coreclr/jit/rangecheck.cpp b/src/coreclr/jit/rangecheck.cpp index 7a5dfcafc421b7..4bf0a99f5e9e93 100644 --- a/src/coreclr/jit/rangecheck.cpp +++ b/src/coreclr/jit/rangecheck.cpp @@ -977,26 +977,9 @@ void RangeCheck::MergeAssertion(BasicBlock* block, GenTree* op, Range* pRange DE ASSERT_TP assertions = BitVecOps::UninitVal(); // If we have a phi arg, we can get to the block from it and use its assertion out. - if (op->gtOper == GT_PHI_ARG) + if (op->OperIs(GT_PHI_ARG)) { - GenTreePhiArg* arg = (GenTreePhiArg*)op; - BasicBlock* pred = arg->gtPredBB; - if (pred->KindIs(BBJ_COND) && pred->FalseTargetIs(block)) - { - assertions = pred->bbAssertionOut; - JITDUMP("Merge assertions from pred " FMT_BB " edge: ", pred->bbNum); - Compiler::optDumpAssertionIndices(assertions, "\n"); - } - else if ((pred->KindIs(BBJ_ALWAYS) && pred->TargetIs(block)) || - (pred->KindIs(BBJ_COND) && pred->TrueTargetIs(block))) - { - if (m_pCompiler->bbJtrueAssertionOut != nullptr) - { - assertions = m_pCompiler->bbJtrueAssertionOut[pred->bbNum]; - JITDUMP("Merge assertions from pred " FMT_BB " JTrue edge: ", pred->bbNum); - Compiler::optDumpAssertionIndices(assertions, "\n"); - } - } + assertions = m_pCompiler->optGetEdgeAssertions(op->AsPhiArg()->gtPredBB, block); } // Get assertions from bbAssertionIn. else if (op->IsLocal()) From 68bd4737b937bf158c43739c52979c3e74cbed5d Mon Sep 17 00:00:00 2001 From: EgorBo Date: Thu, 30 Jan 2025 06:13:19 +0100 Subject: [PATCH 05/11] clean up --- src/coreclr/jit/assertionprop.cpp | 59 ++++++++++++++++++++++--------- src/coreclr/jit/compiler.h | 4 +-- src/coreclr/jit/rangecheck.cpp | 8 ++++- 3 files changed, 51 insertions(+), 20 deletions(-) diff --git a/src/coreclr/jit/assertionprop.cpp b/src/coreclr/jit/assertionprop.cpp index e5872e33f42ff9..bbcabff83c1b31 100644 --- a/src/coreclr/jit/assertionprop.cpp +++ b/src/coreclr/jit/assertionprop.cpp @@ -4367,23 +4367,34 @@ GenTree* Compiler::optAssertionProp_RelOp(ASSERT_VALARG_TP assertions, GenTree* return optAssertionPropLocal_RelOp(assertions, tree, stmt); } -bool Compiler::optAssertionPropPhiDefNotNull(BasicBlock* block, GenTreePhi* phi) +//------------------------------------------------------------------------ +// optAssertionIsNonNullPhi: see if we can prove a GT_PHI will be non-null +// based on assertions +// +// Arguments: +// block - block containing the PHI +// phi - PHI node to check +// +// Return Value: +// true if the tree's value will be non-null +// +bool Compiler::optAssertionIsNonNullPhi(const BasicBlock* block, GenTreePhi* phi) { assert(phi != nullptr); assert(block != nullptr); + assert(!optLocalAssertionProp); // PHI nodes are not present in local assertion prop. // NOTE: We might want to use VNVisitReachingVNs instead here, but we need to // extend it to propagate BasicBlock preds for phi args. // For now, we will iterate over the physical PhiArgs on the Phi node. - JITDUMP("Checking if phi [%06u] is non-null\n", phi->gtTreeID); + JITDUMP("Checking if PHI [%06u] is non-null\n", phi->gtTreeID); // Enumerate phi's args for (GenTreePhi::Use& use : phi->Uses()) { - GenTreePhiArg* phiArg = use.GetNode()->AsPhiArg(); - BasicBlock* argPredBb = phiArg->gtPredBB; - const ValueNum phiArgVN = vnStore->VNConservativeNormalValue(phiArg->gtVNPair); + GenTreePhiArg* phiArg = use.GetNode()->AsPhiArg(); + const ValueNum phiArgVN = vnStore->VNConservativeNormalValue(phiArg->gtVNPair); // Fast path - if VN tells us this particular phiArg is non-null, then we go to the next phiArg. if (vnStore->IsKnownNonNull(phiArgVN)) @@ -4394,13 +4405,14 @@ bool Compiler::optAssertionPropPhiDefNotNull(BasicBlock* block, GenTreePhi* phi) // If this PhiArg is not known to be non-null, then maybe assertions can help. // // Find the outgoing assertion for the predecessor block that targets this block. - ASSERT_TP assertions = optGetEdgeAssertions(argPredBb, block); + ASSERT_TP assertions = optGetEdgeAssertions(block, phiArg->gtPredBB); if (BitVecOps::MayBeUninit(assertions) || BitVecOps::IsEmpty(apTraits, assertions)) { + JITDUMP("No assertions for pred " FMT_BB " ", phiArg->gtPredBB->bbNum); return false; } - JITDUMP("Looking at assertions from pred " FMT_BB " ", argPredBb->bbNum); + JITDUMP("Looking at assertions from pred " FMT_BB " ", phiArg->gtPredBB->bbNum); optDumpAssertionIndices(assertions, "\n"); // Iterate over the assertions and check if there is a "phiArgVN != null" assertion. @@ -4410,7 +4422,7 @@ bool Compiler::optAssertionPropPhiDefNotNull(BasicBlock* block, GenTreePhi* phi) while (iter.NextElem(&index)) { AssertionDsc* curAssertion = optGetAssertion(GetAssertionIndex(index)); - if (curAssertion->CanPropNonNull() && curAssertion->op1.vn == phiArgVN) + if (curAssertion->CanPropNonNull() && (curAssertion->op1.vn == phiArgVN)) { notNull = true; break; @@ -4420,9 +4432,12 @@ bool Compiler::optAssertionPropPhiDefNotNull(BasicBlock* block, GenTreePhi* phi) if (!notNull) { // We can't prove that the phiArg is non-null - bail out. + JITDUMP("PHI's argument [%06u] is not known to be non-null - bail out.\n", phiArg->gtTreeID); return false; } } + + JITDUMP("... all of PHI's arguments are never null!\n", phi->gtTreeID); return true; } @@ -4524,7 +4539,7 @@ GenTree* Compiler::optAssertionPropGlobal_RelOp(ASSERT_VALARG_TP assertions, Gen return nullptr; } - // See if we have "PHI ==/!= null" tree. In that case, we can use optAssertionPropPhiDefNotNull + // See if we have "PHI ==/!= null" tree. In that case, we can use optAssertionIsNonNullPhi // to iterate over all the phi args and check if they are non-null. If all of them are non-null, // then we can fold this tree to true/false. if (op2->IsIntegralConst(0) && op1->TypeIs(TYP_REF) && op1->OperIs(GT_LCL_VAR) && @@ -4537,12 +4552,10 @@ GenTree* Compiler::optAssertionPropGlobal_RelOp(ASSERT_VALARG_TP assertions, Gen GenTreeLclVarCommon* node = ssaDef->GetDefNode(); if (node != nullptr && node->OperIs(GT_STORE_LCL_VAR) && node->Data()->OperIs(GT_PHI)) { - if (optAssertionPropPhiDefNotNull(ssaDef->GetBlock(), node->Data()->AsPhi())) + if (optAssertionIsNonNullPhi(ssaDef->GetBlock(), node->Data()->AsPhi())) { assert(newTree->OperIs(GT_EQ, GT_NE)); newTree = tree->OperIs(GT_EQ) ? gtNewIconNode(0) : gtNewIconNode(1); - newTree = gtWrapWithSideEffects(newTree, tree, GTF_ALL_EFFECT); - DISPTREE(newTree); return optAssertionProp_Update(newTree, tree, stmt); } } @@ -5895,18 +5908,30 @@ ASSERT_VALRET_TP Compiler::optGetVnMappedAssertions(ValueNum vn) return BitVecOps::UninitVal(); } -ASSERT_VALRET_TP Compiler::optGetEdgeAssertions(const BasicBlock* pred, const BasicBlock* block) const +//------------------------------------------------------------------------ +// optGetEdgeAssertions: Given a block and its predecessor, get the assertions +// the predecessor creates for the block. +// +// Arguments: +// block - The block to get the assertions for. +// blockPred - The predecessor of the block (creating the assertions). +// +// Return Value: +// The assertions we have about the value number. +// +ASSERT_VALRET_TP Compiler::optGetEdgeAssertions(const BasicBlock* block, const BasicBlock* blockPred) const { - if (pred->KindIs(BBJ_COND) && pred->FalseTargetIs(block)) + if (blockPred->KindIs(BBJ_COND) && blockPred->FalseTargetIs(block)) { - return pred->bbAssertionOut; + return blockPred->bbAssertionOut; } - if ((pred->KindIs(BBJ_ALWAYS) && pred->TargetIs(block)) || (pred->KindIs(BBJ_COND) && pred->TrueTargetIs(block))) + if ((blockPred->KindIs(BBJ_ALWAYS) && blockPred->TargetIs(block)) || + (blockPred->KindIs(BBJ_COND) && blockPred->TrueTargetIs(block))) { if (bbJtrueAssertionOut != nullptr) { - return bbJtrueAssertionOut[pred->bbNum]; + return bbJtrueAssertionOut[blockPred->bbNum]; } } return BitVecOps::UninitVal(); diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index 073e1e204a3f43..a49fcc41fdc5d3 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -8257,7 +8257,7 @@ class Compiler bool optNonNullAssertionProp_Ind(ASSERT_VALARG_TP assertions, GenTree* indir); bool optWriteBarrierAssertionProp_StoreInd(ASSERT_VALARG_TP assertions, GenTreeStoreInd* indir); - bool optAssertionPropPhiDefNotNull(BasicBlock* block, GenTreePhi* phi); + bool optAssertionIsNonNullPhi(const BasicBlock* block, GenTreePhi* phi); void optAssertionProp_RangeProperties(ASSERT_VALARG_TP assertions, GenTree* tree, @@ -8278,7 +8278,7 @@ class Compiler void optDebugCheckAssertions(AssertionIndex AssertionIndex); #endif - ASSERT_VALRET_TP optGetEdgeAssertions(const BasicBlock* pred, const BasicBlock* block) const; + ASSERT_VALRET_TP optGetEdgeAssertions(const BasicBlock* block, const BasicBlock* blockPred) const; static void optDumpAssertionIndices(const char* header, ASSERT_TP assertions, const char* footer = nullptr); static void optDumpAssertionIndices(ASSERT_TP assertions, const char* footer = nullptr); diff --git a/src/coreclr/jit/rangecheck.cpp b/src/coreclr/jit/rangecheck.cpp index 4bf0a99f5e9e93..6d022778e198cd 100644 --- a/src/coreclr/jit/rangecheck.cpp +++ b/src/coreclr/jit/rangecheck.cpp @@ -979,7 +979,13 @@ void RangeCheck::MergeAssertion(BasicBlock* block, GenTree* op, Range* pRange DE // If we have a phi arg, we can get to the block from it and use its assertion out. if (op->OperIs(GT_PHI_ARG)) { - assertions = m_pCompiler->optGetEdgeAssertions(op->AsPhiArg()->gtPredBB, block); + const BasicBlock* pred = op->AsPhiArg()->gtPredBB; + assertions = m_pCompiler->optGetEdgeAssertions(block, pred); + if (!BitVecOps::MayBeUninit(assertions)) + { + JITDUMP("Merge assertions created by " FMT_BB " for " FMT_BB "\n", pred->bbNum, block->bbNum); + Compiler::optDumpAssertionIndices(assertions, "\n"); + } } // Get assertions from bbAssertionIn. else if (op->IsLocal()) From a349dd7696cf07087b360f58be0a59afc8b135bb Mon Sep 17 00:00:00 2001 From: EgorBo Date: Thu, 30 Jan 2025 07:14:15 +0100 Subject: [PATCH 06/11] clean up --- src/coreclr/jit/assertionprop.cpp | 139 +++++++----------------------- src/coreclr/jit/compiler.h | 2 - 2 files changed, 32 insertions(+), 109 deletions(-) diff --git a/src/coreclr/jit/assertionprop.cpp b/src/coreclr/jit/assertionprop.cpp index bbcabff83c1b31..a14bc55cfecde3 100644 --- a/src/coreclr/jit/assertionprop.cpp +++ b/src/coreclr/jit/assertionprop.cpp @@ -4367,80 +4367,6 @@ GenTree* Compiler::optAssertionProp_RelOp(ASSERT_VALARG_TP assertions, GenTree* return optAssertionPropLocal_RelOp(assertions, tree, stmt); } -//------------------------------------------------------------------------ -// optAssertionIsNonNullPhi: see if we can prove a GT_PHI will be non-null -// based on assertions -// -// Arguments: -// block - block containing the PHI -// phi - PHI node to check -// -// Return Value: -// true if the tree's value will be non-null -// -bool Compiler::optAssertionIsNonNullPhi(const BasicBlock* block, GenTreePhi* phi) -{ - assert(phi != nullptr); - assert(block != nullptr); - assert(!optLocalAssertionProp); // PHI nodes are not present in local assertion prop. - - // NOTE: We might want to use VNVisitReachingVNs instead here, but we need to - // extend it to propagate BasicBlock preds for phi args. - // For now, we will iterate over the physical PhiArgs on the Phi node. - - JITDUMP("Checking if PHI [%06u] is non-null\n", phi->gtTreeID); - - // Enumerate phi's args - for (GenTreePhi::Use& use : phi->Uses()) - { - GenTreePhiArg* phiArg = use.GetNode()->AsPhiArg(); - const ValueNum phiArgVN = vnStore->VNConservativeNormalValue(phiArg->gtVNPair); - - // Fast path - if VN tells us this particular phiArg is non-null, then we go to the next phiArg. - if (vnStore->IsKnownNonNull(phiArgVN)) - { - continue; - } - - // If this PhiArg is not known to be non-null, then maybe assertions can help. - // - // Find the outgoing assertion for the predecessor block that targets this block. - ASSERT_TP assertions = optGetEdgeAssertions(block, phiArg->gtPredBB); - if (BitVecOps::MayBeUninit(assertions) || BitVecOps::IsEmpty(apTraits, assertions)) - { - JITDUMP("No assertions for pred " FMT_BB " ", phiArg->gtPredBB->bbNum); - return false; - } - - JITDUMP("Looking at assertions from pred " FMT_BB " ", phiArg->gtPredBB->bbNum); - optDumpAssertionIndices(assertions, "\n"); - - // Iterate over the assertions and check if there is a "phiArgVN != null" assertion. - bool notNull = false; - BitVecOps::Iter iter(apTraits, assertions); - unsigned index = 0; - while (iter.NextElem(&index)) - { - AssertionDsc* curAssertion = optGetAssertion(GetAssertionIndex(index)); - if (curAssertion->CanPropNonNull() && (curAssertion->op1.vn == phiArgVN)) - { - notNull = true; - break; - } - } - - if (!notNull) - { - // We can't prove that the phiArg is non-null - bail out. - JITDUMP("PHI's argument [%06u] is not known to be non-null - bail out.\n", phiArg->gtTreeID); - return false; - } - } - - JITDUMP("... all of PHI's arguments are never null!\n", phi->gtTreeID); - return true; -} - //------------------------------------------------------------------------ // optAssertionProp: try and optimize a relop via assertion propagation // @@ -4539,21 +4465,32 @@ GenTree* Compiler::optAssertionPropGlobal_RelOp(ASSERT_VALARG_TP assertions, Gen return nullptr; } - // See if we have "PHI ==/!= null" tree. In that case, we can use optAssertionIsNonNullPhi - // to iterate over all the phi args and check if they are non-null. If all of them are non-null, - // then we can fold this tree to true/false. - if (op2->IsIntegralConst(0) && op1->TypeIs(TYP_REF) && op1->OperIs(GT_LCL_VAR) && - op1->AsLclVarCommon()->HasSsaName()) + // See if we have "PHI ==/!= null" tree. If so, we iterate over all PHI's arguments, + // and if all of them are known to be non-null, we can bash the comparison to true/false. + if (op2->IsIntegralConst(0) && op1->TypeIs(TYP_REF) && op1->OperIs(GT_LCL_VAR) && op1->AsLclVar()->HasSsaName()) { - // Get the definition of the local variable via SSA, we're looking for a phi definition. - unsigned lclNum = op1->AsLclVarCommon()->GetLclNum(); - unsigned ssaNum = op1->AsLclVarCommon()->GetSsaNum(); - LclSsaVarDsc* ssaDef = lvaGetDesc(lclNum)->GetPerSsaData(ssaNum); - GenTreeLclVarCommon* node = ssaDef->GetDefNode(); - if (node != nullptr && node->OperIs(GT_STORE_LCL_VAR) && node->Data()->OperIs(GT_PHI)) + LclSsaVarDsc* ssaDef = lvaGetDesc(op1->AsLclVar()->GetLclNum())->GetPerSsaData(op1->AsLclVar()->GetSsaNum()); + GenTreeLclVarCommon* node = ssaDef->GetDefNode(); + if ((node != nullptr) && node->OperIs(GT_STORE_LCL_VAR) && node->Data()->OperIs(GT_PHI)) { - if (optAssertionIsNonNullPhi(ssaDef->GetBlock(), node->Data()->AsPhi())) + JITDUMP("Checking if PHI [%06u] is non-null\n", node->Data()->gtTreeID); + + bool nonNull = true; + for (GenTreePhi::Use& use : node->Data()->AsPhi()->Uses()) + { + GenTreePhiArg* phiArg = use.GetNode()->AsPhiArg(); + const ValueNum phiArgVN = vnStore->VNConservativeNormalValue(phiArg->gtVNPair); + if (!optAssertionVNIsNonNull(phiArgVN, optGetEdgeAssertions(ssaDef->GetBlock(), phiArg->gtPredBB))) + { + JITDUMP("... PHI's argument [%06u] is not known to be non-null - bail out.\n", phiArg->gtTreeID); + nonNull = false; + break; + } + } + + if (nonNull) { + JITDUMP("... all of PHI's arguments are never null!\n"); assert(newTree->OperIs(GT_EQ, GT_NE)); newTree = tree->OperIs(GT_EQ) ? gtNewIconNode(0) : gtNewIconNode(1); return optAssertionProp_Update(newTree, tree, stmt); @@ -5181,31 +5118,19 @@ bool Compiler::optAssertionVNIsNonNull(ValueNum vn, ASSERT_VALARG_TP assertions) return true; } - // Check each assertion to find if we have a vn != null assertion. - // - BitVecOps::Iter iter(apTraits, assertions); - unsigned index = 0; - while (iter.NextElem(&index)) + if (!BitVecOps::MayBeUninit(assertions)) { - AssertionIndex assertionIndex = GetAssertionIndex(index); - if (assertionIndex > optAssertionCount) - { - break; - } - AssertionDsc* curAssertion = optGetAssertion(assertionIndex); - if (!curAssertion->CanPropNonNull()) - { - continue; - } - - if (curAssertion->op1.vn != vn) + BitVecOps::Iter iter(apTraits, assertions); + unsigned index = 0; + while (iter.NextElem(&index)) { - continue; + AssertionDsc* curAssertion = optGetAssertion(GetAssertionIndex(index)); + if (curAssertion->CanPropNonNull() && curAssertion->op1.vn == vn) + { + return true; + } } - - return true; } - return false; } diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index a49fcc41fdc5d3..0f327b76af5b03 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -8257,8 +8257,6 @@ class Compiler bool optNonNullAssertionProp_Ind(ASSERT_VALARG_TP assertions, GenTree* indir); bool optWriteBarrierAssertionProp_StoreInd(ASSERT_VALARG_TP assertions, GenTreeStoreInd* indir); - bool optAssertionIsNonNullPhi(const BasicBlock* block, GenTreePhi* phi); - void optAssertionProp_RangeProperties(ASSERT_VALARG_TP assertions, GenTree* tree, bool* isKnownNonZero, From b85ba3815c6821b825e9f68897edc0010084f7c9 Mon Sep 17 00:00:00 2001 From: EgorBo Date: Thu, 30 Jan 2025 15:53:55 +0100 Subject: [PATCH 07/11] clean up --- src/coreclr/jit/assertionprop.cpp | 89 ++++++++++++++++++++++--------- src/coreclr/jit/compiler.h | 8 +++ 2 files changed, 71 insertions(+), 26 deletions(-) diff --git a/src/coreclr/jit/assertionprop.cpp b/src/coreclr/jit/assertionprop.cpp index a14bc55cfecde3..2910e24625534e 100644 --- a/src/coreclr/jit/assertionprop.cpp +++ b/src/coreclr/jit/assertionprop.cpp @@ -4367,6 +4367,60 @@ GenTree* Compiler::optAssertionProp_RelOp(ASSERT_VALARG_TP assertions, GenTree* return optAssertionPropLocal_RelOp(assertions, tree, stmt); } +//-------------------------------------------------------------------------------- +// optVisitReachingAssertions: given a tree, call the specified callback function on all +// the assertions that reach it via PHI definitions if any. +// +// Arguments: +// GenTree - The tree to visit all the reaching assertions for +// argVisitor - The callback function to call on the vn and its reaching assertions +// +// Return Value: +// AssertVisit::Aborted - an argVisitor returned AssertVisit::Abort, we stop the walk and return +// AssertVisit::Continue - all argVisitor returned AssertVisit::Continue +// +template +Compiler::AssertVisit Compiler::optVisitReachingAssertions(GenTree* tree, TAssertVisitor argVisitor) +{ + // It is better to rely only on VN and do what VNVisitReachingVNs does, but it is tricky + // to propagate BBs there. So, we do a simple walk here (no nested PHI nodes). + // + // We assume that the caller already checked aggregated assertions for the current block, + // so we're interested only in reaching assertions via PHI nodes. + // + if (!tree->OperIs(GT_LCL_VAR) || !tree->AsLclVar()->HasSsaName()) + { + return AssertVisit::Abort; + } + + GenTreeLclVarCommon* lcl = tree->AsLclVarCommon(); + LclSsaVarDsc* ssaDef = lvaGetDesc(lcl->GetLclNum())->GetPerSsaData(lcl->GetSsaNum()); + GenTreeLclVarCommon* node = ssaDef->GetDefNode(); + + if ((node == nullptr) || !node->OperIs(GT_STORE_LCL_VAR) || !node->Data()->OperIs(GT_PHI)) + { + return AssertVisit::Abort; + } + + for (GenTreePhi::Use& use : node->Data()->AsPhi()->Uses()) + { + GenTreePhiArg* phiArg = use.GetNode()->AsPhiArg(); + const ValueNum phiArgVN = vnStore->VNConservativeNormalValue(phiArg->gtVNPair); + ASSERT_TP assertions = optGetEdgeAssertions(ssaDef->GetBlock(), phiArg->gtPredBB); + if (BitVecOps::MayBeUninit(assertions)) + { + return AssertVisit::Abort; + } + + if (argVisitor(phiArgVN, assertions) == AssertVisit::Abort) + { + // The visitor wants to abort the walk. + return AssertVisit::Abort; + } + } + return AssertVisit::Continue; +} + //------------------------------------------------------------------------ // optAssertionProp: try and optimize a relop via assertion propagation // @@ -4467,34 +4521,17 @@ GenTree* Compiler::optAssertionPropGlobal_RelOp(ASSERT_VALARG_TP assertions, Gen // See if we have "PHI ==/!= null" tree. If so, we iterate over all PHI's arguments, // and if all of them are known to be non-null, we can bash the comparison to true/false. - if (op2->IsIntegralConst(0) && op1->TypeIs(TYP_REF) && op1->OperIs(GT_LCL_VAR) && op1->AsLclVar()->HasSsaName()) + if (op2->IsIntegralConst(0) && op1->TypeIs(TYP_REF)) { - LclSsaVarDsc* ssaDef = lvaGetDesc(op1->AsLclVar()->GetLclNum())->GetPerSsaData(op1->AsLclVar()->GetSsaNum()); - GenTreeLclVarCommon* node = ssaDef->GetDefNode(); - if ((node != nullptr) && node->OperIs(GT_STORE_LCL_VAR) && node->Data()->OperIs(GT_PHI)) + auto visitor = [this](ValueNum reachingVN, ASSERT_TP reachingAssertions) { + return optAssertionVNIsNonNull(reachingVN, reachingAssertions) ? AssertVisit::Continue : AssertVisit::Abort; + }; + if (optVisitReachingAssertions(op1, visitor) == AssertVisit::Continue) { - JITDUMP("Checking if PHI [%06u] is non-null\n", node->Data()->gtTreeID); - - bool nonNull = true; - for (GenTreePhi::Use& use : node->Data()->AsPhi()->Uses()) - { - GenTreePhiArg* phiArg = use.GetNode()->AsPhiArg(); - const ValueNum phiArgVN = vnStore->VNConservativeNormalValue(phiArg->gtVNPair); - if (!optAssertionVNIsNonNull(phiArgVN, optGetEdgeAssertions(ssaDef->GetBlock(), phiArg->gtPredBB))) - { - JITDUMP("... PHI's argument [%06u] is not known to be non-null - bail out.\n", phiArg->gtTreeID); - nonNull = false; - break; - } - } - - if (nonNull) - { - JITDUMP("... all of PHI's arguments are never null!\n"); - assert(newTree->OperIs(GT_EQ, GT_NE)); - newTree = tree->OperIs(GT_EQ) ? gtNewIconNode(0) : gtNewIconNode(1); - return optAssertionProp_Update(newTree, tree, stmt); - } + JITDUMP("... all of PHI's arguments are never null!\n"); + assert(newTree->OperIs(GT_EQ, GT_NE)); + newTree = tree->OperIs(GT_EQ) ? gtNewIconNode(0) : gtNewIconNode(1); + return optAssertionProp_Update(newTree, tree, stmt); } } diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index 0f327b76af5b03..0c8534093050e6 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -8257,6 +8257,14 @@ class Compiler bool optNonNullAssertionProp_Ind(ASSERT_VALARG_TP assertions, GenTree* indir); bool optWriteBarrierAssertionProp_StoreInd(ASSERT_VALARG_TP assertions, GenTreeStoreInd* indir); + enum class AssertVisit + { + Continue, + Abort, + }; + template + AssertVisit optVisitReachingAssertions(GenTree* tree, TAssertVisitor argVisitor); + void optAssertionProp_RangeProperties(ASSERT_VALARG_TP assertions, GenTree* tree, bool* isKnownNonZero, From 1cf58fbd59f9e7509ee70a377155ce621e2b574f Mon Sep 17 00:00:00 2001 From: EgorBo Date: Thu, 30 Jan 2025 18:01:31 +0100 Subject: [PATCH 08/11] Address feedback --- src/coreclr/jit/assertionprop.cpp | 48 +++++++++++++------------------ src/coreclr/jit/compiler.h | 2 +- 2 files changed, 21 insertions(+), 29 deletions(-) diff --git a/src/coreclr/jit/assertionprop.cpp b/src/coreclr/jit/assertionprop.cpp index 2910e24625534e..abb1b1c7df46d8 100644 --- a/src/coreclr/jit/assertionprop.cpp +++ b/src/coreclr/jit/assertionprop.cpp @@ -4368,11 +4368,11 @@ GenTree* Compiler::optAssertionProp_RelOp(ASSERT_VALARG_TP assertions, GenTree* } //-------------------------------------------------------------------------------- -// optVisitReachingAssertions: given a tree, call the specified callback function on all +// optVisitReachingAssertions: given a vn, call the specified callback function on all // the assertions that reach it via PHI definitions if any. // // Arguments: -// GenTree - The tree to visit all the reaching assertions for +// vn - The vn to visit all the reaching assertions for // argVisitor - The callback function to call on the vn and its reaching assertions // // Return Value: @@ -4380,23 +4380,18 @@ GenTree* Compiler::optAssertionProp_RelOp(ASSERT_VALARG_TP assertions, GenTree* // AssertVisit::Continue - all argVisitor returned AssertVisit::Continue // template -Compiler::AssertVisit Compiler::optVisitReachingAssertions(GenTree* tree, TAssertVisitor argVisitor) +Compiler::AssertVisit Compiler::optVisitReachingAssertions(ValueNum vn, TAssertVisitor argVisitor) { - // It is better to rely only on VN and do what VNVisitReachingVNs does, but it is tricky - // to propagate BBs there. So, we do a simple walk here (no nested PHI nodes). - // - // We assume that the caller already checked aggregated assertions for the current block, - // so we're interested only in reaching assertions via PHI nodes. - // - if (!tree->OperIs(GT_LCL_VAR) || !tree->AsLclVar()->HasSsaName()) + VNPhiDef phiDef; + if (!vnStore->GetPhiDef(vn, &phiDef)) { + // We assume that the caller already checked assertions for the current block, so we're + // interested only in assertions for PHI definitions. return AssertVisit::Abort; } - GenTreeLclVarCommon* lcl = tree->AsLclVarCommon(); - LclSsaVarDsc* ssaDef = lvaGetDesc(lcl->GetLclNum())->GetPerSsaData(lcl->GetSsaNum()); - GenTreeLclVarCommon* node = ssaDef->GetDefNode(); - + LclSsaVarDsc* ssaDef = lvaGetDesc(phiDef.LclNum)->GetPerSsaData(phiDef.SsaDef); + GenTreeLclVarCommon* node = ssaDef->GetDefNode(); if ((node == nullptr) || !node->OperIs(GT_STORE_LCL_VAR) || !node->Data()->OperIs(GT_PHI)) { return AssertVisit::Abort; @@ -4407,11 +4402,6 @@ Compiler::AssertVisit Compiler::optVisitReachingAssertions(GenTree* tree, TAsser GenTreePhiArg* phiArg = use.GetNode()->AsPhiArg(); const ValueNum phiArgVN = vnStore->VNConservativeNormalValue(phiArg->gtVNPair); ASSERT_TP assertions = optGetEdgeAssertions(ssaDef->GetBlock(), phiArg->gtPredBB); - if (BitVecOps::MayBeUninit(assertions)) - { - return AssertVisit::Abort; - } - if (argVisitor(phiArgVN, assertions) == AssertVisit::Abort) { // The visitor wants to abort the walk. @@ -4526,7 +4516,9 @@ GenTree* Compiler::optAssertionPropGlobal_RelOp(ASSERT_VALARG_TP assertions, Gen auto visitor = [this](ValueNum reachingVN, ASSERT_TP reachingAssertions) { return optAssertionVNIsNonNull(reachingVN, reachingAssertions) ? AssertVisit::Continue : AssertVisit::Abort; }; - if (optVisitReachingAssertions(op1, visitor) == AssertVisit::Continue) + + ValueNum op1vn = vnStore->VNConservativeNormalValue(op1->gtVNPair); + if (optVisitReachingAssertions(op1vn, visitor) == AssertVisit::Continue) { JITDUMP("... all of PHI's arguments are never null!\n"); assert(newTree->OperIs(GT_EQ, GT_NE)); @@ -5883,20 +5875,20 @@ ASSERT_VALRET_TP Compiler::optGetVnMappedAssertions(ValueNum vn) // ASSERT_VALRET_TP Compiler::optGetEdgeAssertions(const BasicBlock* block, const BasicBlock* blockPred) const { - if (blockPred->KindIs(BBJ_COND) && blockPred->FalseTargetIs(block)) - { - return blockPred->bbAssertionOut; - } - - if ((blockPred->KindIs(BBJ_ALWAYS) && blockPred->TargetIs(block)) || - (blockPred->KindIs(BBJ_COND) && blockPred->TrueTargetIs(block))) + if ((blockPred->KindIs(BBJ_COND) && blockPred->TrueTargetIs(block))) { if (bbJtrueAssertionOut != nullptr) { return bbJtrueAssertionOut[blockPred->bbNum]; } } - return BitVecOps::UninitVal(); + + if (blockPred->KindIs(BBJ_ALWAYS, BBJ_COND)) + { + return blockPred->bbAssertionOut; + } + + return BitVecOps::MakeEmpty(apTraits); } /***************************************************************************** diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index 0c8534093050e6..77b84d2495db95 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -8263,7 +8263,7 @@ class Compiler Abort, }; template - AssertVisit optVisitReachingAssertions(GenTree* tree, TAssertVisitor argVisitor); + AssertVisit optVisitReachingAssertions(ValueNum vn, TAssertVisitor argVisitor); void optAssertionProp_RangeProperties(ASSERT_VALARG_TP assertions, GenTree* tree, From 49a1767e0a32ac997cce2b815802a45ac882d3ff Mon Sep 17 00:00:00 2001 From: EgorBo Date: Thu, 30 Jan 2025 18:16:04 +0100 Subject: [PATCH 09/11] formatting --- src/coreclr/jit/assertionprop.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/coreclr/jit/assertionprop.cpp b/src/coreclr/jit/assertionprop.cpp index abb1b1c7df46d8..57e3f615bca67d 100644 --- a/src/coreclr/jit/assertionprop.cpp +++ b/src/coreclr/jit/assertionprop.cpp @@ -4390,8 +4390,8 @@ Compiler::AssertVisit Compiler::optVisitReachingAssertions(ValueNum vn, TAssertV return AssertVisit::Abort; } - LclSsaVarDsc* ssaDef = lvaGetDesc(phiDef.LclNum)->GetPerSsaData(phiDef.SsaDef); - GenTreeLclVarCommon* node = ssaDef->GetDefNode(); + LclSsaVarDsc* ssaDef = lvaGetDesc(phiDef.LclNum)->GetPerSsaData(phiDef.SsaDef); + GenTreeLclVarCommon* node = ssaDef->GetDefNode(); if ((node == nullptr) || !node->OperIs(GT_STORE_LCL_VAR) || !node->Data()->OperIs(GT_PHI)) { return AssertVisit::Abort; From b55fceb33c51a9e4c361f8115e674b6e51edf420 Mon Sep 17 00:00:00 2001 From: Egor Bogatov Date: Thu, 30 Jan 2025 19:03:54 +0100 Subject: [PATCH 10/11] Update src/coreclr/jit/assertionprop.cpp Co-authored-by: Jakob Botsch Nielsen --- src/coreclr/jit/assertionprop.cpp | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/coreclr/jit/assertionprop.cpp b/src/coreclr/jit/assertionprop.cpp index 57e3f615bca67d..c2028229f523cb 100644 --- a/src/coreclr/jit/assertionprop.cpp +++ b/src/coreclr/jit/assertionprop.cpp @@ -4392,10 +4392,7 @@ Compiler::AssertVisit Compiler::optVisitReachingAssertions(ValueNum vn, TAssertV LclSsaVarDsc* ssaDef = lvaGetDesc(phiDef.LclNum)->GetPerSsaData(phiDef.SsaDef); GenTreeLclVarCommon* node = ssaDef->GetDefNode(); - if ((node == nullptr) || !node->OperIs(GT_STORE_LCL_VAR) || !node->Data()->OperIs(GT_PHI)) - { - return AssertVisit::Abort; - } + assert(node->IsPhiDefn()); for (GenTreePhi::Use& use : node->Data()->AsPhi()->Uses()) { From 36c355b5743b4edabf1af644dad385f0570d02f3 Mon Sep 17 00:00:00 2001 From: EgorBo Date: Thu, 30 Jan 2025 19:09:54 +0100 Subject: [PATCH 11/11] Address feedback --- src/coreclr/jit/assertionprop.cpp | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/src/coreclr/jit/assertionprop.cpp b/src/coreclr/jit/assertionprop.cpp index c2028229f523cb..ef91f916fb9563 100644 --- a/src/coreclr/jit/assertionprop.cpp +++ b/src/coreclr/jit/assertionprop.cpp @@ -5878,14 +5878,9 @@ ASSERT_VALRET_TP Compiler::optGetEdgeAssertions(const BasicBlock* block, const B { return bbJtrueAssertionOut[blockPred->bbNum]; } + return BitVecOps::MakeEmpty(apTraits); } - - if (blockPred->KindIs(BBJ_ALWAYS, BBJ_COND)) - { - return blockPred->bbAssertionOut; - } - - return BitVecOps::MakeEmpty(apTraits); + return blockPred->bbAssertionOut; } /*****************************************************************************