From cee558386aa021b2a139c3b744f2fa60cd3a1fd8 Mon Sep 17 00:00:00 2001 From: "Aman Khalid (from Dev Box)" Date: Wed, 13 Mar 2024 13:53:08 -0400 Subject: [PATCH 01/12] Remove compute edge weight phase --- src/coreclr/jit/block.h | 6 - src/coreclr/jit/compiler.cpp | 4 - src/coreclr/jit/compiler.h | 9 +- src/coreclr/jit/fgprofile.cpp | 629 ---------------------------------- 4 files changed, 4 insertions(+), 644 deletions(-) diff --git a/src/coreclr/jit/block.h b/src/coreclr/jit/block.h index cd2a79fd098a5..3489a18263b51 100644 --- a/src/coreclr/jit/block.h +++ b/src/coreclr/jit/block.h @@ -658,12 +658,6 @@ struct FlowEdge return m_edgeWeightMax; } - // These two methods are used to set new values for edge weights. - // They return false if the newWeight is not between the current [min..max] - // when slop is non-zero we allow for the case where our weights might be off by 'slop' - // - bool setEdgeWeightMinChecked(weight_t newWeight, BasicBlock* bDst, weight_t slop, bool* wbUsedSlop); - bool setEdgeWeightMaxChecked(weight_t newWeight, BasicBlock* bDst, weight_t slop, bool* wbUsedSlop); void setEdgeWeights(weight_t newMinWeight, weight_t newMaxWeight, BasicBlock* bDst); weight_t getLikelihood() const diff --git a/src/coreclr/jit/compiler.cpp b/src/coreclr/jit/compiler.cpp index 000f2f9ea952a..28adc6fc4dc15 100644 --- a/src/coreclr/jit/compiler.cpp +++ b/src/coreclr/jit/compiler.cpp @@ -5024,10 +5024,6 @@ void Compiler::compCompile(void** methodCodePtr, uint32_t* methodCodeSize, JitFl // update the flowgraph if we modified it during the optimization phase // DoPhase(this, PHASE_OPT_UPDATE_FLOW_GRAPH, &Compiler::fgUpdateFlowGraphPhase); - - // Recompute the edge weight if we have modified the flow graph - // - DoPhase(this, PHASE_COMPUTE_EDGE_WEIGHTS2, &Compiler::fgComputeEdgeWeights); } // Iterate if requested, resetting annotations first. diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index 2158721861b93..1616137ddff3e 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -5176,10 +5176,10 @@ class Compiler // the end of compilation. The first and last nodes are stored in the block. NodeThreading fgNodeThreading; bool fgCanRelocateEHRegions; // true if we are allowed to relocate the EH regions - bool fgEdgeWeightsComputed; // true after we have called fgComputeEdgeWeights - bool fgHaveValidEdgeWeights; // true if we were successful in computing all of the edge weights - bool fgSlopUsedInEdgeWeights; // true if their was some slop used when computing the edge weights - bool fgRangeUsedInEdgeWeights; // true if some of the edgeWeight are expressed in Min..Max form + bool fgEdgeWeightsComputed; // true after we have called fgComputeEdgeWeights TODO: remove + bool fgHaveValidEdgeWeights; // true if we were successful in computing all of the edge weights TODO: remove + bool fgSlopUsedInEdgeWeights; // true if their was some slop used when computing the edge weights TODO: remove + bool fgRangeUsedInEdgeWeights; // true if some of the edgeWeight are expressed in Min..Max form TODO: remove weight_t fgCalledCount; // count of the number of times this method was called // This is derived from the profile data // or is BB_UNITY_WEIGHT when we don't have profile data @@ -6015,7 +6015,6 @@ class Compiler PhaseStatus fgComputeBlockAndEdgeWeights(); bool fgComputeMissingBlockWeights(weight_t* returnWeight); bool fgComputeCalledCount(weight_t returnWeight); - PhaseStatus fgComputeEdgeWeights(); bool fgReorderBlocks(bool useProfile); diff --git a/src/coreclr/jit/fgprofile.cpp b/src/coreclr/jit/fgprofile.cpp index 82192c9f28ca6..fe82405ed7116 100644 --- a/src/coreclr/jit/fgprofile.cpp +++ b/src/coreclr/jit/fgprofile.cpp @@ -4248,229 +4248,6 @@ bool Compiler::fgIncorporateEdgeCounts() return e.IsGood(); } -//------------------------------------------------------------------------ -// setEdgeWeightMinChecked: possibly update minimum edge weight -// -// Arguments: -// newWeight - proposed new weight -// bDst - destination block for edge -// slop - profile slush fund -// wbUsedSlop [out] - true if we tapped into the slush fund -// -// Returns: -// true if the edge weight was adjusted -// false if the edge weight update was inconsistent with the -// edge's current [min,max} -// -bool FlowEdge::setEdgeWeightMinChecked(weight_t newWeight, BasicBlock* bDst, weight_t slop, bool* wbUsedSlop) -{ - // Negative weights are nonsensical. - // - // If we can't cover the deficit with slop, fail. - // If we can, set the new weight to zero. - // - bool usedSlop = false; - - if (newWeight < BB_ZERO_WEIGHT) - { - if ((newWeight + slop) < BB_ZERO_WEIGHT) - { - return false; - } - - newWeight = BB_ZERO_WEIGHT; - usedSlop = true; - } - - bool result = false; - - if ((newWeight <= m_edgeWeightMax) && (newWeight >= m_edgeWeightMin)) - { - m_edgeWeightMin = newWeight; - result = true; - } - else if (slop > 0) - { - // We allow for a small amount of inaccuracy in block weight counts. - if (m_edgeWeightMax < newWeight) - { - // We have already determined that this edge's weight - // is less than newWeight, so we just allow for the slop - if (newWeight <= (m_edgeWeightMax + slop)) - { - result = true; - usedSlop = true; - - if (m_edgeWeightMax != BB_ZERO_WEIGHT) - { - // We will raise m_edgeWeightMin and Max towards newWeight - m_edgeWeightMin = m_edgeWeightMax; - m_edgeWeightMax = newWeight; - } - } - } - else if (m_edgeWeightMin > newWeight) - { - // We have already determined that this edge's weight - // is more than newWeight, so we just allow for the slop - if ((newWeight + slop) >= m_edgeWeightMin) - { - result = true; - usedSlop = true; - - if (m_edgeWeightMax != BB_ZERO_WEIGHT) - { - // We will lower m_edgeWeightMin towards newWeight - // But not below zero. - // - m_edgeWeightMin = max(BB_ZERO_WEIGHT, newWeight); - } - } - } - - // If we are returning true then we should have adjusted the range so that - // the newWeight is in new range [Min..Max] or fgEdgeWeightMax is zero. - // - if (result) - { - assert((m_edgeWeightMax == BB_ZERO_WEIGHT) || - ((newWeight <= m_edgeWeightMax) && (newWeight >= m_edgeWeightMin))); - } - } - - if (result && usedSlop && (wbUsedSlop != nullptr)) - { - *wbUsedSlop = true; - } - -#if DEBUG - if (result) - { - JITDUMP("Updated min weight of " FMT_BB " -> " FMT_BB " to [" FMT_WT ".." FMT_WT "]\n", getSourceBlock()->bbNum, - bDst->bbNum, m_edgeWeightMin, m_edgeWeightMax); - } - else - { - JITDUMP("Not adjusting min weight of " FMT_BB " -> " FMT_BB "; new value " FMT_WT " not in range [" FMT_WT - ".." FMT_WT "] (+/- " FMT_WT ")\n", - getSourceBlock()->bbNum, bDst->bbNum, newWeight, m_edgeWeightMin, m_edgeWeightMax, slop); - result = false; // break here - } -#endif // DEBUG - - return result; -} - -//------------------------------------------------------------------------ -// setEdgeWeightMaxChecked: possibly update maximum edge weight -// -// Arguments: -// newWeight - proposed new weight -// bDst - destination block for edge -// slop - profile slush fund -// wbUsedSlop [out] - true if we tapped into the slush fund -// -// Returns: -// true if the edge weight was adjusted -// false if the edge weight update was inconsistent with the -// edge's current [min,max} -// -bool FlowEdge::setEdgeWeightMaxChecked(weight_t newWeight, BasicBlock* bDst, weight_t slop, bool* wbUsedSlop) -{ - // Negative weights are nonsensical. - // - // If we can't cover the deficit with slop, fail. - // If we can, set the new weight to zero. - // - bool usedSlop = false; - - if (newWeight < BB_ZERO_WEIGHT) - { - if ((newWeight + slop) < BB_ZERO_WEIGHT) - { - return false; - } - - newWeight = BB_ZERO_WEIGHT; - usedSlop = true; - } - - bool result = false; - - if ((newWeight >= m_edgeWeightMin) && (newWeight <= m_edgeWeightMax)) - { - m_edgeWeightMax = newWeight; - result = true; - } - else if (slop > 0) - { - // We allow for a small amount of inaccuracy in block weight counts. - if (m_edgeWeightMax < newWeight) - { - // We have already determined that this edge's weight - // is less than newWeight, so we just allow for the slop - if (newWeight <= (m_edgeWeightMax + slop)) - { - result = true; - usedSlop = true; - - if (m_edgeWeightMax != BB_ZERO_WEIGHT) - { - // We will allow this to raise m_edgeWeightMax towards newWeight - m_edgeWeightMax = newWeight; - } - } - } - else if (m_edgeWeightMin > newWeight) - { - // We have already determined that this edge's weight - // is more than newWeight, so we just allow for the slop - if ((newWeight + slop) >= m_edgeWeightMin) - { - result = true; - usedSlop = true; - - if (m_edgeWeightMax != BB_ZERO_WEIGHT) - { - // We will allow this to lower m_edgeWeightMin and Max towards newWeight - m_edgeWeightMax = m_edgeWeightMin; - m_edgeWeightMin = newWeight; - } - } - } - - // If we are returning true then we should have adjusted the range so that - // the newWeight is in new range [Min..Max] or fgEdgeWeightMax is zero - if (result) - { - assert((m_edgeWeightMax == BB_ZERO_WEIGHT) || - ((newWeight <= m_edgeWeightMax) && (newWeight >= m_edgeWeightMin))); - } - } - - if (result && usedSlop && (wbUsedSlop != nullptr)) - { - *wbUsedSlop = true; - } - -#if DEBUG - if (result) - { - JITDUMP("Updated max weight of " FMT_BB " -> " FMT_BB " to [" FMT_WT ".." FMT_WT "]\n", getSourceBlock()->bbNum, - bDst->bbNum, m_edgeWeightMin, m_edgeWeightMax); - } - else - { - JITDUMP("Not adjusting max weight of " FMT_BB " -> " FMT_BB "; new value " FMT_WT " not in range [" FMT_WT - ".." FMT_WT "] (+/- " FMT_WT ")\n", - getSourceBlock()->bbNum, bDst->bbNum, newWeight, m_edgeWeightMin, m_edgeWeightMax, slop); - result = false; // break here - } -#endif // DEBUG - - return result; -} - //------------------------------------------------------------------------ // setEdgeWeights: Sets the minimum lower (m_edgeWeightMin) value // and the maximum upper (m_edgeWeightMax) value @@ -4530,13 +4307,6 @@ PhaseStatus Compiler::fgComputeBlockAndEdgeWeights() JITDUMP(" -- no profile data, so using default called count\n"); } - PhaseStatus edgeStatus = fgComputeEdgeWeights(); - - if (edgeStatus != PhaseStatus::MODIFIED_NOTHING) - { - return edgeStatus; - } - return madeChanges ? PhaseStatus::MODIFIED_EVERYTHING : PhaseStatus::MODIFIED_NOTHING; } @@ -4776,405 +4546,6 @@ bool Compiler::fgComputeCalledCount(weight_t returnWeight) return madeChanges; } -//------------------------------------------------------------- -// fgComputeEdgeWeights: compute edge weights from block weights -// -// Returns: -// Suitable phase status -// -PhaseStatus Compiler::fgComputeEdgeWeights() -{ - const bool isOptimizing = opts.OptimizationEnabled(); - const bool usingProfileWeights = fgIsUsingProfileWeights(); - - if (!isOptimizing || !usingProfileWeights) - { - JITDUMP(" -- not optimizing or no profile data, so not computing edge weights\n"); - return PhaseStatus::MODIFIED_NOTHING; - } - - BasicBlock* bSrc; - BasicBlock* bDst; - weight_t slop; - unsigned goodEdgeCountCurrent = 0; - unsigned goodEdgeCountPrevious = 0; - bool inconsistentProfileData = false; - bool hasIncompleteEdgeWeights = false; - bool usedSlop = false; - unsigned numEdges = 0; - unsigned iterations = 0; - - JITDUMP("Initial weight assignments\n\n"); - - // Now we will compute the initial m_edgeWeightMin and m_edgeWeightMax values - for (bDst = fgFirstBB; bDst != nullptr; bDst = bDst->Next()) - { - weight_t bDstWeight = bDst->bbWeight; - - // We subtract out the called count so that bDstWeight is - // the sum of all edges that go into this block from this method. - // - if (bDst == fgFirstBB) - { - bDstWeight -= fgCalledCount; - } - - for (FlowEdge* const edge : bDst->PredEdges()) - { - bool assignOK = true; - - bSrc = edge->getSourceBlock(); - // We are processing the control flow edge (bSrc -> bDst) - - numEdges++; - - // - // If the bSrc or bDst blocks do not have exact profile weights - // then we must reset any values that they currently have - // - - if (!bSrc->hasProfileWeight() || !bDst->hasProfileWeight()) - { - edge->setEdgeWeights(BB_ZERO_WEIGHT, BB_MAX_WEIGHT, bDst); - } - - slop = BasicBlock::GetSlopFraction(bSrc, bDst) + 1; - switch (bSrc->GetKind()) - { - case BBJ_ALWAYS: - case BBJ_EHCATCHRET: - case BBJ_CALLFINALLY: - case BBJ_CALLFINALLYRET: - // We know the exact edge weight - assignOK &= edge->setEdgeWeightMinChecked(bSrc->bbWeight, bDst, slop, &usedSlop); - assignOK &= edge->setEdgeWeightMaxChecked(bSrc->bbWeight, bDst, slop, &usedSlop); - break; - - case BBJ_COND: - case BBJ_SWITCH: - case BBJ_EHFINALLYRET: - case BBJ_EHFAULTRET: - case BBJ_EHFILTERRET: - if (edge->edgeWeightMax() > bSrc->bbWeight) - { - // The maximum edge weight to block can't be greater than the weight of bSrc - assignOK &= edge->setEdgeWeightMaxChecked(bSrc->bbWeight, bDst, slop, &usedSlop); - } - break; - - default: - // We should never have an edge that starts from one of these jump kinds - noway_assert(!"Unexpected bbKind"); - break; - } - - // The maximum edge weight to block can't be greater than the weight of bDst - if (edge->edgeWeightMax() > bDstWeight) - { - assignOK &= edge->setEdgeWeightMaxChecked(bDstWeight, bDst, slop, &usedSlop); - } - - if (!assignOK) - { - // Here we have inconsistent profile data - inconsistentProfileData = true; - // No point in continuing - goto EARLY_EXIT; - } - } - } - - fgEdgeCount = numEdges; - - iterations = 0; - - do - { - JITDUMP("\nSolver pass %u\n", iterations); - - iterations++; - goodEdgeCountPrevious = goodEdgeCountCurrent; - goodEdgeCountCurrent = 0; - hasIncompleteEdgeWeights = false; - - JITDUMP("\n -- step 1 --\n"); - for (bDst = fgFirstBB; bDst != nullptr; bDst = bDst->Next()) - { - for (FlowEdge* const edge : bDst->PredEdges()) - { - bool assignOK = true; - - // We are processing the control flow edge (bSrc -> bDst) - bSrc = edge->getSourceBlock(); - - slop = BasicBlock::GetSlopFraction(bSrc, bDst) + 1; - if (bSrc->KindIs(BBJ_COND)) - { - weight_t diff; - FlowEdge* otherEdge; - BasicBlock* otherDst; - if (bSrc->FalseTargetIs(bDst)) - { - otherEdge = bSrc->GetTrueEdge(); - } - else - { - otherEdge = bSrc->GetFalseEdge(); - } - otherDst = otherEdge->getDestinationBlock(); - - // If we see min/max violations, just give up on the computations - // - const bool edgeWeightSensible = edge->edgeWeightMin() <= edge->edgeWeightMax(); - const bool otherEdgeWeightSensible = otherEdge->edgeWeightMin() <= otherEdge->edgeWeightMax(); - - assignOK &= edgeWeightSensible && otherEdgeWeightSensible; - - if (assignOK) - { - // Adjust edge->m_edgeWeightMin up or adjust otherEdge->m_edgeWeightMax down - diff = bSrc->bbWeight - (edge->edgeWeightMin() + otherEdge->edgeWeightMax()); - if (diff > 0) - { - assignOK &= - edge->setEdgeWeightMinChecked(edge->edgeWeightMin() + diff, bDst, slop, &usedSlop); - } - else if (diff < 0) - { - assignOK &= otherEdge->setEdgeWeightMaxChecked(otherEdge->edgeWeightMax() + diff, otherDst, - slop, &usedSlop); - } - - // Adjust otherEdge->m_edgeWeightMin up or adjust edge->m_edgeWeightMax down - diff = bSrc->bbWeight - (otherEdge->edgeWeightMin() + edge->edgeWeightMax()); - if (diff > 0) - { - assignOK &= otherEdge->setEdgeWeightMinChecked(otherEdge->edgeWeightMin() + diff, otherDst, - slop, &usedSlop); - } - else if (diff < 0) - { - assignOK &= - edge->setEdgeWeightMaxChecked(edge->edgeWeightMax() + diff, bDst, slop, &usedSlop); - } - } - - if (!assignOK) - { - // Here we have inconsistent profile data - inconsistentProfileData = true; - // No point in continuing - goto EARLY_EXIT; - } -#ifdef DEBUG - // Now edge->m_edgeWeightMin and otherEdge->m_edgeWeightMax) should add up to bSrc->bbWeight - diff = bSrc->bbWeight - (edge->edgeWeightMin() + otherEdge->edgeWeightMax()); - - if (!((-slop) <= diff) && (diff <= slop)) - { - JITDUMP("Edge weight discrepancy: " FMT_BB "[" FMT_WT "] -> {" FMT_BB "[min:" FMT_WT - "], " FMT_BB "[max: " FMT_WT "]} diff " FMT_WT " exceeds slop " FMT_WT "\n", - bSrc->bbNum, bSrc->bbWeight, bDst->bbNum, edge->edgeWeightMin(), otherDst->bbNum, - otherEdge->edgeWeightMax(), diff, slop); - } - - // Now otherEdge->m_edgeWeightMin and edge->m_edgeWeightMax) should add up to bSrc->bbWeight - diff = bSrc->bbWeight - (otherEdge->edgeWeightMin() + edge->edgeWeightMax()); - if (!((-slop) <= diff) && (diff <= slop)) - { - JITDUMP("Edge weight discrepancy: " FMT_BB "[" FMT_WT "] -> {" FMT_BB "[max:" FMT_WT - "], " FMT_BB "[min: " FMT_WT "]} diff " FMT_WT " exceeds slop " FMT_WT "\n", - bSrc->bbNum, bSrc->bbWeight, bDst->bbNum, edge->edgeWeightMax(), otherDst->bbNum, - otherEdge->edgeWeightMin(), diff, slop); - } -#endif // DEBUG - } - } - } - - JITDUMP("\n -- step 2 --\n"); - - for (bDst = fgFirstBB; bDst != nullptr; bDst = bDst->Next()) - { - weight_t bDstWeight = bDst->bbWeight; - - if (bDstWeight == BB_MAX_WEIGHT) - { - inconsistentProfileData = true; - // No point in continuing - goto EARLY_EXIT; - } - else - { - // We subtract out the called count so that bDstWeight is - // the sum of all edges that go into this block from this method. - // - if (bDst == fgFirstBB) - { - bDstWeight -= fgCalledCount; - } - - weight_t minEdgeWeightSum = 0; - weight_t maxEdgeWeightSum = 0; - - // Calculate the sums of the minimum and maximum edge weights - for (FlowEdge* const edge : bDst->PredEdges()) - { - maxEdgeWeightSum += edge->edgeWeightMax(); - minEdgeWeightSum += edge->edgeWeightMin(); - } - - // maxEdgeWeightSum is the sum of all m_edgeWeightMax values into bDst - // minEdgeWeightSum is the sum of all m_edgeWeightMin values into bDst - - for (FlowEdge* const edge : bDst->PredEdges()) - { - bool assignOK = true; - - // We are processing the control flow edge (bSrc -> bDst) - bSrc = edge->getSourceBlock(); - slop = BasicBlock::GetSlopFraction(bSrc, bDst) + 1; - - // otherMaxEdgesWeightSum is the sum of all of the other edges m_edgeWeightMax values - // This can be used to compute a lower bound for our minimum edge weight - // - weight_t const otherMaxEdgesWeightSum = maxEdgeWeightSum - edge->edgeWeightMax(); - - if (otherMaxEdgesWeightSum >= BB_ZERO_WEIGHT) - { - if (bDstWeight >= otherMaxEdgesWeightSum) - { - // minWeightCalc is our minWeight when every other path to bDst takes it's m_edgeWeightMax - // value - weight_t minWeightCalc = (weight_t)(bDstWeight - otherMaxEdgesWeightSum); - if (minWeightCalc > edge->edgeWeightMin()) - { - assignOK &= edge->setEdgeWeightMinChecked(minWeightCalc, bDst, slop, &usedSlop); - } - } - } - - // otherMinEdgesWeightSum is the sum of all of the other edges m_edgeWeightMin values - // This can be used to compute an upper bound for our maximum edge weight - // - weight_t const otherMinEdgesWeightSum = minEdgeWeightSum - edge->edgeWeightMin(); - - if (otherMinEdgesWeightSum >= BB_ZERO_WEIGHT) - { - if (bDstWeight >= otherMinEdgesWeightSum) - { - // maxWeightCalc is our maxWeight when every other path to bDst takes it's m_edgeWeightMin - // value - weight_t maxWeightCalc = (weight_t)(bDstWeight - otherMinEdgesWeightSum); - if (maxWeightCalc < edge->edgeWeightMax()) - { - assignOK &= edge->setEdgeWeightMaxChecked(maxWeightCalc, bDst, slop, &usedSlop); - } - } - } - - if (!assignOK) - { - // Here we have inconsistent profile data - JITDUMP("Inconsistent profile data at " FMT_BB " -> " FMT_BB ": dest weight " FMT_WT - ", min/max into dest is " FMT_WT "/" FMT_WT ", edge " FMT_WT "/" FMT_WT "\n", - bSrc->bbNum, bDst->bbNum, bDstWeight, minEdgeWeightSum, maxEdgeWeightSum, - edge->edgeWeightMin(), edge->edgeWeightMax()); - - inconsistentProfileData = true; - // No point in continuing - goto EARLY_EXIT; - } - - // When m_edgeWeightMin equals m_edgeWeightMax we have a "good" edge weight - if (edge->edgeWeightMin() == edge->edgeWeightMax()) - { - // Count how many "good" edge weights we have - // Each time through we should have more "good" weights - // We exit the while loop when no longer find any new "good" edges - goodEdgeCountCurrent++; - } - else - { - // Remember that we have seen at least one "Bad" edge weight - // so that we will repeat the while loop again - hasIncompleteEdgeWeights = true; - } - } - } - } - - assert(!inconsistentProfileData); // Should use EARLY_EXIT when it is false. - - if (numEdges == goodEdgeCountCurrent) - { - noway_assert(hasIncompleteEdgeWeights == false); - break; - } - - } while (hasIncompleteEdgeWeights && (goodEdgeCountCurrent > goodEdgeCountPrevious) && (iterations < 8)); - -EARLY_EXIT:; - -#ifdef DEBUG - if (verbose) - { - if (inconsistentProfileData) - { - printf("fgComputeEdgeWeights() found inconsistent profile data, not using the edge weights\n"); - } - else - { - if (hasIncompleteEdgeWeights) - { - printf("fgComputeEdgeWeights() was able to compute exact edge weights for %3d of the %3d edges, using " - "%d passes.\n", - goodEdgeCountCurrent, numEdges, iterations); - } - else - { - printf("fgComputeEdgeWeights() was able to compute exact edge weights for all of the %3d edges, using " - "%d passes.\n", - numEdges, iterations); - } - - fgPrintEdgeWeights(); - } - } -#endif // DEBUG - - fgSlopUsedInEdgeWeights = usedSlop; - fgRangeUsedInEdgeWeights = false; - - // See if any edge weight are expressed in [min..max] form - - for (BasicBlock* const bDst : Blocks()) - { - if (bDst->bbPreds != nullptr) - { - for (FlowEdge* const edge : bDst->PredEdges()) - { - // This is the control flow edge (edge->getBlock() -> bDst) - - if (edge->edgeWeightMin() != edge->edgeWeightMax()) - { - fgRangeUsedInEdgeWeights = true; - break; - } - } - if (fgRangeUsedInEdgeWeights) - { - break; - } - } - } - - fgHaveValidEdgeWeights = !inconsistentProfileData; - fgEdgeWeightsComputed = true; - - return PhaseStatus::MODIFIED_EVERYTHING; -} - //------------------------------------------------------------------------ // fgProfileWeightsEqual: check if two profile weights are equal // (or nearly so) From 9488f854bc448f6fa0d71092aa26af9879cace73 Mon Sep 17 00:00:00 2001 From: "Aman Khalid (from Dev Box)" Date: Wed, 13 Mar 2024 14:33:12 -0400 Subject: [PATCH 02/12] Remove setEdgeWeights --- src/coreclr/jit/block.h | 2 - src/coreclr/jit/compiler.h | 1 - src/coreclr/jit/fgflow.cpp | 36 ------- src/coreclr/jit/fgopt.cpp | 128 ++++------------------ src/coreclr/jit/fgprofile.cpp | 23 ---- src/coreclr/jit/morph.cpp | 84 +++------------ src/coreclr/jit/optimizebools.cpp | 8 +- src/coreclr/jit/optimizer.cpp | 169 ++---------------------------- 8 files changed, 45 insertions(+), 406 deletions(-) diff --git a/src/coreclr/jit/block.h b/src/coreclr/jit/block.h index 3489a18263b51..a3db8c2058016 100644 --- a/src/coreclr/jit/block.h +++ b/src/coreclr/jit/block.h @@ -658,8 +658,6 @@ struct FlowEdge return m_edgeWeightMax; } - void setEdgeWeights(weight_t newMinWeight, weight_t newMaxWeight, BasicBlock* bDst); - weight_t getLikelihood() const { return m_likelihood; diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index 1616137ddff3e..58cc205db502d 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -6825,7 +6825,6 @@ class Compiler BasicBlock* optTryAdvanceLoopCompactionInsertionPoint(FlowGraphNaturalLoop* loop, BasicBlock* insertionPoint, BasicBlock* top, BasicBlock* bottom); bool optCreatePreheader(FlowGraphNaturalLoop* loop); void optSetWeightForPreheaderOrExit(FlowGraphNaturalLoop* loop, BasicBlock* block); - weight_t optEstimateEdgeLikelihood(BasicBlock* from, BasicBlock* to, bool* fromProfile); bool optCanonicalizeExits(FlowGraphNaturalLoop* loop); bool optCanonicalizeExit(FlowGraphNaturalLoop* loop, BasicBlock* exit); diff --git a/src/coreclr/jit/fgflow.cpp b/src/coreclr/jit/fgflow.cpp index f9a1721218683..30066b4941684 100644 --- a/src/coreclr/jit/fgflow.cpp +++ b/src/coreclr/jit/fgflow.cpp @@ -191,42 +191,6 @@ FlowEdge* Compiler::fgAddRefPred(BasicBlock* block, BasicBlock* blockPred, FlowE // flow->setLikelihood(oldEdge->getLikelihood()); } - - if (fgHaveValidEdgeWeights) - { - // We are creating an edge from blockPred to block - // and we have already computed the edge weights, so - // we will try to setup this new edge with valid edge weights. - // - if (oldEdge != nullptr) - { - // If our caller has given us the old edge weights - // then we will use them. - // - flow->setEdgeWeights(oldEdge->edgeWeightMin(), oldEdge->edgeWeightMax(), block); - } - else - { - // Set the max edge weight to be the minimum of block's or blockPred's weight - // - weight_t newWeightMax = min(block->bbWeight, blockPred->bbWeight); - - // If we are inserting a conditional block the minimum weight is zero, - // otherwise it is the same as the edge's max weight. - if (blockPred->NumSucc() > 1) - { - flow->setEdgeWeights(BB_ZERO_WEIGHT, newWeightMax, block); - } - else - { - flow->setEdgeWeights(flow->edgeWeightMax(), newWeightMax, block); - } - } - } - else - { - flow->setEdgeWeights(BB_ZERO_WEIGHT, BB_MAX_WEIGHT, block); - } } // Pred list should (still) be ordered. diff --git a/src/coreclr/jit/fgopt.cpp b/src/coreclr/jit/fgopt.cpp index 3052d2fca63ea..1e35c02a9c5e3 100644 --- a/src/coreclr/jit/fgopt.cpp +++ b/src/coreclr/jit/fgopt.cpp @@ -1464,75 +1464,27 @@ bool Compiler::fgOptimizeBranchToEmptyUnconditional(BasicBlock* block, BasicBloc // When we optimize a branch to branch we need to update the profile weight // of bDest by subtracting out the block/edge weight of the path that is being optimized. // - if (fgHaveValidEdgeWeights && bDest->hasProfileWeight()) - { - FlowEdge* edge1 = fgGetPredForBlock(bDest, block); - noway_assert(edge1 != nullptr); - - weight_t edgeWeight; - - if (edge1->edgeWeightMin() != edge1->edgeWeightMax()) - { - // - // We only have an estimate for the edge weight - // - edgeWeight = (edge1->edgeWeightMin() + edge1->edgeWeightMax()) / 2; - // - // Clear the profile weight flag - // - bDest->RemoveFlags(BBF_PROF_WEIGHT); - } - else - { - // - // We only have the exact edge weight - // - edgeWeight = edge1->edgeWeightMin(); - } - - // - // Update the bDest->bbWeight - // - if (bDest->bbWeight > edgeWeight) - { - bDest->bbWeight -= edgeWeight; - } - else - { - bDest->bbWeight = BB_ZERO_WEIGHT; - bDest->SetFlags(BBF_RUN_RARELY); // Set the RarelyRun flag - } - - FlowEdge* edge2 = bDest->GetTargetEdge(); - - if (edge2 != nullptr) - { - // - // Update the edge2 min/max weights - // - weight_t newEdge2Min; - weight_t newEdge2Max; - - if (edge2->edgeWeightMin() > edge1->edgeWeightMin()) - { - newEdge2Min = edge2->edgeWeightMin() - edge1->edgeWeightMin(); - } - else - { - newEdge2Min = BB_ZERO_WEIGHT; - } - - if (edge2->edgeWeightMax() > edge1->edgeWeightMin()) - { - newEdge2Max = edge2->edgeWeightMax() - edge1->edgeWeightMin(); - } - else - { - newEdge2Max = BB_ZERO_WEIGHT; - } - edge2->setEdgeWeights(newEdge2Min, newEdge2Max, bDest); - } - } + // TODO: Update block weight using likely weight + // if (bDest->hasProfileWeight()) + // { + // FlowEdge* const edge = fgGetPredForBlock(bDest, block); + // noway_assert(edge != nullptr); + + // const weight_t edgeWeight = edge->getLikelyWeight(); + + // // + // // Update the bDest->bbWeight + // // + // if (bDest->bbWeight > edgeWeight) + // { + // bDest->bbWeight -= edgeWeight; + // } + // else + // { + // bDest->bbWeight = BB_ZERO_WEIGHT; + // bDest->SetFlags(BBF_RUN_RARELY); // Set the RarelyRun flag + // } + // } // Optimize the JUMP to empty unconditional JUMP to go to the new target switch (block->GetKind()) @@ -3070,47 +3022,9 @@ bool Compiler::fgOptimizeSwitchJumps() newBlock->setBBProfileWeight(blockToNewBlockWeight); - blockToTargetEdge->setEdgeWeights(blockToTargetWeight, blockToTargetWeight, dominantTarget); blockToTargetEdge->setLikelihood(fraction); - blockToNewBlockEdge->setEdgeWeights(blockToNewBlockWeight, blockToNewBlockWeight, block); blockToNewBlockEdge->setLikelihood(max(0, 1.0 - fraction)); - // There may be other switch cases that lead to this same block, but there's just - // one edge in the flowgraph. So we need to subtract off the profile data that now flows - // along the peeled edge. - // - for (FlowEdge* pred = dominantTarget->bbPreds; pred != nullptr; pred = pred->getNextPredEdge()) - { - if (pred->getSourceBlock() == newBlock) - { - if (pred->getDupCount() == 1) - { - // The only switch case leading to the dominant target was the one we peeled. - // So the edge from the switch now has zero weight. - // - pred->setEdgeWeights(BB_ZERO_WEIGHT, BB_ZERO_WEIGHT, dominantTarget); - } - else - { - // Other switch cases also lead to the dominant target. - // Subtract off the weight we transferred to the peel. - // - weight_t newMinWeight = pred->edgeWeightMin() - blockToTargetWeight; - weight_t newMaxWeight = pred->edgeWeightMax() - blockToTargetWeight; - - if (newMinWeight < BB_ZERO_WEIGHT) - { - newMinWeight = BB_ZERO_WEIGHT; - } - if (newMaxWeight < BB_ZERO_WEIGHT) - { - newMaxWeight = BB_ZERO_WEIGHT; - } - pred->setEdgeWeights(newMinWeight, newMaxWeight, dominantTarget); - } - } - } - // For now we leave the switch as is, since there's no way // to indicate that one of the cases is now unreachable. // diff --git a/src/coreclr/jit/fgprofile.cpp b/src/coreclr/jit/fgprofile.cpp index fe82405ed7116..4a68e1a76b1c9 100644 --- a/src/coreclr/jit/fgprofile.cpp +++ b/src/coreclr/jit/fgprofile.cpp @@ -4248,29 +4248,6 @@ bool Compiler::fgIncorporateEdgeCounts() return e.IsGood(); } -//------------------------------------------------------------------------ -// setEdgeWeights: Sets the minimum lower (m_edgeWeightMin) value -// and the maximum upper (m_edgeWeightMax) value -// Asserts that the max value is greater or equal to the min value -// -// Arguments: -// theMinWeight - the new minimum lower (m_edgeWeightMin) -// theMaxWeight - the new maximum upper (m_edgeWeightMin) -// bDst - the destination block for the edge -// -void FlowEdge::setEdgeWeights(weight_t theMinWeight, weight_t theMaxWeight, BasicBlock* bDst) -{ - assert(theMinWeight <= theMaxWeight); - assert(theMinWeight >= 0.0); - assert(theMaxWeight >= 0.0); - - JITDUMP("Setting edge weights for " FMT_BB " -> " FMT_BB " to [" FMT_WT " .. " FMT_WT "]\n", - getSourceBlock()->bbNum, bDst->bbNum, theMinWeight, theMaxWeight); - - m_edgeWeightMin = theMinWeight; - m_edgeWeightMax = theMaxWeight; -} - //------------------------------------------------------------- // fgComputeBlockAndEdgeWeights: determine weights for blocks // and optionally for edges diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp index 92dece7096091..7fc174729c958 100644 --- a/src/coreclr/jit/morph.cpp +++ b/src/coreclr/jit/morph.cpp @@ -13226,80 +13226,28 @@ Compiler::FoldResult Compiler::fgFoldConditional(BasicBlock* block) block->SetFlags(BBF_NONE_QUIRK); } - if (fgHaveValidEdgeWeights) + // We examine the taken edge (block -> bTaken) + // if block has valid profile weight and bTaken does not we try to adjust bTaken's weight + // else if bTaken has valid profile weight and block does not we try to adjust block's weight + // We can only adjust the block weights when (the edge block -> bTaken) is the only edge into bTaken + // + if (block->hasProfileWeight()) { - // We are removing an edge from block to bNotTaken - // and we have already computed the edge weights, so - // we will try to adjust some of the weights - // - BasicBlock* bUpdated = nullptr; // non-NULL if we updated the weight of an internal block - - // We examine the taken edge (block -> bTaken) - // if block has valid profile weight and bTaken does not we try to adjust bTaken's weight - // else if bTaken has valid profile weight and block does not we try to adjust block's weight - // We can only adjust the block weights when (the edge block -> bTaken) is the only edge into bTaken - // - if (block->hasProfileWeight()) - { - // The edge weights for (block -> bTaken) are 100% of block's weight - - edgeTaken->setEdgeWeights(block->bbWeight, block->bbWeight, bTaken); - - if (!bTaken->hasProfileWeight()) - { - if ((bTaken->countOfInEdges() == 1) || (bTaken->bbWeight < block->bbWeight)) - { - // Update the weight of bTaken - bTaken->inheritWeight(block); - bUpdated = bTaken; - } - } - } - else if (bTaken->hasProfileWeight()) + if (!bTaken->hasProfileWeight()) { - if (bTaken->countOfInEdges() == 1) + if ((bTaken->countOfInEdges() == 1) || (bTaken->bbWeight < block->bbWeight)) { - // There is only one in edge to bTaken - edgeTaken->setEdgeWeights(bTaken->bbWeight, bTaken->bbWeight, bTaken); - - // Update the weight of block - block->inheritWeight(bTaken); - bUpdated = block; + // Update the weight of bTaken + bTaken->inheritWeight(block); } } - - if (bUpdated != nullptr) + } + else if (bTaken->hasProfileWeight()) + { + if (bTaken->countOfInEdges() == 1) { - weight_t newMinWeight; - weight_t newMaxWeight; - - FlowEdge* edge; - // Now fix the weights of the edges out of 'bUpdated' - switch (bUpdated->GetKind()) - { - case BBJ_COND: - edge = bUpdated->GetFalseEdge(); - newMaxWeight = bUpdated->bbWeight; - newMinWeight = min(edge->edgeWeightMin(), newMaxWeight); - edge->setEdgeWeights(newMinWeight, newMaxWeight, bUpdated->GetFalseTarget()); - - edge = bUpdated->GetTrueEdge(); - newMaxWeight = bUpdated->bbWeight; - newMinWeight = min(edge->edgeWeightMin(), newMaxWeight); - edge->setEdgeWeights(newMinWeight, newMaxWeight, bUpdated->GetFalseTarget()); - break; - - case BBJ_ALWAYS: - edge = bUpdated->GetTargetEdge(); - newMaxWeight = bUpdated->bbWeight; - newMinWeight = min(edge->edgeWeightMin(), newMaxWeight); - edge->setEdgeWeights(newMinWeight, newMaxWeight, bUpdated->Next()); - break; - - default: - // We don't handle BBJ_SWITCH - break; - } + // Update the weight of block + block->inheritWeight(bTaken); } } diff --git a/src/coreclr/jit/optimizebools.cpp b/src/coreclr/jit/optimizebools.cpp index 4363e946af51d..5308307762479 100644 --- a/src/coreclr/jit/optimizebools.cpp +++ b/src/coreclr/jit/optimizebools.cpp @@ -1322,11 +1322,9 @@ void OptBoolsDsc::optOptimizeBoolsUpdateTrees() (1.0 - origB1TrueLikelihood) + origB1TrueLikelihood * origB2FalseEdge->getLikelihood(); } - // Fix B1 true edge likelihood and min/max weights + // Fix B1 true edge likelihood // origB1TrueEdge->setLikelihood(newB1TrueLikelihood); - weight_t const newB1TrueWeight = m_b1->bbWeight * newB1TrueLikelihood; - origB1TrueEdge->setEdgeWeights(newB1TrueWeight, newB1TrueWeight, m_b1->GetTrueTarget()); assert(m_b1->KindIs(BBJ_COND)); assert(m_b2->KindIs(BBJ_COND)); @@ -1341,11 +1339,9 @@ void OptBoolsDsc::optOptimizeBoolsUpdateTrees() FlowEdge* const newB1FalseEdge = origB2FalseEdge; m_b1->SetFalseEdge(newB1FalseEdge); - // Fix B1 false edge likelihood and min/max weights. + // Fix B1 false edge likelihood // newB1FalseEdge->setLikelihood(1.0 - newB1TrueLikelihood); - weight_t const newB1FalseWeight = m_b1->bbWeight * (1.0 - newB1TrueLikelihood); - newB1FalseEdge->setEdgeWeights(newB1FalseWeight, newB1FalseWeight, m_b1->GetTrueTarget()); } // Get rid of the second block diff --git a/src/coreclr/jit/optimizer.cpp b/src/coreclr/jit/optimizer.cpp index e837dbfe72d21..0b4335af748b2 100644 --- a/src/coreclr/jit/optimizer.cpp +++ b/src/coreclr/jit/optimizer.cpp @@ -2312,54 +2312,6 @@ bool Compiler::optInvertWhileLoop(BasicBlock* block) weightTop); bTest->inheritWeight(bTop); - // Determine the new edge weights. - // - // We project the next/jump ratio for block and bTest by using - // the original likelihoods out of bTest. - // - // Note "next" is the loop top block, not bTest's bbNext, - // we'll call this latter block "after". - // - weight_t const testToNextLikelihood = min(1.0, weightTop / weightTest); - weight_t const testToAfterLikelihood = 1.0 - testToNextLikelihood; - - // Adjust edges out of bTest (which now has weight weightTop) - // - weight_t const testToNextWeight = weightTop * testToNextLikelihood; - weight_t const testToAfterWeight = weightTop * testToAfterLikelihood; - - FlowEdge* const edgeTestToNext = bTest->GetTrueEdge(); - FlowEdge* const edgeTestToAfter = bTest->GetFalseEdge(); - - JITDUMP("Setting weight of " FMT_BB " -> " FMT_BB " to " FMT_WT " (iterate loop)\n", bTest->bbNum, bTop->bbNum, - testToNextWeight); - JITDUMP("Setting weight of " FMT_BB " -> " FMT_BB " to " FMT_WT " (exit loop)\n", bTest->bbNum, - bTest->Next()->bbNum, testToAfterWeight); - - edgeTestToNext->setEdgeWeights(testToNextWeight, testToNextWeight, bTop); - edgeTestToAfter->setEdgeWeights(testToAfterWeight, testToAfterWeight, bTest->GetFalseTarget()); - - // Adjust edges out of block, using the same distribution. - // - JITDUMP("Profile weight of " FMT_BB " remains unchanged at " FMT_WT "\n", block->bbNum, weightBlock); - - weight_t const blockToNextLikelihood = testToNextLikelihood; - weight_t const blockToAfterLikelihood = testToAfterLikelihood; - - weight_t const blockToNextWeight = weightBlock * blockToNextLikelihood; - weight_t const blockToAfterWeight = weightBlock * blockToAfterLikelihood; - - FlowEdge* const edgeBlockToNext = bNewCond->GetFalseEdge(); - FlowEdge* const edgeBlockToAfter = bNewCond->GetTrueEdge(); - - JITDUMP("Setting weight of " FMT_BB " -> " FMT_BB " to " FMT_WT " (enter loop)\n", bNewCond->bbNum, - bNewCond->GetFalseTarget()->bbNum, blockToNextWeight); - JITDUMP("Setting weight of " FMT_BB " -> " FMT_BB " to " FMT_WT " (avoid loop)\n", bNewCond->bbNum, - bNewCond->GetTrueTarget()->bbNum, blockToAfterWeight); - - edgeBlockToNext->setEdgeWeights(blockToNextWeight, blockToNextWeight, bNewCond->GetFalseTarget()); - edgeBlockToAfter->setEdgeWeights(blockToAfterWeight, blockToAfterWeight, bNewCond->GetTrueTarget()); - #ifdef DEBUG // If we're checking profile data, see if profile for the two target blocks is consistent. // @@ -3169,91 +3121,6 @@ bool Compiler::optCanonicalizeExit(FlowGraphNaturalLoop* loop, BasicBlock* exit) return true; } -//----------------------------------------------------------------------------- -// optEstimateEdgeLikelihood: Given a block "from" that may transfer control to -// "to", estimate the likelihood that this will happen taking profile into -// account if available. -// -// Parameters: -// from - From block -// to - To block -// fromProfile - [out] Whether or not the estimate is based on profile data -// -// Returns: -// Estimated likelihood of the edge being taken. -// -weight_t Compiler::optEstimateEdgeLikelihood(BasicBlock* from, BasicBlock* to, bool* fromProfile) -{ - *fromProfile = (from->HasFlag(BBF_PROF_WEIGHT) != BBF_EMPTY) && (to->HasFlag(BBF_PROF_WEIGHT) != BBF_EMPTY); - if (!fgIsUsingProfileWeights() || !from->HasFlag(BBF_PROF_WEIGHT) || !to->HasFlag(BBF_PROF_WEIGHT) || - from->KindIs(BBJ_ALWAYS)) - { - return 1.0 / from->NumSucc(this); - } - - bool useEdgeWeights = fgHaveValidEdgeWeights; - - weight_t takenCount = 0; - weight_t notTakenCount = 0; - - if (useEdgeWeights) - { - from->VisitRegularSuccs(this, [&, to](BasicBlock* succ) { - *fromProfile &= succ->hasProfileWeight(); - FlowEdge* edge = fgGetPredForBlock(succ, from); - weight_t edgeWeight = (edge->edgeWeightMin() + edge->edgeWeightMax()) / 2.0; - - if (succ == to) - { - takenCount += edgeWeight; - } - else - { - notTakenCount += edgeWeight; - } - return BasicBlockVisit::Continue; - }); - - // Watch out for cases where edge weights were not properly maintained - // so that it appears no profile flow goes to 'to'. - // - useEdgeWeights = !fgProfileWeightsConsistent(takenCount, BB_ZERO_WEIGHT); - } - - if (!useEdgeWeights) - { - takenCount = 0; - notTakenCount = 0; - - from->VisitRegularSuccs(this, [&, to](BasicBlock* succ) { - *fromProfile &= succ->hasProfileWeight(); - if (succ == to) - { - takenCount += succ->bbWeight; - } - else - { - notTakenCount += succ->bbWeight; - } - - return BasicBlockVisit::Continue; - }); - } - - if (!*fromProfile) - { - return 1.0 / from->NumSucc(this); - } - - if (fgProfileWeightsConsistent(takenCount, BB_ZERO_WEIGHT)) - { - return 0; - } - - weight_t likelihood = takenCount / (takenCount + notTakenCount); - return likelihood; -} - //----------------------------------------------------------------------------- // optSetWeightForPreheaderOrExit: Set the weight of a newly created preheader // or exit, after it has been added to the flowgraph. @@ -3264,37 +3131,18 @@ weight_t Compiler::optEstimateEdgeLikelihood(BasicBlock* from, BasicBlock* to, b // void Compiler::optSetWeightForPreheaderOrExit(FlowGraphNaturalLoop* loop, BasicBlock* block) { + assert(!block->HasAnyFlag(BBF_RUN_RARELY | BBF_PROF_WEIGHT)); bool hasProfWeight = true; - - assert(block->GetUniqueSucc() != nullptr); - // Inherit first estimate from the target target; optEstimateEdgeLikelihood - // may use it in its estimate if we do not have edge weights to estimate - // from (we also assume the edges into 'block' already inherited their edge - // weights from the previous edge). - block->inheritWeight(block->GetTarget()); - weight_t newWeight = BB_ZERO_WEIGHT; - for (FlowEdge* edge : block->PredEdges()) - { - BasicBlock* predBlock = edge->getSourceBlock(); - - bool fromProfile = false; - weight_t likelihood = optEstimateEdgeLikelihood(predBlock, block, &fromProfile); - hasProfWeight &= fromProfile; - - weight_t contribution = predBlock->bbWeight * likelihood; - JITDUMP(" Estimated likelihood " FMT_BB " -> " FMT_BB " to be " FMT_WT " (contribution: " FMT_WT ")\n", - predBlock->bbNum, block->bbNum, likelihood, contribution); - newWeight += contribution; - - // Normalize pred -> new block weight - edge->setEdgeWeights(contribution, contribution, block); + for (FlowEdge* const edge : block->PredEdges()) + { + newWeight += edge->getLikelyWeight(); + hasProfWeight &= edge->getSourceBlock()->hasProfileWeight(); } - block->RemoveFlags(BBF_PROF_WEIGHT | BBF_RUN_RARELY); - block->bbWeight = newWeight; + if (hasProfWeight) { block->SetFlags(BBF_PROF_WEIGHT); @@ -3305,11 +3153,6 @@ void Compiler::optSetWeightForPreheaderOrExit(FlowGraphNaturalLoop* loop, BasicB block->SetFlags(BBF_RUN_RARELY); return; } - - // Normalize block -> target weight - FlowEdge* const edgeFromBlock = block->GetTargetEdge(); - assert(edgeFromBlock != nullptr); - edgeFromBlock->setEdgeWeights(block->bbWeight, block->bbWeight, block->GetTarget()); } /***************************************************************************** From 714bcc344f1716f24876eeed067049e8bfaa2b35 Mon Sep 17 00:00:00 2001 From: "Aman Khalid (from Dev Box)" Date: Wed, 13 Mar 2024 15:59:35 -0400 Subject: [PATCH 03/12] Remove edge weight members --- src/coreclr/jit/block.h | 16 ------ src/coreclr/jit/compiler.h | 2 +- src/coreclr/jit/fgbasic.cpp | 21 ++------ src/coreclr/jit/fgdiagnostic.cpp | 71 ++++++++------------------- src/coreclr/jit/fgopt.cpp | 83 ++++++++++---------------------- src/coreclr/jit/fgprofile.cpp | 70 ++------------------------- src/coreclr/jit/optimizer.cpp | 12 +++-- 7 files changed, 64 insertions(+), 211 deletions(-) diff --git a/src/coreclr/jit/block.h b/src/coreclr/jit/block.h index a3db8c2058016..e4e154de0b8b4 100644 --- a/src/coreclr/jit/block.h +++ b/src/coreclr/jit/block.h @@ -582,10 +582,6 @@ struct FlowEdge // The destination of the control flow BasicBlock* m_destBlock; - // Edge weights - weight_t m_edgeWeightMin; - weight_t m_edgeWeightMax; - // Likelihood that m_sourceBlock transfers control along this edge. // Values in range [0..1] weight_t m_likelihood; @@ -601,8 +597,6 @@ struct FlowEdge : m_nextPredEdge(rest) , m_sourceBlock(sourceBlock) , m_destBlock(destBlock) - , m_edgeWeightMin(0) - , m_edgeWeightMax(0) , m_likelihood(0) , m_dupCount(0) , m_likelihoodSet(false) @@ -648,16 +642,6 @@ struct FlowEdge m_destBlock = newBlock; } - weight_t edgeWeightMin() const - { - return m_edgeWeightMin; - } - - weight_t edgeWeightMax() const - { - return m_edgeWeightMax; - } - weight_t getLikelihood() const { return m_likelihood; diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index 58cc205db502d..9f17beecddf87 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -1582,7 +1582,7 @@ enum API_ICorJitInfo_Names enum class ProfileChecks : unsigned int { CHECK_NONE = 0, - CHECK_CLASSIC = 1 << 0, // check "classic" jit weights + CHECK_CLASSIC = 1 << 0, // check "classic" jit weights TODO REMOVE CHECK_HASLIKELIHOOD = 1 << 1, // check all FlowEdges for hasLikelihood CHECK_LIKELIHOODSUM = 1 << 2, // check block succesor likelihoods sum to 1 CHECK_LIKELY = 1 << 3, // fully check likelihood based weights diff --git a/src/coreclr/jit/fgbasic.cpp b/src/coreclr/jit/fgbasic.cpp index 200c72a4d4e1c..9f02bc1968402 100644 --- a/src/coreclr/jit/fgbasic.cpp +++ b/src/coreclr/jit/fgbasic.cpp @@ -5395,6 +5395,7 @@ BasicBlock* Compiler::fgConnectFallThrough(BasicBlock* bSrc, BasicBlock* bDst) // Add a new block after bSrc which jumps to 'bDst' jmpBlk = fgNewBBafter(BBJ_ALWAYS, bSrc, true); FlowEdge* const oldEdge = bSrc->GetFalseEdge(); + // Access the likelihood of oldEdge before // it gets reset by SetTargetEdge below. // @@ -5406,29 +5407,15 @@ BasicBlock* Compiler::fgConnectFallThrough(BasicBlock* bSrc, BasicBlock* bDst) // When adding a new jmpBlk we will set the bbWeight and bbFlags // - if (fgHaveValidEdgeWeights && fgHaveProfileWeights()) + if (fgHaveProfileWeights()) { - jmpBlk->bbWeight = (newEdge->edgeWeightMin() + newEdge->edgeWeightMax()) / 2; - if (bSrc->bbWeight == BB_ZERO_WEIGHT) - { - jmpBlk->bbWeight = BB_ZERO_WEIGHT; - } + jmpBlk->bbWeight = newEdge->getLikelyWeight(); + jmpBlk->SetFlags(BBF_PROF_WEIGHT); if (jmpBlk->bbWeight == BB_ZERO_WEIGHT) { jmpBlk->SetFlags(BBF_RUN_RARELY); } - - weight_t weightDiff = (newEdge->edgeWeightMax() - newEdge->edgeWeightMin()); - weight_t slop = BasicBlock::GetSlopFraction(bSrc, bDst); - // - // If the [min/max] values for our edge weight is within the slop factor - // then we will set the BBF_PROF_WEIGHT flag for the block - // - if (weightDiff <= slop) - { - jmpBlk->SetFlags(BBF_PROF_WEIGHT); - } } else { diff --git a/src/coreclr/jit/fgdiagnostic.cpp b/src/coreclr/jit/fgdiagnostic.cpp index c78fe6f7124e9..6de79410e3e3b 100644 --- a/src/coreclr/jit/fgdiagnostic.cpp +++ b/src/coreclr/jit/fgdiagnostic.cpp @@ -28,26 +28,17 @@ void Compiler::fgPrintEdgeWeights() printf(FMT_BB " ", bSrc->bbNum); - if (edge->edgeWeightMin() < BB_MAX_WEIGHT) + const weight_t weight = edge->getLikelyWeight(); + + if (weight < BB_MAX_WEIGHT) { - printf("(%f", edge->edgeWeightMin()); + printf("(%f)", weight); } else { - printf("(MAX"); + printf("(MAX)"); } - if (edge->edgeWeightMin() != edge->edgeWeightMax()) - { - if (edge->edgeWeightMax() < BB_MAX_WEIGHT) - { - printf("..%f", edge->edgeWeightMax()); - } - else - { - printf("..MAX"); - } - } - printf(")"); + if (edge->getNextPredEdge() != nullptr) { printf(", "); @@ -735,7 +726,6 @@ bool Compiler::fgDumpFlowGraph(Phases phase, PhasePosition pos) JITDUMP("Writing out flow graph %s phase %s\n", (pos == PhasePosition::PrePhase) ? "before" : "after", PhaseNames[phase]); - bool validWeights = fgHaveValidEdgeWeights; double weightDivisor = (double)BasicBlock::getCalledCount(this); const char* escapedString; const char* regionString = "NONE"; @@ -791,14 +781,6 @@ bool Compiler::fgDumpFlowGraph(Phases phase, PhasePosition pos) { fprintf(fgxFile, "\n hasLoops=\"true\""); } - if (validWeights) - { - fprintf(fgxFile, "\n validEdgeWeights=\"true\""); - if (!fgSlopUsedInEdgeWeights && !fgRangeUsedInEdgeWeights) - { - fprintf(fgxFile, "\n exactEdgeWeights=\"true\""); - } - } if (fgFirstColdBlock != nullptr) { fprintf(fgxFile, "\n firstColdBlock=\"%d\"", fgFirstColdBlock->bbNum); @@ -1081,11 +1063,8 @@ bool Compiler::fgDumpFlowGraph(Phases phase, PhasePosition pos) fprintf(fgxFile, " ["); } - if (validWeights) - { - weight_t edgeWeight = (edge->edgeWeightMin() + edge->edgeWeightMax()) / 2; - fprintf(fgxFile, "%slabel=\"%7.2f\"", sep, (double)edgeWeight / weightDivisor); - } + const weight_t edgeWeight = edge->getLikelyWeight(); + fprintf(fgxFile, "%slabel=\"%7.2f\"", sep, (double)edgeWeight / weightDivisor); fprintf(fgxFile, "];\n"); } @@ -1106,32 +1085,22 @@ bool Compiler::fgDumpFlowGraph(Phases phase, PhasePosition pos) fprintf(fgxFile, "\n switchDefault=\"true\""); } } - if (validWeights) - { - weight_t edgeWeight = (edge->edgeWeightMin() + edge->edgeWeightMax()) / 2; - fprintf(fgxFile, "\n weight="); - fprintfDouble(fgxFile, ((double)edgeWeight) / weightDivisor); + + const weight_t edgeWeight = edge->getLikelyWeight(); + fprintf(fgxFile, "\n weight="); + fprintfDouble(fgxFile, ((double)edgeWeight) / weightDivisor); - if (edge->edgeWeightMin() != edge->edgeWeightMax()) + if (edgeWeight > 0) + { + if (edgeWeight < bSource->bbWeight) { - fprintf(fgxFile, "\n minWeight="); - fprintfDouble(fgxFile, ((double)edge->edgeWeightMin()) / weightDivisor); - fprintf(fgxFile, "\n maxWeight="); - fprintfDouble(fgxFile, ((double)edge->edgeWeightMax()) / weightDivisor); + fprintf(fgxFile, "\n out="); + fprintfDouble(fgxFile, ((double)edgeWeight) / sourceWeightDivisor); } - - if (edgeWeight > 0) + if (edgeWeight < bTarget->bbWeight) { - if (edgeWeight < bSource->bbWeight) - { - fprintf(fgxFile, "\n out="); - fprintfDouble(fgxFile, ((double)edgeWeight) / sourceWeightDivisor); - } - if (edgeWeight < bTarget->bbWeight) - { - fprintf(fgxFile, "\n in="); - fprintfDouble(fgxFile, ((double)edgeWeight) / targetWeightDivisor); - } + fprintf(fgxFile, "\n in="); + fprintfDouble(fgxFile, ((double)edgeWeight) / targetWeightDivisor); } } } diff --git a/src/coreclr/jit/fgopt.cpp b/src/coreclr/jit/fgopt.cpp index 1e35c02a9c5e3..1ac51a29de1dc 100644 --- a/src/coreclr/jit/fgopt.cpp +++ b/src/coreclr/jit/fgopt.cpp @@ -1780,29 +1780,24 @@ bool Compiler::fgOptimizeSwitchBranches(BasicBlock* block) { // // When we optimize a branch to branch we need to update the profile weight - // of bDest by subtracting out the block/edge weight of the path that is being optimized. + // of bDest by subtracting out the block weight of the path that is being optimized. // + FlowEdge* const oldEdge = *jmpTab; + if (fgIsUsingProfileWeights() && bDest->hasProfileWeight()) { - if (fgHaveValidEdgeWeights) + weight_t const branchThroughWeight = oldEdge->getLikelyWeight(); + if (bDest->bbWeight > branchThroughWeight) { - FlowEdge* edge = *jmpTab; - weight_t branchThroughWeight = edge->edgeWeightMin(); - - if (bDest->bbWeight > branchThroughWeight) - { - bDest->bbWeight -= branchThroughWeight; - } - else - { - bDest->bbWeight = BB_ZERO_WEIGHT; - bDest->SetFlags(BBF_RUN_RARELY); - } + bDest->bbWeight -= branchThroughWeight; + } + else + { + bDest->bbSetRunRarely(); } } // Update the switch jump table - FlowEdge* const oldEdge = *jmpTab; fgRemoveRefPred(oldEdge); FlowEdge* const newEdge = fgAddRefPred(bNewDest, block, oldEdge); *jmpTab = newEdge; @@ -3575,43 +3570,19 @@ bool Compiler::fgReorderBlocks(bool useProfile) // bool moveDestUp = true; - if (fgHaveValidEdgeWeights) - { - // - // The edge bPrev -> bDest must have a higher minimum weight - // than every other edge into bDest - // - FlowEdge* edgeFromPrev = bPrev->GetTargetEdge(); - noway_assert(edgeFromPrev != nullptr); + // + // The edge bPrev -> bDest must have a higher weight + // than every other edge into bDest + // + weight_t const weightToBeat = bPrev->GetTargetEdge()->getLikelyWeight(); - // Examine all of the other edges into bDest - for (FlowEdge* const edge : bDest->PredEdges()) - { - if (edge != edgeFromPrev) - { - if (edge->edgeWeightMax() >= edgeFromPrev->edgeWeightMin()) - { - moveDestUp = false; - break; - } - } - } - } - else + // Examine all of the other edges into bDest + for (FlowEdge* const edge : bDest->PredEdges()) { - // - // The block bPrev must have a higher weight - // than every other block that goes into bDest - // - - // Examine all of the other edges into bDest - for (BasicBlock* const predBlock : bDest->PredBlocks()) + if (edge->getLikelyWeight() > weightToBeat) { - if ((predBlock != bPrev) && (predBlock->bbWeight >= bPrev->bbWeight)) - { - moveDestUp = false; - break; - } + moveDestUp = false; + break; } } @@ -3680,14 +3651,12 @@ bool Compiler::fgReorderBlocks(bool useProfile) noway_assert(edgeToBlock != nullptr); // // Calculate the taken ratio - // A takenRation of 0.10 means taken 10% of the time, not taken 90% of the time - // A takenRation of 0.50 means taken 50% of the time, not taken 50% of the time - // A takenRation of 0.90 means taken 90% of the time, not taken 10% of the time + // A takenRatio of 0.10 means taken 10% of the time, not taken 90% of the time + // A takenRatio of 0.50 means taken 50% of the time, not taken 50% of the time + // A takenRatio of 0.90 means taken 90% of the time, not taken 10% of the time // - double takenCount = - ((double)edgeToDest->edgeWeightMin() + (double)edgeToDest->edgeWeightMax()) / 2.0; - double notTakenCount = - ((double)edgeToBlock->edgeWeightMin() + (double)edgeToBlock->edgeWeightMax()) / 2.0; + double takenCount = edgeToDest->getLikelyWeight(); + double notTakenCount = edgeToBlock->getLikelyWeight(); double totalCount = takenCount + notTakenCount; // If the takenRatio (takenCount / totalCount) is greater or equal to 51% then we will reverse @@ -3699,7 +3668,7 @@ bool Compiler::fgReorderBlocks(bool useProfile) else { // set profHotWeight - profHotWeight = (edgeToBlock->edgeWeightMin() + edgeToBlock->edgeWeightMax()) / 2 - 1; + profHotWeight = edgeToBlock->getLikelyWeight() - 1; } } else diff --git a/src/coreclr/jit/fgprofile.cpp b/src/coreclr/jit/fgprofile.cpp index 4a68e1a76b1c9..ba39201fb45f4 100644 --- a/src/coreclr/jit/fgprofile.cpp +++ b/src/coreclr/jit/fgprofile.cpp @@ -4802,26 +4802,21 @@ void Compiler::fgDebugCheckProfileWeights(ProfileChecks checks) // bool Compiler::fgDebugCheckIncomingProfileData(BasicBlock* block, ProfileChecks checks) { - const bool verifyClassicWeights = fgEdgeWeightsComputed && hasFlag(checks, ProfileChecks::CHECK_CLASSIC); const bool verifyLikelyWeights = hasFlag(checks, ProfileChecks::CHECK_LIKELY); const bool verifyHasLikelihood = hasFlag(checks, ProfileChecks::CHECK_HASLIKELIHOOD); - if (!(verifyClassicWeights || verifyLikelyWeights || verifyHasLikelihood)) + if (!verifyLikelyWeights && !verifyHasLikelihood) { return true; } weight_t const blockWeight = block->bbWeight; - weight_t incomingWeightMin = 0; - weight_t incomingWeightMax = 0; weight_t incomingLikelyWeight = 0; unsigned missingLikelyWeight = 0; bool foundPreds = false; for (FlowEdge* const predEdge : block->PredEdges()) { - incomingWeightMin += predEdge->edgeWeightMin(); - incomingWeightMax += predEdge->edgeWeightMax(); if (predEdge->hasLikelihood()) { if (BasicBlock::sameHndRegion(block, predEdge->getSourceBlock())) @@ -4839,35 +4834,10 @@ bool Compiler::fgDebugCheckIncomingProfileData(BasicBlock* block, ProfileChecks foundPreds = true; } - bool classicWeightsValid = true; bool likelyWeightsValid = true; if (foundPreds) { - if (verifyClassicWeights) - { - if (!fgProfileWeightsConsistent(incomingWeightMin, incomingWeightMax)) - { - JITDUMP(" " FMT_BB " - incoming min " FMT_WT " inconsistent with incoming max " FMT_WT "\n", - block->bbNum, incomingWeightMin, incomingWeightMax); - classicWeightsValid = false; - } - - if (!fgProfileWeightsConsistent(blockWeight, incomingWeightMin)) - { - JITDUMP(" " FMT_BB " - block weight " FMT_WT " inconsistent with incoming min " FMT_WT "\n", - block->bbNum, blockWeight, incomingWeightMin); - classicWeightsValid = false; - } - - if (!fgProfileWeightsConsistent(blockWeight, incomingWeightMax)) - { - JITDUMP(" " FMT_BB " - block weight " FMT_WT " inconsistent with incoming max " FMT_WT "\n", - block->bbNum, blockWeight, incomingWeightMax); - classicWeightsValid = false; - } - } - if (verifyLikelyWeights) { if (!fgProfileWeightsConsistent(blockWeight, incomingLikelyWeight)) @@ -4889,7 +4859,7 @@ bool Compiler::fgDebugCheckIncomingProfileData(BasicBlock* block, ProfileChecks } } - return classicWeightsValid && likelyWeightsValid; + return likelyWeightsValid; } //------------------------------------------------------------------------ @@ -4908,16 +4878,14 @@ bool Compiler::fgDebugCheckIncomingProfileData(BasicBlock* block, ProfileChecks // bool Compiler::fgDebugCheckOutgoingProfileData(BasicBlock* block, ProfileChecks checks) { - const bool verifyClassicWeights = fgEdgeWeightsComputed && hasFlag(checks, ProfileChecks::CHECK_CLASSIC); const bool verifyHasLikelihood = hasFlag(checks, ProfileChecks::CHECK_HASLIKELIHOOD); const bool verifyLikelihoodSum = hasFlag(checks, ProfileChecks::CHECK_LIKELIHOODSUM); - if (!(verifyClassicWeights || verifyHasLikelihood || verifyLikelihoodSum)) + if (!verifyHasLikelihood && !verifyLikelihoodSum) { return true; } - bool classicWeightsValid = true; bool likelyWeightsValid = true; // We want switch targets unified, but not EH edges. @@ -4927,8 +4895,6 @@ bool Compiler::fgDebugCheckOutgoingProfileData(BasicBlock* block, ProfileChecks if ((numSuccs > 0) && !block->KindIs(BBJ_EHFINALLYRET, BBJ_EHFAULTRET, BBJ_EHFILTERRET)) { weight_t const blockWeight = block->bbWeight; - weight_t outgoingWeightMin = 0; - weight_t outgoingWeightMax = 0; weight_t outgoingLikelihood = 0; // Walk successor edges and add up flow counts. @@ -4941,9 +4907,6 @@ bool Compiler::fgDebugCheckOutgoingProfileData(BasicBlock* block, ProfileChecks assert(succEdge != nullptr); BasicBlock* succBlock = succEdge->getDestinationBlock(); - outgoingWeightMin += succEdge->edgeWeightMin(); - outgoingWeightMax += succEdge->edgeWeightMax(); - if (succEdge->hasLikelihood()) { outgoingLikelihood += succEdge->getLikelihood(); @@ -4958,34 +4921,9 @@ bool Compiler::fgDebugCheckOutgoingProfileData(BasicBlock* block, ProfileChecks if (missingEdges > 0) { JITDUMP(" " FMT_BB " - missing %d successor edges\n", block->bbNum, missingEdges); - classicWeightsValid = false; likelyWeightsValid = false; } - if (verifyClassicWeights) - { - if (!fgProfileWeightsConsistent(outgoingWeightMin, outgoingWeightMax)) - { - JITDUMP(" " FMT_BB " - outgoing min " FMT_WT " inconsistent with outgoing max " FMT_WT "\n", - block->bbNum, outgoingWeightMin, outgoingWeightMax); - classicWeightsValid = false; - } - - if (!fgProfileWeightsConsistent(blockWeight, outgoingWeightMin)) - { - JITDUMP(" " FMT_BB " - block weight " FMT_WT " inconsistent with outgoing min " FMT_WT "\n", - block->bbNum, blockWeight, outgoingWeightMin); - classicWeightsValid = false; - } - - if (!fgProfileWeightsConsistent(blockWeight, outgoingWeightMax)) - { - JITDUMP(" " FMT_BB " - block weight " FMT_WT " inconsistent with outgoing max " FMT_WT "\n", - block->bbNum, blockWeight, outgoingWeightMax); - classicWeightsValid = false; - } - } - if (verifyHasLikelihood) { if (missingLikelihood > 0) @@ -5017,7 +4955,7 @@ bool Compiler::fgDebugCheckOutgoingProfileData(BasicBlock* block, ProfileChecks } } - return classicWeightsValid && likelyWeightsValid; + return likelyWeightsValid; } #endif // DEBUG diff --git a/src/coreclr/jit/optimizer.cpp b/src/coreclr/jit/optimizer.cpp index 0b4335af748b2..39d0ac57cac19 100644 --- a/src/coreclr/jit/optimizer.cpp +++ b/src/coreclr/jit/optimizer.cpp @@ -3131,7 +3131,6 @@ bool Compiler::optCanonicalizeExit(FlowGraphNaturalLoop* loop, BasicBlock* exit) // void Compiler::optSetWeightForPreheaderOrExit(FlowGraphNaturalLoop* loop, BasicBlock* block) { - assert(!block->HasAnyFlag(BBF_RUN_RARELY | BBF_PROF_WEIGHT)); bool hasProfWeight = true; weight_t newWeight = BB_ZERO_WEIGHT; @@ -3142,16 +3141,23 @@ void Compiler::optSetWeightForPreheaderOrExit(FlowGraphNaturalLoop* loop, BasicB } block->bbWeight = newWeight; - + if (hasProfWeight) { block->SetFlags(BBF_PROF_WEIGHT); } + else + { + block->RemoveFlags(BBF_PROF_WEIGHT); + } if (newWeight == BB_ZERO_WEIGHT) { block->SetFlags(BBF_RUN_RARELY); - return; + } + else + { + block->RemoveFlags(BBF_RUN_RARELY); } } From 673f7ce04b8bdf38543b840ae4bb2fefd138ca9c Mon Sep 17 00:00:00 2001 From: "Aman Khalid (from Dev Box)" Date: Wed, 13 Mar 2024 16:05:18 -0400 Subject: [PATCH 04/12] Remove ProfileChecks::CHECK_CLASSIC --- src/coreclr/jit/compiler.h | 11 +++++------ src/coreclr/jit/fgprofile.cpp | 7 +++---- src/coreclr/jit/jitconfigvalues.h | 7 ++++++- 3 files changed, 14 insertions(+), 11 deletions(-) diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index 9f17beecddf87..d4d9bf67e2e4f 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -1582,12 +1582,11 @@ enum API_ICorJitInfo_Names enum class ProfileChecks : unsigned int { CHECK_NONE = 0, - CHECK_CLASSIC = 1 << 0, // check "classic" jit weights TODO REMOVE - CHECK_HASLIKELIHOOD = 1 << 1, // check all FlowEdges for hasLikelihood - CHECK_LIKELIHOODSUM = 1 << 2, // check block succesor likelihoods sum to 1 - CHECK_LIKELY = 1 << 3, // fully check likelihood based weights - RAISE_ASSERT = 1 << 4, // assert on check failure - CHECK_ALL_BLOCKS = 1 << 5, // check blocks even if bbHasProfileWeight is false + CHECK_HASLIKELIHOOD = 1 << 0, // check all FlowEdges for hasLikelihood + CHECK_LIKELIHOODSUM = 1 << 1, // check block succesor likelihoods sum to 1 + CHECK_LIKELY = 1 << 2, // fully check likelihood based weights + RAISE_ASSERT = 1 << 3, // assert on check failure + CHECK_ALL_BLOCKS = 1 << 4, // check blocks even if bbHasProfileWeight is false }; inline constexpr ProfileChecks operator ~(ProfileChecks a) diff --git a/src/coreclr/jit/fgprofile.cpp b/src/coreclr/jit/fgprofile.cpp index ba39201fb45f4..989d9eae673f7 100644 --- a/src/coreclr/jit/fgprofile.cpp +++ b/src/coreclr/jit/fgprofile.cpp @@ -4611,14 +4611,13 @@ void Compiler::fgDebugCheckProfileWeights(ProfileChecks checks) // and/or // new likelihood based weights. // - const bool verifyClassicWeights = fgEdgeWeightsComputed && hasFlag(checks, ProfileChecks::CHECK_CLASSIC); const bool verifyLikelyWeights = hasFlag(checks, ProfileChecks::CHECK_LIKELY); const bool verifyHasLikelihood = hasFlag(checks, ProfileChecks::CHECK_HASLIKELIHOOD); const bool verifyLikelihoodSum = hasFlag(checks, ProfileChecks::CHECK_LIKELIHOODSUM); const bool assertOnFailure = hasFlag(checks, ProfileChecks::RAISE_ASSERT); const bool checkAllBlocks = hasFlag(checks, ProfileChecks::CHECK_ALL_BLOCKS); - if (!(verifyClassicWeights || verifyLikelyWeights || verifyHasLikelihood)) + if (!verifyLikelyWeights && !verifyHasLikelihood) { JITDUMP("[profile weight checks disabled]\n"); return; @@ -4734,7 +4733,7 @@ void Compiler::fgDebugCheckProfileWeights(ProfileChecks checks) // Verify overall input-output balance. // - if (verifyClassicWeights || verifyLikelyWeights) + if (verifyLikelyWeights) { if (entryProfiled && exitProfiled) { @@ -4760,7 +4759,7 @@ void Compiler::fgDebugCheckProfileWeights(ProfileChecks checks) { JITDUMP("No blocks were profiled, so nothing to check\n"); } - else if (verifyClassicWeights || verifyLikelyWeights) + else if (verifyLikelyWeights) { JITDUMP("Profile is self-consistent (%d profiled blocks, %d unprofiled)\n", profiledBlocks, unprofiledBlocks); diff --git a/src/coreclr/jit/jitconfigvalues.h b/src/coreclr/jit/jitconfigvalues.h index ee8320344ce42..c2440b4a65ccc 100644 --- a/src/coreclr/jit/jitconfigvalues.h +++ b/src/coreclr/jit/jitconfigvalues.h @@ -145,7 +145,12 @@ CONFIG_METHODSET(JitPrintInlinedMethods, W("JitPrintInlinedMethods")) CONFIG_METHODSET(JitPrintDevirtualizedMethods, W("JitPrintDevirtualizedMethods")) // -1: just do internal checks -// Else bitflag: 0x1 check classic, 0x2 check likely, 0x4 enable asserts +// Else bitflag: +// - 0x1: check edges have likelihoods +// - 0x2: check edge likelihoods sum to 1.0 +// - 0x4: fully check likelihoods +// - 0x8: assert on check failure +// - 0x10: check block profile weights CONFIG_INTEGER(JitProfileChecks, W("JitProfileChecks"), -1) CONFIG_INTEGER(JitRequired, W("JITRequired"), -1) CONFIG_INTEGER(JitRoundFloat, W("JITRoundFloat"), DEFAULT_ROUND_LEVEL) From a3f88baa15a4d49de997d0b5f705057e1b486386 Mon Sep 17 00:00:00 2001 From: "Aman Khalid (from Dev Box)" Date: Wed, 13 Mar 2024 16:15:39 -0400 Subject: [PATCH 05/12] Remove unused edge weight vars --- src/coreclr/jit/codegencommon.cpp | 5 +- src/coreclr/jit/compiler.h | 12 +-- src/coreclr/jit/fgbasic.cpp | 8 +- src/coreclr/jit/fgopt.cpp | 141 ++++++++---------------------- src/coreclr/jit/fgprofile.cpp | 1 - 5 files changed, 45 insertions(+), 122 deletions(-) diff --git a/src/coreclr/jit/codegencommon.cpp b/src/coreclr/jit/codegencommon.cpp index f95775a6c1fb2..18cf5b548d8ef 100644 --- a/src/coreclr/jit/codegencommon.cpp +++ b/src/coreclr/jit/codegencommon.cpp @@ -1859,9 +1859,8 @@ void CodeGen::genGenerateMachineCode() if (compiler->fgHaveProfileWeights()) { - printf("; with %s: edge weights are %s, and fgCalledCount is " FMT_WT "\n", - compiler->compGetPgoSourceName(), compiler->fgHaveValidEdgeWeights ? "valid" : "invalid", - compiler->fgCalledCount); + printf("; with %s: fgCalledCount is " FMT_WT "\n", + compiler->compGetPgoSourceName(), compiler->fgCalledCount); } if (compiler->fgPgoFailReason != nullptr) diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index d4d9bf67e2e4f..5b53804a15d7e 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -5174,14 +5174,10 @@ class Compiler // - Rationalization links all nodes into linear form which is kept until // the end of compilation. The first and last nodes are stored in the block. NodeThreading fgNodeThreading; - bool fgCanRelocateEHRegions; // true if we are allowed to relocate the EH regions - bool fgEdgeWeightsComputed; // true after we have called fgComputeEdgeWeights TODO: remove - bool fgHaveValidEdgeWeights; // true if we were successful in computing all of the edge weights TODO: remove - bool fgSlopUsedInEdgeWeights; // true if their was some slop used when computing the edge weights TODO: remove - bool fgRangeUsedInEdgeWeights; // true if some of the edgeWeight are expressed in Min..Max form TODO: remove - weight_t fgCalledCount; // count of the number of times this method was called - // This is derived from the profile data - // or is BB_UNITY_WEIGHT when we don't have profile data + bool fgCanRelocateEHRegions; // true if we are allowed to relocate the EH regions + weight_t fgCalledCount; // count of the number of times this method was called + // This is derived from the profile data + // or is BB_UNITY_WEIGHT when we don't have profile data #if defined(FEATURE_EH_FUNCLETS) bool fgFuncletsCreated; // true if the funclet creation phase has been run diff --git a/src/coreclr/jit/fgbasic.cpp b/src/coreclr/jit/fgbasic.cpp index 9f02bc1968402..9539d5ca2bd1a 100644 --- a/src/coreclr/jit/fgbasic.cpp +++ b/src/coreclr/jit/fgbasic.cpp @@ -23,12 +23,8 @@ void Compiler::fgInit() /* We haven't yet computed the bbPreds lists */ fgPredsComputed = false; - /* We haven't yet computed the edge weight */ - fgEdgeWeightsComputed = false; - fgHaveValidEdgeWeights = false; - fgSlopUsedInEdgeWeights = false; - fgRangeUsedInEdgeWeights = true; - fgCalledCount = BB_ZERO_WEIGHT; + /* We haven't yet computed block weights */ + fgCalledCount = BB_ZERO_WEIGHT; /* Initialize the basic block list */ diff --git a/src/coreclr/jit/fgopt.cpp b/src/coreclr/jit/fgopt.cpp index 1ac51a29de1dc..1167af92dce32 100644 --- a/src/coreclr/jit/fgopt.cpp +++ b/src/coreclr/jit/fgopt.cpp @@ -1684,31 +1684,6 @@ bool Compiler::fgOptimizeEmptyBlock(BasicBlock* block) break; } - // When using profile weights, fgComputeEdgeWeights expects the first non-internal block to have profile - // weight. - // Make sure we don't break that invariant. - if (fgIsUsingProfileWeights() && block->hasProfileWeight() && !block->HasFlag(BBF_INTERNAL)) - { - BasicBlock* bNext = block->Next(); - - // Check if the next block can't maintain the invariant. - if ((bNext == nullptr) || bNext->HasFlag(BBF_INTERNAL) || !bNext->hasProfileWeight()) - { - // Check if the current block is the first non-internal block. - BasicBlock* curBB = bPrev; - while ((curBB != nullptr) && curBB->HasFlag(BBF_INTERNAL)) - { - curBB = curBB->Prev(); - } - if (curBB == nullptr) - { - // This block is the first non-internal block and it has profile weight. - // Don't delete it. - break; - } - } - } - /* Remove the block */ compCurBB = block; fgRemoveBlock(block, /* unreachable */ false); @@ -3625,87 +3600,45 @@ bool Compiler::fgReorderBlocks(bool useProfile) // We will consider all blocks that have less weight than profHotWeight to be // uncommonly run blocks as compared with the hot path of bPrev taken-jump to bDest // - if (fgHaveValidEdgeWeights) - { - // We have valid edge weights, however even with valid edge weights - // we may have a minimum and maximum range for each edges value - // - // We will check that the min weight of the bPrev to bDest edge - // is more than twice the max weight of the bPrev to block edge. - // - // bPrev --> [BB04, weight 31] - // | \. - // edgeToBlock -------------> O \. - // [min=8,max=10] V \. - // block --> [BB05, weight 10] \. - // \. - // edgeToDest ----------------------------> O - // [min=21,max=23] | - // V - // bDest ---------------> [BB08, weight 21] - // - assert(bPrev->FalseTargetIs(block)); - FlowEdge* edgeToDest = bPrev->GetTrueEdge(); - FlowEdge* edgeToBlock = bPrev->GetFalseEdge(); - noway_assert(edgeToDest != nullptr); - noway_assert(edgeToBlock != nullptr); - // - // Calculate the taken ratio - // A takenRatio of 0.10 means taken 10% of the time, not taken 90% of the time - // A takenRatio of 0.50 means taken 50% of the time, not taken 50% of the time - // A takenRatio of 0.90 means taken 90% of the time, not taken 10% of the time - // - double takenCount = edgeToDest->getLikelyWeight(); - double notTakenCount = edgeToBlock->getLikelyWeight(); - double totalCount = takenCount + notTakenCount; + // We will check that the weight of the bPrev to bDest edge + // is more than twice the weight of the bPrev to block edge. + // + // bPrev --> [BB04, weight 31] + // | \. + // edgeToBlock -------------> O \. + // [min=8,max=10] V \. + // block --> [BB05, weight 10] \. + // \. + // edgeToDest ----------------------------> O + // [min=21,max=23] | + // V + // bDest ---------------> [BB08, weight 21] + // + assert(bPrev->FalseTargetIs(block)); + FlowEdge* edgeToDest = bPrev->GetTrueEdge(); + FlowEdge* edgeToBlock = bPrev->GetFalseEdge(); + noway_assert(edgeToDest != nullptr); + noway_assert(edgeToBlock != nullptr); + // + // Calculate the taken ratio + // A takenRatio of 0.10 means taken 10% of the time, not taken 90% of the time + // A takenRatio of 0.50 means taken 50% of the time, not taken 50% of the time + // A takenRatio of 0.90 means taken 90% of the time, not taken 10% of the time + // + double takenCount = edgeToDest->getLikelyWeight(); + double notTakenCount = edgeToBlock->getLikelyWeight(); + double totalCount = takenCount + notTakenCount; - // If the takenRatio (takenCount / totalCount) is greater or equal to 51% then we will reverse - // the branch - if (takenCount < (0.51 * totalCount)) - { - reorderBlock = false; - } - else - { - // set profHotWeight - profHotWeight = edgeToBlock->getLikelyWeight() - 1; - } + // If the takenRatio (takenCount / totalCount) is greater or equal to 51% then we will reverse + // the branch + if (takenCount < (0.51 * totalCount)) + { + reorderBlock = false; } else { - // We don't have valid edge weight so we will be more conservative - // We could have bPrev, block or bDest as part of a loop and thus have extra weight - // - // We will do two checks: - // 1. Check that the weight of bDest is at least two times more than block - // 2. Check that the weight of bPrev is at least three times more than block - // - // bPrev --> [BB04, weight 31] - // | \. - // V \. - // block --> [BB05, weight 10] \. - // \. - // | - // V - // bDest ---------------> [BB08, weight 21] - // - // For this case weightDest is calculated as (21+1)/2 or 11 - // and weightPrev is calculated as (31+2)/3 also 11 - // - // Generally both weightDest and weightPrev should calculate - // the same value unless bPrev or bDest are part of a loop - // - weight_t weightDest = bDest->isMaxBBWeight() ? bDest->bbWeight : (bDest->bbWeight + 1) / 2; - weight_t weightPrev = bPrev->isMaxBBWeight() ? bPrev->bbWeight : (bPrev->bbWeight + 2) / 3; - - // select the lower of weightDest and weightPrev - profHotWeight = (weightDest < weightPrev) ? weightDest : weightPrev; - - // if the weight of block is greater (or equal) to profHotWeight then we don't reverse the cond - if (block->bbWeight >= profHotWeight) - { - reorderBlock = false; - } + // set profHotWeight + profHotWeight = edgeToBlock->getLikelyWeight() - 1; } } } @@ -4834,9 +4767,9 @@ bool Compiler::fgUpdateFlowGraph(bool doTailDuplication /* = false */, bool isPh // if (fgIsUsingProfileWeights()) { - // if block and bdest are in different hot/cold regions we can't do this optimization + // if block and bDest are in different hot/cold regions we can't do this optimization // because we can't allow fall-through into the cold region. - if (!fgEdgeWeightsComputed || fgInDifferentRegions(block, bDest)) + if (fgInDifferentRegions(block, bDest)) { optimizeJump = false; } diff --git a/src/coreclr/jit/fgprofile.cpp b/src/coreclr/jit/fgprofile.cpp index 989d9eae673f7..704c90dbe03dc 100644 --- a/src/coreclr/jit/fgprofile.cpp +++ b/src/coreclr/jit/fgprofile.cpp @@ -4260,7 +4260,6 @@ PhaseStatus Compiler::fgComputeBlockAndEdgeWeights() const bool usingProfileWeights = fgIsUsingProfileWeights(); bool madeChanges = false; fgModified = false; - fgHaveValidEdgeWeights = false; fgCalledCount = BB_UNITY_WEIGHT; #if DEBUG From c44933f8c937f4716ed58b703e606d4c44f307cc Mon Sep 17 00:00:00 2001 From: "Aman Khalid (from Dev Box)" Date: Wed, 13 Mar 2024 17:36:22 -0400 Subject: [PATCH 06/12] Revert fgOptimizeEmptyBlock change --- src/coreclr/jit/fgopt.cpp | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/src/coreclr/jit/fgopt.cpp b/src/coreclr/jit/fgopt.cpp index 1167af92dce32..f6928782b619d 100644 --- a/src/coreclr/jit/fgopt.cpp +++ b/src/coreclr/jit/fgopt.cpp @@ -1684,6 +1684,31 @@ bool Compiler::fgOptimizeEmptyBlock(BasicBlock* block) break; } + // When using profile weights, fgComputeCalledCount expects the first non-internal block to have profile + // weight. + // Make sure we don't break that invariant. + if (fgIsUsingProfileWeights() && block->hasProfileWeight() && !block->HasFlag(BBF_INTERNAL)) + { + BasicBlock* bNext = block->Next(); + + // Check if the next block can't maintain the invariant. + if ((bNext == nullptr) || bNext->HasFlag(BBF_INTERNAL) || !bNext->hasProfileWeight()) + { + // Check if the current block is the first non-internal block. + BasicBlock* curBB = bPrev; + while ((curBB != nullptr) && curBB->HasFlag(BBF_INTERNAL)) + { + curBB = curBB->Prev(); + } + if (curBB == nullptr) + { + // This block is the first non-internal block and it has profile weight. + // Don't delete it. + break; + } + } + } + /* Remove the block */ compCurBB = block; fgRemoveBlock(block, /* unreachable */ false); From de0392c7fb584fecba5070b0bb4e4dc2a4700296 Mon Sep 17 00:00:00 2001 From: "Aman Khalid (from Dev Box)" Date: Wed, 13 Mar 2024 22:35:58 -0400 Subject: [PATCH 07/12] Style --- src/coreclr/jit/codegencommon.cpp | 4 ++-- src/coreclr/jit/fgdiagnostic.cpp | 2 +- src/coreclr/jit/fgopt.cpp | 4 ++-- src/coreclr/jit/fgprofile.cpp | 24 ++++++++++++------------ src/coreclr/jit/optimizer.cpp | 4 ++-- 5 files changed, 19 insertions(+), 19 deletions(-) diff --git a/src/coreclr/jit/codegencommon.cpp b/src/coreclr/jit/codegencommon.cpp index 18cf5b548d8ef..f66ccf5e8a22e 100644 --- a/src/coreclr/jit/codegencommon.cpp +++ b/src/coreclr/jit/codegencommon.cpp @@ -1859,8 +1859,8 @@ void CodeGen::genGenerateMachineCode() if (compiler->fgHaveProfileWeights()) { - printf("; with %s: fgCalledCount is " FMT_WT "\n", - compiler->compGetPgoSourceName(), compiler->fgCalledCount); + printf("; with %s: fgCalledCount is " FMT_WT "\n", compiler->compGetPgoSourceName(), + compiler->fgCalledCount); } if (compiler->fgPgoFailReason != nullptr) diff --git a/src/coreclr/jit/fgdiagnostic.cpp b/src/coreclr/jit/fgdiagnostic.cpp index 6de79410e3e3b..0ab8a7498e6d2 100644 --- a/src/coreclr/jit/fgdiagnostic.cpp +++ b/src/coreclr/jit/fgdiagnostic.cpp @@ -1085,7 +1085,7 @@ bool Compiler::fgDumpFlowGraph(Phases phase, PhasePosition pos) fprintf(fgxFile, "\n switchDefault=\"true\""); } } - + const weight_t edgeWeight = edge->getLikelyWeight(); fprintf(fgxFile, "\n weight="); fprintfDouble(fgxFile, ((double)edgeWeight) / weightDivisor); diff --git a/src/coreclr/jit/fgopt.cpp b/src/coreclr/jit/fgopt.cpp index f6928782b619d..00044f4be9eda 100644 --- a/src/coreclr/jit/fgopt.cpp +++ b/src/coreclr/jit/fgopt.cpp @@ -3650,9 +3650,9 @@ bool Compiler::fgReorderBlocks(bool useProfile) // A takenRatio of 0.50 means taken 50% of the time, not taken 50% of the time // A takenRatio of 0.90 means taken 90% of the time, not taken 10% of the time // - double takenCount = edgeToDest->getLikelyWeight(); + double takenCount = edgeToDest->getLikelyWeight(); double notTakenCount = edgeToBlock->getLikelyWeight(); - double totalCount = takenCount + notTakenCount; + double totalCount = takenCount + notTakenCount; // If the takenRatio (takenCount / totalCount) is greater or equal to 51% then we will reverse // the branch diff --git a/src/coreclr/jit/fgprofile.cpp b/src/coreclr/jit/fgprofile.cpp index 704c90dbe03dc..1e674f9be629c 100644 --- a/src/coreclr/jit/fgprofile.cpp +++ b/src/coreclr/jit/fgprofile.cpp @@ -4610,11 +4610,11 @@ void Compiler::fgDebugCheckProfileWeights(ProfileChecks checks) // and/or // new likelihood based weights. // - const bool verifyLikelyWeights = hasFlag(checks, ProfileChecks::CHECK_LIKELY); - const bool verifyHasLikelihood = hasFlag(checks, ProfileChecks::CHECK_HASLIKELIHOOD); - const bool verifyLikelihoodSum = hasFlag(checks, ProfileChecks::CHECK_LIKELIHOODSUM); - const bool assertOnFailure = hasFlag(checks, ProfileChecks::RAISE_ASSERT); - const bool checkAllBlocks = hasFlag(checks, ProfileChecks::CHECK_ALL_BLOCKS); + const bool verifyLikelyWeights = hasFlag(checks, ProfileChecks::CHECK_LIKELY); + const bool verifyHasLikelihood = hasFlag(checks, ProfileChecks::CHECK_HASLIKELIHOOD); + const bool verifyLikelihoodSum = hasFlag(checks, ProfileChecks::CHECK_LIKELIHOODSUM); + const bool assertOnFailure = hasFlag(checks, ProfileChecks::RAISE_ASSERT); + const bool checkAllBlocks = hasFlag(checks, ProfileChecks::CHECK_ALL_BLOCKS); if (!verifyLikelyWeights && !verifyHasLikelihood) { @@ -4800,8 +4800,8 @@ void Compiler::fgDebugCheckProfileWeights(ProfileChecks checks) // bool Compiler::fgDebugCheckIncomingProfileData(BasicBlock* block, ProfileChecks checks) { - const bool verifyLikelyWeights = hasFlag(checks, ProfileChecks::CHECK_LIKELY); - const bool verifyHasLikelihood = hasFlag(checks, ProfileChecks::CHECK_HASLIKELIHOOD); + const bool verifyLikelyWeights = hasFlag(checks, ProfileChecks::CHECK_LIKELY); + const bool verifyHasLikelihood = hasFlag(checks, ProfileChecks::CHECK_HASLIKELIHOOD); if (!verifyLikelyWeights && !verifyHasLikelihood) { @@ -4832,7 +4832,7 @@ bool Compiler::fgDebugCheckIncomingProfileData(BasicBlock* block, ProfileChecks foundPreds = true; } - bool likelyWeightsValid = true; + bool likelyWeightsValid = true; if (foundPreds) { @@ -4876,15 +4876,15 @@ bool Compiler::fgDebugCheckIncomingProfileData(BasicBlock* block, ProfileChecks // bool Compiler::fgDebugCheckOutgoingProfileData(BasicBlock* block, ProfileChecks checks) { - const bool verifyHasLikelihood = hasFlag(checks, ProfileChecks::CHECK_HASLIKELIHOOD); - const bool verifyLikelihoodSum = hasFlag(checks, ProfileChecks::CHECK_LIKELIHOODSUM); + const bool verifyHasLikelihood = hasFlag(checks, ProfileChecks::CHECK_HASLIKELIHOOD); + const bool verifyLikelihoodSum = hasFlag(checks, ProfileChecks::CHECK_LIKELIHOODSUM); if (!verifyHasLikelihood && !verifyLikelihoodSum) { return true; } - bool likelyWeightsValid = true; + bool likelyWeightsValid = true; // We want switch targets unified, but not EH edges. // @@ -4919,7 +4919,7 @@ bool Compiler::fgDebugCheckOutgoingProfileData(BasicBlock* block, ProfileChecks if (missingEdges > 0) { JITDUMP(" " FMT_BB " - missing %d successor edges\n", block->bbNum, missingEdges); - likelyWeightsValid = false; + likelyWeightsValid = false; } if (verifyHasLikelihood) diff --git a/src/coreclr/jit/optimizer.cpp b/src/coreclr/jit/optimizer.cpp index 39d0ac57cac19..17d838250e413 100644 --- a/src/coreclr/jit/optimizer.cpp +++ b/src/coreclr/jit/optimizer.cpp @@ -3131,8 +3131,8 @@ bool Compiler::optCanonicalizeExit(FlowGraphNaturalLoop* loop, BasicBlock* exit) // void Compiler::optSetWeightForPreheaderOrExit(FlowGraphNaturalLoop* loop, BasicBlock* block) { - bool hasProfWeight = true; - weight_t newWeight = BB_ZERO_WEIGHT; + bool hasProfWeight = true; + weight_t newWeight = BB_ZERO_WEIGHT; for (FlowEdge* const edge : block->PredEdges()) { From dbc9a5cb8879d5f5702de2447086cfbe8a269cd2 Mon Sep 17 00:00:00 2001 From: "Aman Khalid (from Dev Box)" Date: Thu, 14 Mar 2024 12:02:24 -0400 Subject: [PATCH 08/12] Rename block weight phase --- src/coreclr/jit/compiler.cpp | 4 ++-- src/coreclr/jit/compiler.h | 2 +- src/coreclr/jit/compphases.h | 3 +-- src/coreclr/jit/fgprofile.cpp | 5 ++--- 4 files changed, 6 insertions(+), 8 deletions(-) diff --git a/src/coreclr/jit/compiler.cpp b/src/coreclr/jit/compiler.cpp index 28adc6fc4dc15..43cfbde27fa05 100644 --- a/src/coreclr/jit/compiler.cpp +++ b/src/coreclr/jit/compiler.cpp @@ -4794,9 +4794,9 @@ void Compiler::compCompile(void** methodCodePtr, uint32_t* methodCodeSize, JitFl // DoPhase(this, PHASE_GS_COOKIE, &Compiler::gsPhase); - // Compute the block and edge weights + // Compute the block weights // - DoPhase(this, PHASE_COMPUTE_EDGE_WEIGHTS, &Compiler::fgComputeBlockAndEdgeWeights); + DoPhase(this, PHASE_COMPUTE_BLOCK_WEIGHTS, &Compiler::fgComputeBlockWeights); #if defined(FEATURE_EH_FUNCLETS) diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index 5b53804a15d7e..32b9b1ef2aff6 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -6007,7 +6007,7 @@ class Compiler #ifdef DEBUG void fgPrintEdgeWeights(); #endif - PhaseStatus fgComputeBlockAndEdgeWeights(); + PhaseStatus fgComputeBlockWeights(); bool fgComputeMissingBlockWeights(weight_t* returnWeight); bool fgComputeCalledCount(weight_t returnWeight); diff --git a/src/coreclr/jit/compphases.h b/src/coreclr/jit/compphases.h index ff6dd70d3e72f..bdf5d044c6816 100644 --- a/src/coreclr/jit/compphases.h +++ b/src/coreclr/jit/compphases.h @@ -54,7 +54,7 @@ CompPhaseNameMacro(PHASE_MORPH_GLOBAL, "Morph - Global", CompPhaseNameMacro(PHASE_POST_MORPH, "Post-Morph", false, -1, false) CompPhaseNameMacro(PHASE_MORPH_END, "Morph - Finish", false, -1, true) CompPhaseNameMacro(PHASE_GS_COOKIE, "GS Cookie", false, -1, false) -CompPhaseNameMacro(PHASE_COMPUTE_EDGE_WEIGHTS, "Compute edge weights (1, false)",false, -1, false) +CompPhaseNameMacro(PHASE_COMPUTE_BLOCK_WEIGHTS, "Compute block weights", false, -1, false) #if defined(FEATURE_EH_FUNCLETS) CompPhaseNameMacro(PHASE_CREATE_FUNCLETS, "Create EH funclets", false, -1, false) #endif // FEATURE_EH_FUNCLETS @@ -95,7 +95,6 @@ CompPhaseNameMacro(PHASE_ASSERTION_PROP_MAIN, "Assertion prop", CompPhaseNameMacro(PHASE_IF_CONVERSION, "If conversion", false, -1, false) CompPhaseNameMacro(PHASE_VN_BASED_DEAD_STORE_REMOVAL,"VN-based dead store removal", false, -1, false) CompPhaseNameMacro(PHASE_OPT_UPDATE_FLOW_GRAPH, "Update flow graph opt pass", false, -1, false) -CompPhaseNameMacro(PHASE_COMPUTE_EDGE_WEIGHTS2, "Compute edge weights (2, false)",false, -1, false) CompPhaseNameMacro(PHASE_STRESS_SPLIT_TREE, "Stress gtSplitTree", false, -1, false) CompPhaseNameMacro(PHASE_EXPAND_RTLOOKUPS, "Expand runtime lookups", false, -1, true) CompPhaseNameMacro(PHASE_EXPAND_STATIC_INIT, "Expand static init", false, -1, true) diff --git a/src/coreclr/jit/fgprofile.cpp b/src/coreclr/jit/fgprofile.cpp index 1e674f9be629c..046d0dba42a54 100644 --- a/src/coreclr/jit/fgprofile.cpp +++ b/src/coreclr/jit/fgprofile.cpp @@ -4249,13 +4249,12 @@ bool Compiler::fgIncorporateEdgeCounts() } //------------------------------------------------------------- -// fgComputeBlockAndEdgeWeights: determine weights for blocks -// and optionally for edges +// fgComputeBlockWeights: determine weights for blocks // // Returns: // Suitable phase status // -PhaseStatus Compiler::fgComputeBlockAndEdgeWeights() +PhaseStatus Compiler::fgComputeBlockWeights() { const bool usingProfileWeights = fgIsUsingProfileWeights(); bool madeChanges = false; From 2d01359ebadb154dfad3687306746d791f55340c Mon Sep 17 00:00:00 2001 From: "Aman Khalid (from Dev Box)" Date: Fri, 15 Mar 2024 11:22:37 -0400 Subject: [PATCH 09/12] Fix fgConnectFallThrough --- src/coreclr/jit/fgbasic.cpp | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/src/coreclr/jit/fgbasic.cpp b/src/coreclr/jit/fgbasic.cpp index ccfa8478e1f4e..b8e4eda37f6e4 100644 --- a/src/coreclr/jit/fgbasic.cpp +++ b/src/coreclr/jit/fgbasic.cpp @@ -5441,13 +5441,7 @@ BasicBlock* Compiler::fgConnectFallThrough(BasicBlock* bSrc, BasicBlock* bDst) // if (fgHaveProfileWeights()) { - jmpBlk->bbWeight = newEdge->getLikelyWeight(); - jmpBlk->SetFlags(BBF_PROF_WEIGHT); - - if (jmpBlk->bbWeight == BB_ZERO_WEIGHT) - { - jmpBlk->SetFlags(BBF_RUN_RARELY); - } + jmpBlk->setBBProfileWeight(newEdge->getLikelyWeight()); } else { From ddd705b8be4a2148a8080ae1e0d6ce7ed117be0c Mon Sep 17 00:00:00 2001 From: "Aman Khalid (from Dev Box)" Date: Fri, 15 Mar 2024 11:51:07 -0400 Subject: [PATCH 10/12] Use likely weights in fgOptimizeBranchToEmptyUnconditional --- src/coreclr/jit/fgopt.cpp | 43 +++++++++++++++++++-------------------- 1 file changed, 21 insertions(+), 22 deletions(-) diff --git a/src/coreclr/jit/fgopt.cpp b/src/coreclr/jit/fgopt.cpp index fb5fbb6213588..1e55b86645fe0 100644 --- a/src/coreclr/jit/fgopt.cpp +++ b/src/coreclr/jit/fgopt.cpp @@ -1462,29 +1462,28 @@ bool Compiler::fgOptimizeBranchToEmptyUnconditional(BasicBlock* block, BasicBloc // // When we optimize a branch to branch we need to update the profile weight - // of bDest by subtracting out the block/edge weight of the path that is being optimized. + // of bDest by subtracting out the weight of the path that is being optimized. // - // TODO: Update block weight using likely weight - // if (bDest->hasProfileWeight()) - // { - // FlowEdge* const edge = fgGetPredForBlock(bDest, block); - // noway_assert(edge != nullptr); - - // const weight_t edgeWeight = edge->getLikelyWeight(); - - // // - // // Update the bDest->bbWeight - // // - // if (bDest->bbWeight > edgeWeight) - // { - // bDest->bbWeight -= edgeWeight; - // } - // else - // { - // bDest->bbWeight = BB_ZERO_WEIGHT; - // bDest->SetFlags(BBF_RUN_RARELY); // Set the RarelyRun flag - // } - // } + if (bDest->hasProfileWeight()) + { + FlowEdge* const edge = fgGetPredForBlock(bDest, block); + noway_assert(edge != nullptr); + + const weight_t edgeWeight = edge->getLikelyWeight(); + + // + // Update the bDest->bbWeight + // + if (bDest->bbWeight > edgeWeight) + { + bDest->bbWeight -= edgeWeight; + } + else + { + bDest->bbWeight = BB_ZERO_WEIGHT; + bDest->SetFlags(BBF_RUN_RARELY); // Set the RarelyRun flag + } + } // Optimize the JUMP to empty unconditional JUMP to go to the new target switch (block->GetKind()) From 38248c3f69a5e7044ccbd24f7b98d6ff95c263a1 Mon Sep 17 00:00:00 2001 From: "Aman Khalid (from Dev Box)" Date: Mon, 15 Apr 2024 17:07:02 -0400 Subject: [PATCH 11/12] Style --- src/coreclr/jit/fgprofile.cpp | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/coreclr/jit/fgprofile.cpp b/src/coreclr/jit/fgprofile.cpp index 19261b55ad516..b65b4104c15c6 100644 --- a/src/coreclr/jit/fgprofile.cpp +++ b/src/coreclr/jit/fgprofile.cpp @@ -4660,11 +4660,11 @@ bool Compiler::fgDebugCheckProfileWeights(ProfileChecks checks) // and/or // new likelihood based weights. // - const bool verifyLikelyWeights = hasFlag(checks, ProfileChecks::CHECK_LIKELY); - const bool verifyHasLikelihood = hasFlag(checks, ProfileChecks::CHECK_HASLIKELIHOOD); - const bool verifyLikelihoodSum = hasFlag(checks, ProfileChecks::CHECK_LIKELIHOODSUM); - const bool assertOnFailure = hasFlag(checks, ProfileChecks::RAISE_ASSERT) && fgPgoConsistent; - const bool checkAllBlocks = hasFlag(checks, ProfileChecks::CHECK_ALL_BLOCKS); + const bool verifyLikelyWeights = hasFlag(checks, ProfileChecks::CHECK_LIKELY); + const bool verifyHasLikelihood = hasFlag(checks, ProfileChecks::CHECK_HASLIKELIHOOD); + const bool verifyLikelihoodSum = hasFlag(checks, ProfileChecks::CHECK_LIKELIHOODSUM); + const bool assertOnFailure = hasFlag(checks, ProfileChecks::RAISE_ASSERT) && fgPgoConsistent; + const bool checkAllBlocks = hasFlag(checks, ProfileChecks::CHECK_ALL_BLOCKS); if (!verifyLikelyWeights && !verifyHasLikelihood) { @@ -4937,7 +4937,7 @@ bool Compiler::fgDebugCheckIncomingProfileData(BasicBlock* block, ProfileChecks foundEHPreds = false; } - bool likelyWeightsValid = true; + bool likelyWeightsValid = true; // If we have EH preds we may not have consistent incoming flow. // From 6fdeb1da8da893439506954519f5e23433c851a6 Mon Sep 17 00:00:00 2001 From: "Aman Khalid (from Dev Box)" Date: Mon, 15 Apr 2024 17:09:36 -0400 Subject: [PATCH 12/12] Remove unused vars --- src/coreclr/jit/fgprofile.cpp | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/coreclr/jit/fgprofile.cpp b/src/coreclr/jit/fgprofile.cpp index b65b4104c15c6..79cd63e3ac3b5 100644 --- a/src/coreclr/jit/fgprofile.cpp +++ b/src/coreclr/jit/fgprofile.cpp @@ -4932,8 +4932,6 @@ bool Compiler::fgDebugCheckIncomingProfileData(BasicBlock* block, ProfileChecks if (block->isBBCallFinallyPairTail()) { incomingLikelyWeight = block->Prev()->bbWeight; - incomingWeightMin = incomingLikelyWeight; - incomingWeightMax = incomingLikelyWeight; foundEHPreds = false; }