Skip to content

Commit

Permalink
JIT: peel off dominant switch case under PGO (#52827)
Browse files Browse the repository at this point in the history 
If we have PGO data and the dominant non-default switch case has more than
55% of the profile, add an explicit test for that case upstream of the switch.

We don't see switches all that often anymore as CSC is quite aggressive about
turning them into if-then-else trees, but they still show up in the async
methods.
  • Loading branch information
@AndyAyersMS
AndyAyersMS committed May 19, 2021
1 parent f6f1220 commit 6ec2fba
Show file tree
Hide file tree
Showing 6 changed files with 442 additions and 103 deletions.
53 changes: 45 additions & 8 deletions src/coreclr/jit/block.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -646,19 +646,32 @@ void BasicBlock::dspJumpKind()
break;

case BBJ_SWITCH:
{
printf(" ->");

unsigned jumpCnt;
jumpCnt = bbJumpSwt->bbsCount;
BasicBlock** jumpTab;
jumpTab = bbJumpSwt->bbsDstTab;
do
const unsigned jumpCnt = bbJumpSwt->bbsCount;
BasicBlock** const jumpTab = bbJumpSwt->bbsDstTab;

for (unsigned i = 0; i < jumpCnt; i++)
{
printf("%c" FMT_BB, (jumpTab == bbJumpSwt->bbsDstTab) ? ' ' : ',', (*jumpTab)->bbNum);
} while (++jumpTab, --jumpCnt);
printf("%c" FMT_BB, (i == 0) ? ' ' : ',', jumpTab[i]->bbNum);

const bool isDefault = bbJumpSwt->bbsHasDefault && (i == jumpCnt - 1);
if (isDefault)
{
printf("[def]");
}

const bool isDominant = bbJumpSwt->bbsHasDominantCase && (i == bbJumpSwt->bbsDominantCase);
if (isDominant)
{
printf("[dom(" FMT_WT ")]", bbJumpSwt->bbsDominantFraction);
}
}

printf(" (switch)");
break;
}
break;

default:
unreached();
Expand Down Expand Up @@ -1630,3 +1643,27 @@ bool BasicBlock::hasEHBoundaryOut()

return returnVal;
}

//------------------------------------------------------------------------
// BBswtDesc copy ctor: copy a switch descriptor
//
// Arguments:
// comp - compiler instance
// other - existing switch descriptor to copy
//
BBswtDesc::BBswtDesc(Compiler* comp, const BBswtDesc* other)
: bbsDstTab(nullptr)
, bbsCount(other->bbsCount)
, bbsDominantCase(other->bbsDominantCase)
, bbsDominantFraction(other->bbsDominantFraction)
, bbsHasDefault(other->bbsHasDefault)
, bbsHasDominantCase(other->bbsHasDominantCase)
{
// Allocate and fill in a new dst tab
//
bbsDstTab = new (comp, CMK_BasicBlock) BasicBlock*[bbsCount];
for (unsigned i = 0; i < bbsCount; i++)
{
bbsDstTab[i] = other->bbsDstTab[i];
}
}
86 changes: 48 additions & 38 deletions src/coreclr/jit/block.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -85,44 +85,7 @@ class typeInfo;
struct BasicBlockList;
struct flowList;
struct EHblkDsc;

/*****************************************************************************
*
* The following describes a switch block.
*
* Things to know:
* 1. If bbsHasDefault is true, the default case is the last one in the array of basic block addresses
* namely bbsDstTab[bbsCount - 1].
* 2. bbsCount must be at least 1, for the default case. bbsCount cannot be zero. It appears that the ECMA spec
* allows for a degenerate switch with zero cases. Normally, the optimizer will optimize degenerate
* switches with just a default case to a BBJ_ALWAYS branch, and a switch with just two cases to a BBJ_COND.
* However, in debuggable code, we might not do that, so bbsCount might be 1.
*/
struct BBswtDesc
{
BasicBlock** bbsDstTab; // case label table address
unsigned bbsCount; // count of cases (includes 'default' if bbsHasDefault)
bool bbsHasDefault;

BBswtDesc() : bbsHasDefault(true)
{
}

void removeDefault()
{
assert(bbsHasDefault);
assert(bbsCount > 0);
bbsHasDefault = false;
bbsCount--;
}

BasicBlock* getDefault()
{
assert(bbsHasDefault);
assert(bbsCount > 0);
return bbsDstTab[bbsCount - 1];
}
};
struct BBswtDesc;

