This is an archive of the discontinued LLVM Phabricator instance.

[PGO] Early detection regarding whether pgo counter promotion is possible
ClosedPublic

Authored by chris.chrulski on Jan 22 2020, 12:55 PM.

Details

Summary

This fixes a problem with the current behavior when assertions are enabled.
A loop that exits to a catchswitch instruction is skipped for the counter
promotion, however this check was being done after the PGOCounterPromoter
tried to collect an insertion point for the exit block. A call to
getFirstInsertionPt() on a block that begins with a catchswitch instruction
triggers an assertion. This change performs a check whether the counter
promotion is possible prior to collecting the ExitBlocks and InsertPts.

Diff Detail

Event Timeline

Herald added a project: Restricted Project. · View Herald TranscriptJan 22 2020, 12:55 PM
Herald added a subscriber: hiraditya. · View Herald Transcript
davidxl added inline comments.Jan 22 2020, 1:25 PM
llvm/lib/Transforms/Instrumentation/InstrProfiling.cpp
357

Why this change (changing ExitingBlocks to ExitBlocks)?

chris.chrulski marked an inline comment as done.Jan 22 2020, 1:49 PM
chris.chrulski added inline comments.
llvm/lib/Transforms/Instrumentation/InstrProfiling.cpp
357

Sorry, that was a mistake on my part. Thanks for catching that. I misread that one was setting 'exiting' vs' exit' in the call. will update patch.

Fix for bug in initial revision.

davidxl added inline comments.Jan 22 2020, 2:30 PM
llvm/lib/Transforms/Instrumentation/InstrProfiling.cpp
328

You can pass LoopExitBlocks reference as a parameter to this function so that the call to getExitBlocks can be shared with the caller of it.

chris.chrulski marked an inline comment as done.Jan 22 2020, 3:04 PM
chris.chrulski added inline comments.
llvm/lib/Transforms/Instrumentation/InstrProfiling.cpp
328

ok. will do.

Updated interface for isPromotionPossible routine to take in LoopExitBlocks.

davidxl accepted this revision.Jan 22 2020, 3:09 PM

lgtm

This revision is now accepted and ready to land.Jan 22 2020, 3:09 PM
This revision was automatically updated to reflect the committed changes.