This is an archive of the discontinued LLVM Phabricator instance.

PGO] Handle cases of failing to split critical edges
ClosedPublic

Authored by xur on May 24 2019, 5:11 PM.

Details

Summary

Fix PR41279 where critical edges to EHPad are not split.

The fix is to not instrument those critical edge. We used to be able to know the size of counters right after MST is computed. With this, we have to pre-collect the instrument BBs to know the size, and then instrument them.

Diff Detail

Event Timeline

xur created this revision.May 24 2019, 5:11 PM
Herald added a project: Restricted Project. · View Herald TranscriptMay 24 2019, 5:11 PM
Herald added a subscriber: hiraditya. · View Herald Transcript
grandinj added inline comments.
llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp
750

instuments -> instrumented

756

WorkList.reserve(MST.AllEdges.size()) ?

davidxl added inline comments.May 27 2019, 6:00 PM
llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp
755

Nit. The name 'WorkList' implies it will be updated on the fly -- but here it is the opposite. How about just name it 'EdgeList'

1075

Explain this change.

1078

Same here.

xur marked 7 inline comments as done.May 28 2019, 8:59 AM
xur added inline comments.
llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp
755

changed to EdgeList.

756

Good idea. Done

xur updated this revision to Diff 201690.May 28 2019, 9:11 AM
xur marked 2 inline comments as done.

Integrated comments from Noel and David.

davidxl added inline comments.May 28 2019, 10:07 AM
llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp
1075

nit: edge count --> edge profile count (to be not confused with number of edges).

1079

edge count --> edge profile count; BB count --> BB profile count.

xur marked 2 inline comments as done.May 28 2019, 10:46 AM
xur updated this revision to Diff 201706.May 28 2019, 10:48 AM

Fix comments suggested by David.

This revision is now accepted and ready to land.May 28 2019, 11:07 AM
This revision was automatically updated to reflect the committed changes.