Skip to content

JIT: don't allow negative edge weights #52884

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
May 18, 2021
Merged

JIT: don't allow negative edge weights #52884

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
91 changes: 60 additions & 31 deletions src/coreclr/jit/fgprofile.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2577,7 +2577,26 @@ bool flowList::setEdgeWeightMinChecked(BasicBlock::weight_t newWeight,
BasicBlock::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 <= flEdgeWeightMax) && (newWeight >= flEdgeWeightMin))
{
flEdgeWeightMin = newWeight;
Expand All @@ -2592,19 +2611,15 @@ bool flowList::setEdgeWeightMinChecked(BasicBlock::weight_t newWeight,
// is less than newWeight, so we just allow for the slop
if (newWeight <= (flEdgeWeightMax + slop))
{
result = true;
result = true;
usedSlop = true;

if (flEdgeWeightMax != BB_ZERO_WEIGHT)
{
// We will raise flEdgeWeightMin and Max towards newWeight
flEdgeWeightMin = flEdgeWeightMax;
flEdgeWeightMax = newWeight;
}

if (wbUsedSlop != nullptr)
{
*wbUsedSlop = true;
}
}
}
else if (flEdgeWeightMin > newWeight)
Expand All @@ -2613,33 +2628,34 @@ bool flowList::setEdgeWeightMinChecked(BasicBlock::weight_t newWeight,
// is more than newWeight, so we just allow for the slop
if ((newWeight + slop) >= flEdgeWeightMin)
{
result = true;
result = true;
usedSlop = true;

if (flEdgeWeightMax != BB_ZERO_WEIGHT)
{
// We will lower flEdgeWeightMin towards newWeight
flEdgeWeightMin = newWeight;
}

if (wbUsedSlop != nullptr)
{
*wbUsedSlop = true;
// But not below zero.
//
flEdgeWeightMin = 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.
// Also we should have set wbUsedSlop to true.
//
if (result)
{
assert((flEdgeWeightMax == BB_ZERO_WEIGHT) ||
((newWeight <= flEdgeWeightMax) && (newWeight >= flEdgeWeightMin)));

assert((wbUsedSlop == nullptr) || (*wbUsedSlop));
}
}

if (result && usedSlop && (wbUsedSlop != nullptr))
{
*wbUsedSlop = true;
}

#if DEBUG
if (result)
{
Expand Down Expand Up @@ -2677,7 +2693,26 @@ bool flowList::setEdgeWeightMaxChecked(BasicBlock::weight_t newWeight,
BasicBlock::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 >= flEdgeWeightMin) && (newWeight <= flEdgeWeightMax))
{
flEdgeWeightMax = newWeight;
Expand All @@ -2692,18 +2727,14 @@ bool flowList::setEdgeWeightMaxChecked(BasicBlock::weight_t newWeight,
// is less than newWeight, so we just allow for the slop
if (newWeight <= (flEdgeWeightMax + slop))
{
result = true;
result = true;
usedSlop = true;

if (flEdgeWeightMax != BB_ZERO_WEIGHT)
{
// We will allow this to raise flEdgeWeightMax towards newWeight
flEdgeWeightMax = newWeight;
}

if (wbUsedSlop != nullptr)
{
*wbUsedSlop = true;
}
}
}
else if (flEdgeWeightMin > newWeight)
Expand All @@ -2712,34 +2743,32 @@ bool flowList::setEdgeWeightMaxChecked(BasicBlock::weight_t newWeight,
// is more than newWeight, so we just allow for the slop
if ((newWeight + slop) >= flEdgeWeightMin)
{
result = true;
result = true;
usedSlop = true;

if (flEdgeWeightMax != BB_ZERO_WEIGHT)
{
// We will allow this to lower flEdgeWeightMin and Max towards newWeight
flEdgeWeightMax = flEdgeWeightMin;
flEdgeWeightMin = newWeight;
}

if (wbUsedSlop != nullptr)
{
*wbUsedSlop = true;
}
}
}

// 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
// Also we should have set wbUsedSlop to true, unless it is NULL
if (result)
{
assert((flEdgeWeightMax == BB_ZERO_WEIGHT) ||
((newWeight <= flEdgeWeightMax) && (newWeight >= flEdgeWeightMin)));

assert((wbUsedSlop == nullptr) || (*wbUsedSlop));
}
}

if (result && usedSlop && (wbUsedSlop != nullptr))
{
*wbUsedSlop = true;
}

#if DEBUG
if (result)
{
Expand Down