Skip to content

Commit

Permalink
More fixes
Browse files Browse the repository at this point in the history 
This includes various fixes that are being separately PR'ed:
1. dotnet#99744:
Introduce HandleKindDataIsInvariant helper
2. dotnet#99743:
Add basic support for TYP_MASK constants
3. dotnet#99742:
Fix problem with scaling general loop blocks; add dumpers
4. dotnet#99740:
Add edge likelihood dumping; fix one edge likelihood update case

Also:
1. Add support for running JitOptRepeat under JitStress. This is still
over-written by JitOptRepeat being forced on at 4 iterations.
  • Loading branch information
@BruceForstall
BruceForstall committed Mar 14, 2024
1 parent 9629653 commit 496992f
Show file tree
Hide file tree
Showing 12 changed files with 341 additions and 93 deletions.
65 changes: 44 additions & 21 deletions src/coreclr/jit/block.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -79,11 +79,20 @@ void FlowEdge::setLikelihood(weight_t likelihood)
{
assert(likelihood >= 0.0);
assert(likelihood <= 1.0);

if (m_likelihoodSet)
{
JITDUMP("setting likelihood of " FMT_BB " -> " FMT_BB " from " FMT_WT " to " FMT_WT "\n", m_sourceBlock->bbNum,
m_destBlock->bbNum, m_likelihood, likelihood);
}
else
{
JITDUMP("setting likelihood of " FMT_BB " -> " FMT_BB " to " FMT_WT "\n", m_sourceBlock->bbNum,
m_destBlock->bbNum, likelihood);
}

m_likelihoodSet = true;
m_likelihood = likelihood;

JITDUMP("setting likelihood of " FMT_BB " -> " FMT_BB " to " FMT_WT "\n", m_sourceBlock->bbNum, m_destBlock->bbNum,
m_likelihood);
}

//------------------------------------------------------------------------
Expand Down Expand Up @@ -114,10 +123,11 @@ void FlowEdge::addLikelihood(weight_t addedLikelihood)

assert(newLikelihood >= 0.0);
assert(newLikelihood <= 1.0);
m_likelihood = newLikelihood;

JITDUMP("updating likelihood of " FMT_BB " -> " FMT_BB " to " FMT_WT "\n", m_sourceBlock->bbNum, m_destBlock->bbNum,
m_likelihood);
JITDUMP("updating likelihood of " FMT_BB " -> " FMT_BB " from " FMT_WT " to " FMT_WT "\n", m_sourceBlock->bbNum,
m_destBlock->bbNum, m_likelihood, newLikelihood);

m_likelihood = newLikelihood;
}