struct StackEntry
{
Expand Down Expand Up @@ -1235,6 +1198,51 @@ typedef JitHashTable<BasicBlock*, JitPtrKeyFuncs<BasicBlock>, BlkVector> BlkToBl
// Map from Block to Block. Used for a variety of purposes.
typedef JitHashTable<BasicBlock*, JitPtrKeyFuncs<BasicBlock>, BasicBlock*> BlockToBlockMap;

// BBswtDesc -- descriptor for a switch block
//
// Things to know:
// 1. If bbsHasDefault is true, the default case is the last one in the array of basic block addresses
// namely bbsDstTab[bbsCount - 1].
// 2. bbsCount must be at least 1, for the default case. bbsCount cannot be zero. It appears that the ECMA spec
// allows for a degenerate switch with zero cases. Normally, the optimizer will optimize degenerate
// switches with just a default case to a BBJ_ALWAYS branch, and a switch with just two cases to a BBJ_COND.
// However, in debuggable code, we might not do that, so bbsCount might be 1.
//
struct BBswtDesc
{
BasicBlock** bbsDstTab; // case label table address
unsigned bbsCount; // count of cases (includes 'default' if bbsHasDefault)

// Case number and likelihood of most likely case
// (only known with PGO, only valid if bbsHasDominantCase is true)
unsigned bbsDominantCase;
BasicBlock::weight_t bbsDominantFraction;

bool bbsHasDefault; // true if last switch case is a default case
bool bbsHasDominantCase; // true if switch has a dominant case

BBswtDesc() : bbsHasDefault(true), bbsHasDominantCase(false)
{
}

BBswtDesc(Compiler* comp, const BBswtDesc* other);

void removeDefault()
{
assert(bbsHasDefault);
assert(bbsCount > 0);
bbsHasDefault = false;
bbsCount--;
}

BasicBlock* getDefault()
{
assert(bbsHasDefault);
assert(bbsCount > 0);
return bbsDstTab[bbsCount - 1];
}
};

// In compiler terminology the control flow between two BasicBlocks
// is typically referred to as an "edge". Most well known are the
// backward branches for loops, which are often called "back-edges".
Expand Down Expand Up @@ -1291,6 +1299,8 @@ struct BasicBlockList
}
};

// flowList -- control flow edge
//
struct flowList
{
public:
Expand Down
37 changes: 26 additions & 11 deletions src/coreclr/jit/fgdiagnostic.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1933,27 +1933,42 @@ void Compiler::fgTableDispBasicBlock(BasicBlock* block, int ibcColWidth /* = 0 *
break;

case BBJ_SWITCH:
{
printf("->");

unsigned jumpCnt;
jumpCnt = block->bbJumpSwt->bbsCount;
BasicBlock** jumpTab;
jumpTab = block->bbJumpSwt->bbsDstTab;
int switchWidth;
switchWidth = 0;
do
const BBswtDesc* const bbJumpSwt = block->bbJumpSwt;
const unsigned jumpCnt = bbJumpSwt->bbsCount;
BasicBlock** const jumpTab = bbJumpSwt->bbsDstTab;
int switchWidth = 0;

for (unsigned i = 0; i < jumpCnt; i++)
{
printf("%c" FMT_BB, (jumpTab == block->bbJumpSwt->bbsDstTab) ? ' ' : ',', (*jumpTab)->bbNum);
switchWidth += 1 /* space/comma */ + 2 /* BB */ + max(CountDigits((*jumpTab)->bbNum), 2);
} while (++jumpTab, --jumpCnt);
printf("%c" FMT_BB, (i == 0) ? ' ' : ',', jumpTab[i]->bbNum);
switchWidth += 1 /* space/comma */ + 2 /* BB */ + max(CountDigits(jumpTab[i]->bbNum), 2);

const bool isDefault = bbJumpSwt->bbsHasDefault && (i == jumpCnt - 1);
if (isDefault)
{
printf("[def]");
switchWidth += 5;
}

const bool isDominant = bbJumpSwt->bbsHasDominantCase && (i == bbJumpSwt->bbsDominantCase);
if (isDominant)
{
printf("[dom(" FMT_WT ")]", bbJumpSwt->bbsDominantFraction);
switchWidth += 10;
}
}

if (switchWidth < 7)
{
printf("%*s", 8 - switchWidth, "");
}

printf(" (switch)");
break;
}
break;
}
}

Expand Down
Loading

0 comments on commit 6ec2fba

Please sign in to comment.