Skip to content

Commit f5b1885

Browse files
committed
Remove fixme about probability < 1:1 ratio
I added the fixme to this PR because I do not understand the importance of the associated prior comments from others. There has been no response to the fixme since I posted the PR seven months ago, so I assume it is not important going forward.
1 parent a1a5460 commit f5b1885

File tree

1 file changed

+3
-17
lines changed

1 file changed

+3
-17
lines changed

llvm/lib/Transforms/Utils/LoopPeel.cpp

Lines changed: 3 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1347,7 +1347,7 @@ bool llvm::peelLoop(Loop *L, unsigned PeelCount, bool PeelLast, LoopInfo *LI,
13471347

13481348
// Update metadata for the estimated trip count. The original branch weight
13491349
// metadata is already correct for both the remaining loop and the peeled loop
1350-
// iterations, so don't adjust it.
1350+
// iterations, so do not adjust it.
13511351
//
13521352
// For example, consider what happens when peeling 2 iterations from a loop
13531353
// with an estimated trip count of 10 and inserting them before the remaining
@@ -1363,23 +1363,9 @@ bool llvm::peelLoop(Loop *L, unsigned PeelCount, bool PeelLast, LoopInfo *LI,
13631363
// while examining it in isolation without considering the probability of
13641364
// actually reaching it, we store the new trip count as separate metadata.
13651365
if (auto EstimatedTripCount = getLoopEstimatedTripCount(L)) {
1366-
// FIXME: The previous updateBranchWeights implementation had this
1367-
// comment:
1368-
//
1369-
// Don't set the probability of taking the edge from latch to loop header
1370-
// to less than 1:1 ratio (meaning Weight should not be lower than
1371-
// SubWeight), as this could significantly reduce the loop's hotness,
1372-
// which would be incorrect in the case of underestimating the trip count.
1373-
//
1374-
// See e8d5db206c2f commit log for further discussion. That seems to
1375-
// suggest that we should avoid ever setting a trip count of < 2 here
1376-
// (equal chance of continuing and exiting means the loop will likely
1377-
// continue once and then exit once). Or is keeping the original branch
1378-
// weights already a sufficient improvement for whatever analysis cares
1379-
// about this case?
13801366
unsigned EstimatedTripCountNew = *EstimatedTripCount;
1381-
if (EstimatedTripCountNew < TotalPeeled) // FIXME: TotalPeeled + 2?
1382-
EstimatedTripCountNew = 0; // FIXME: = 2?
1367+
if (EstimatedTripCountNew < TotalPeeled)
1368+
EstimatedTripCountNew = 0;
13831369
else
13841370
EstimatedTripCountNew -= TotalPeeled;
13851371
setLoopEstimatedTripCount(L, EstimatedTripCountNew);

0 commit comments

Comments
 (0)