//------------------------------------------------------------------------
Expand Down Expand Up @@ -678,20 +688,33 @@ void BasicBlock::dspSuccs(Compiler* compiler)
// things strictly.
void BasicBlock::dspKind() const
{
auto dspBlockNum = [](const BasicBlock* b) -> const char* {
auto dspBlockNum = [](const FlowEdge* e) -> const char* {
static char buffers[3][64]; // static array of 3 to allow 3 concurrent calls in one printf()
static int nextBufferIndex = 0;

auto& buffer = buffers[nextBufferIndex];
nextBufferIndex = (nextBufferIndex + 1) % ArrLen(buffers);
auto& buffer = buffers[nextBufferIndex];
nextBufferIndex = (nextBufferIndex + 1) % ArrLen(buffers);
const size_t sizeOfBuffer = ArrLen(buffer);
int written;

const BasicBlock* b = e->getDestinationBlock();
if (b == nullptr)
{
_snprintf_s(buffer, ArrLen(buffer), ArrLen(buffer), "NULL");
written = _snprintf_s(buffer, sizeOfBuffer, sizeOfBuffer, "NULL");
}
else
{
_snprintf_s(buffer, ArrLen(buffer), ArrLen(buffer), FMT_BB, b->bbNum);
written = _snprintf_s(buffer, sizeOfBuffer, sizeOfBuffer, FMT_BB, b->bbNum);
}

const bool printEdgeLikelihoods = true; // TODO: parameterize this?
if (printEdgeLikelihoods)
{
if (e->hasLikelihood())
{
written = _snprintf_s(buffer + written, sizeOfBuffer - written, sizeOfBuffer - written, "(" FMT_WT ")",
e->getLikelihood());
}
}

return buffer;
Expand All @@ -715,7 +738,7 @@ void BasicBlock::dspKind() const

for (unsigned i = 0; i < jumpCnt; i++)
{
printf("%c%s", (i == 0) ? ' ' : ',', dspBlockNum(jumpTab[i]->getDestinationBlock()));
printf("%c%s", (i == 0) ? ' ' : ',', dspBlockNum(jumpTab[i]));
}
}

Expand All @@ -728,11 +751,11 @@ void BasicBlock::dspKind() const
break;

case BBJ_EHFILTERRET:
printf(" -> %s (fltret)", dspBlockNum(GetTarget()));
printf(" -> %s (fltret)", dspBlockNum(GetTargetEdge()));
break;

case BBJ_EHCATCHRET:
printf(" -> %s (cret)", dspBlockNum(GetTarget()));
printf(" -> %s (cret)", dspBlockNum(GetTargetEdge()));
break;

case BBJ_THROW:
Expand All @@ -746,28 +769,28 @@ void BasicBlock::dspKind() const
case BBJ_ALWAYS:
if (HasFlag(BBF_KEEP_BBJ_ALWAYS))
{
printf(" -> %s (ALWAYS)", dspBlockNum(GetTarget()));
printf(" -> %s (ALWAYS)", dspBlockNum(GetTargetEdge()));
}
else
{
printf(" -> %s (always)", dspBlockNum(GetTarget()));
printf(" -> %s (always)", dspBlockNum(GetTargetEdge()));
}
break;

case BBJ_LEAVE:
printf(" -> %s (leave)", dspBlockNum(GetTarget()));
printf(" -> %s (leave)", dspBlockNum(GetTargetEdge()));
break;

case BBJ_CALLFINALLY:
printf(" -> %s (callf)", dspBlockNum(GetTarget()));
printf(" -> %s (callf)", dspBlockNum(GetTargetEdge()));
break;

case BBJ_CALLFINALLYRET:
printf(" -> %s (callfr)", dspBlockNum(GetTarget()));
printf(" -> %s (callfr)", dspBlockNum(GetTargetEdge()));
break;

case BBJ_COND:
printf(" -> %s,%s (cond)", dspBlockNum(GetTrueTarget()), dspBlockNum(GetFalseTarget()));
printf(" -> %s,%s (cond)", dspBlockNum(GetTrueEdge()), dspBlockNum(GetFalseEdge()));
break;

case BBJ_SWITCH:
Expand All @@ -779,7 +802,7 @@ void BasicBlock::dspKind() const

for (unsigned i = 0; i < jumpCnt; i++)
{
printf("%c%s", (i == 0) ? ' ' : ',', dspBlockNum(jumpTab[i]->getDestinationBlock()));
printf("%c%s", (i == 0) ? ' ' : ',', dspBlockNum(jumpTab[i]));

const bool isDefault = bbSwtTargets->bbsHasDefault && (i == jumpCnt - 1);
if (isDefault)
Expand Down
29 changes: 27 additions & 2 deletions src/coreclr/jit/block.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,9 +46,13 @@ typedef BitVec_ValRet_T ASSERT_VALRET_TP;
// Use this format for loop indices
#define FMT_LP "L%02u"

// And this format for profile weights
// Use this format for profile weights
#define FMT_WT "%.7g"

// Use this format for profile weights where we want to conserve horizontal space, at the expense of displaying
// less precision.
#define FMT_WT_NARROW "%.3g"

/*****************************************************************************
*
* Each basic block ends with a jump which is described as a value
Expand Down Expand Up @@ -1037,6 +1041,27 @@ struct BasicBlock : private LIR::Range
return (bbFalseEdge == nullptr) ? nullptr : bbFalseEdge->getDestinationBlock();
}

// Return the target edge; it might be null. Only used during dumping.
FlowEdge* GetTargetEdgeRaw() const
{
assert(HasTarget());
return bbTargetEdge;
}

// Return the BBJ_COND true target edge; it might be null. Only used during dumping.
FlowEdge* GetTrueEdgeRaw() const
{
assert(KindIs(BBJ_COND));
return bbTrueEdge;
}

// Return the BBJ_COND false target edge; it might be null. Only used during dumping.
FlowEdge* GetFalseEdgeRaw() const
{
assert(KindIs(BBJ_COND));
return bbFalseEdge;
}

#endif // DEBUG

private:
Expand Down Expand Up @@ -1537,7 +1562,7 @@ struct BasicBlock : private LIR::Range
}

// PredBlocksEditing: convenience method for enabling range-based `for` iteration over predecessor blocks, e.g.:
// for (BasicBlock* const predBlock : block->PredBlocksList()) ...
// for (BasicBlock* const predBlock : block->PredBlocksEditing()) ...
// This iterator tolerates modifications to bbPreds.
//
PredBlockList<true> PredBlocksEditing() const
Expand Down
10 changes: 7 additions & 3 deletions src/coreclr/jit/clrjit.natvis
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,8 @@ The .NET Foundation licenses this file to you under the MIT license.
<!--
Visual Studio debugger visualizers for RyuJIT.
Documentation for VS natvis format: https://docs.microsoft.com/en-us/visualstudio/debugger/create-custom-views-of-native-objects?view=vs-2019
Documentation for VS debugger format specifiers: https://docs.microsoft.com/en-us/visualstudio/debugger/format-specifiers-in-cpp?view=vs-2019
Documentation for VS natvis format: https://learn.microsoft.com/en-us/visualstudio/debugger/create-custom-views-of-native-objects?view=vs-2022
Documentation for VS debugger format specifiers: https://learn.microsoft.com/en-us/visualstudio/debugger/format-specifiers-in-cpp?view=vs-2022
-->

<AutoVisualizer xmlns="http://schemas.microsoft.com/vstudio/debugger/natvis/2010">
Expand All @@ -27,6 +26,11 @@ Documentation for VS debugger format specifiers: https://docs.microsoft.com/en-u
<DisplayString>BB{bbNum,d}; {bbKind,en}</DisplayString>
</Type>

<Type Name="FlowEdge">
<DisplayString Condition="m_dupCount!=1">BB{m_sourceBlock->bbNum,d}->BB{m_destBlock->bbNum,d} ({m_likelihood,g}) (dup {m_dupCount,d})</DisplayString>
<DisplayString>BB{m_sourceBlock->bbNum,d}->BB{m_destBlock->bbNum,d} ({m_likelihood,g})</DisplayString>
</Type>

<Type Name="Compiler::LoopDsc">
<DisplayString Condition="lpFlags &amp; LPFLG_REMOVED">REMOVED</DisplayString>
<DisplayString Condition="lpFlags &amp; LPFLG_HAS_PREHEAD">[BB{lpTop->bbNum,d}..BB{lpBottom->bbNum,d}] pre-h:BB{lpHead->bbNum,d} e:BB{lpEntry->bbNum,d} {lpFlags,en}</DisplayString>
Expand Down
44 changes: 33 additions & 11 deletions src/coreclr/jit/compiler.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2872,6 +2872,7 @@ void Compiler::compInitOptions(JitFlags* jitFlags)
opts.dspUnwind = false;
opts.compLongAddress = false;
opts.optRepeat = false;
opts.optRepeatCount = 1;

#ifdef LATE_DISASM
opts.doLateDisasm = false;
Expand Down Expand Up @@ -2955,13 +2956,30 @@ void Compiler::compInitOptions(JitFlags* jitFlags)

if (JitConfig.JitOptRepeat().contains(info.compMethodHnd, info.compClassHnd, &info.compMethodInfo->args))
{
opts.optRepeat = true;
opts.optRepeat = true;
opts.optRepeatCount = JitConfig.JitOptRepeatCount();

JITDUMP("\n*************** JitOptRepeat enabled; repetition count: %d\n\n", opts.optRepeatCount);
}

opts.dspMetrics = (JitConfig.JitMetrics() != 0);
if (!opts.optRepeat && compStressCompile(STRESS_OPT_REPEAT, 10))
{
// Turn on optRepeat as part of JitStress. In this case, decide how many iterations to do, from 2 to 5,
// based on a random number seeded by the method hash.
opts.optRepeat = true;

CLRRandom rng;
rng.Init(info.compMethodHash());
opts.optRepeatCount = rng.Next(4) + 2; // generates [2..5]

JITDUMP("\n*************** JitOptRepeat under stress; repetition count: %d\n\n", opts.optRepeatCount);
}

////////////////// TESTING
opts.optRepeat = true;
opts.optRepeat = true;
opts.optRepeatCount = 4;

opts.dspMetrics = (JitConfig.JitMetrics() != 0);
}

if (verboseDump)
Expand Down Expand Up @@ -4916,7 +4934,7 @@ void Compiler::compCompile(void** methodCodePtr, uint32_t* methodCodeSize, JitFl

if (opts.optRepeat)
{
iterations = JitConfig.JitOptRepeatCount();
iterations = opts.optRepeatCount;
}
#endif // defined(OPT_CONFIG)

Expand Down Expand Up @@ -5033,6 +5051,13 @@ void Compiler::compCompile(void** methodCodePtr, uint32_t* methodCodeSize, JitFl
DoPhase(this, PHASE_COMPUTE_EDGE_WEIGHTS2, &Compiler::fgComputeEdgeWeights);
}

#ifdef DEBUG
if (verbose && opts.optRepeat)
{
printf("\n*************** JitOptRepeat: iterations remaining: %d\n\n", iterations - 1);
}
#endif // DEBUG

// Iterate if requested, resetting annotations first.
if (--iterations == 0)
{
Expand All @@ -5045,13 +5070,6 @@ void Compiler::compCompile(void** methodCodePtr, uint32_t* methodCodeSize, JitFl
//
DoPhase(this, PHASE_CANONICALIZE_ENTRY, &Compiler::fgCanonicalizeFirstBB);

#ifdef DEBUG
if (verbose)
{
printf("\n*************** JitOptRepeat: iterations remaining: %d\n\n", iterations);
}
#endif

ResetOptAnnotations();
RecomputeFlowGraphAnnotations();
}
Expand Down Expand Up @@ -5866,6 +5884,10 @@ void Compiler::RecomputeFlowGraphAnnotations()
fgInvalidateDfsTree();
fgDfsBlocksAndRemove();
optFindLoops();

// Should we call this using the phase method:
// DoPhase(this, PHASE_SET_BLOCK_WEIGHTS, &Compiler::optSetBlockWeights);
// ? It could be called multiple times.
optSetBlockWeights();

if (m_domTree == nullptr)
Expand Down
25 changes: 20 additions & 5 deletions src/coreclr/jit/compiler.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1584,7 +1584,7 @@ enum class ProfileChecks : unsigned int
CHECK_NONE = 0,
CHECK_CLASSIC = 1 << 0, // check "classic" jit weights
CHECK_HASLIKELIHOOD = 1 << 1, // check all FlowEdges for hasLikelihood
CHECK_LIKELIHOODSUM = 1 << 2, // check block succesor likelihoods sum to 1
CHECK_LIKELIHOODSUM = 1 << 2, // check block successor 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
Expand Down Expand Up @@ -1951,6 +1951,10 @@ class FlowGraphDfsTree
return m_hasCycle;
}

#ifdef DEBUG
void Dump() const;
#endif // DEBUG

bool Contains(BasicBlock* block) const;
bool IsAncestor(BasicBlock* ancestor, BasicBlock* descendant) const;
};
Expand Down Expand Up @@ -2230,7 +2234,7 @@ class FlowGraphNaturalLoops
return m_dfsTree;
}

size_t NumLoops()
size_t NumLoops() const
{
return m_loops.size();
}
Expand Down Expand Up @@ -2322,6 +2326,7 @@ class FlowGraphDominatorTree
}

static BasicBlock* IntersectDom(BasicBlock* block1, BasicBlock* block2);

public:
const FlowGraphDfsTree* GetDfsTree()
{
Expand Down Expand Up @@ -2355,6 +2360,10 @@ class BlockToNaturalLoopMap
FlowGraphNaturalLoop* GetLoop(BasicBlock* block);

static BlockToNaturalLoopMap* Build(FlowGraphNaturalLoops* loops);

#ifdef DEBUG
void Dump() const;
#endif // DEBUG
};

// Represents a data structure that can answer A -> B reachability queries in
Expand Down Expand Up @@ -6076,7 +6085,11 @@ class Compiler

void fgDispBBLiveness(BasicBlock* block);
void fgDispBBLiveness();
void fgTableDispBasicBlock(const BasicBlock* block, const BasicBlock* nextBlock = nullptr, int blockTargetFieldWidth = 21, int ibcColWidth = 0);
void fgTableDispBasicBlock(const BasicBlock* block,
const BasicBlock* nextBlock = nullptr,
bool printEdgeLikelihoods = true,
int blockTargetFieldWidth = 21,
int ibcColWidth = 0);
void fgDispBasicBlocks(BasicBlock* firstBlock, BasicBlock* lastBlock, bool dumpTrees);
void fgDispBasicBlocks(bool dumpTrees = false);
void fgDumpStmtTree(const BasicBlock* block, Statement* stmt);
Expand Down Expand Up @@ -9816,7 +9829,8 @@ XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX
bool altJit; // True if we are an altjit and are compiling this method

#ifdef OPT_CONFIG
bool optRepeat; // Repeat optimizer phases k times
bool optRepeat; // Repeat optimizer phases k times
int optRepeatCount; // How many times to repeat. By default, comes from JitConfig.JitOptRepeatCount().
#endif

bool disAsm; // Display native code as it is generated
Expand Down Expand Up @@ -10061,13 +10075,14 @@ XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX
STRESS_MODE(PHYSICAL_PROMOTION) /* Use physical promotion */ \
STRESS_MODE(PHYSICAL_PROMOTION_COST) \
STRESS_MODE(UNWIND) /* stress unwind info; e.g., create function fragments */ \
STRESS_MODE(OPT_REPEAT) /* stress JitOptRepeat */ \
\
/* After COUNT_VARN, stress level 2 does all of these all the time */ \
\
STRESS_MODE(COUNT_VARN) \
\
/* "Check" stress areas that can be exhaustively used if we */ \
/* dont care about performance at all */ \
/* don't care about performance at all */ \
\
STRESS_MODE(FORCE_INLINE) /* Treat every method as AggressiveInlining */ \
STRESS_MODE(CHK_FLOW_UPDATE) \
Expand Down
Loading

0 comments on commit 496992f

Please sign in to comment.