Page MenuHomePhabricator

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

Repository
rL LLVM

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 ↗(On Diff #201366)

instuments -> instrumented

756 ↗(On Diff #201366)

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

davidxl added inline comments.May 27 2019, 6:00 PM
llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp
755 ↗(On Diff #201366)

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 ↗(On Diff #201366)

Explain this change.

1078 ↗(On Diff #201366)

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 ↗(On Diff #201366)

changed to EdgeList.

756 ↗(On Diff #201366)

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 ↗(On Diff #201366)

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

1081 ↗(On Diff #201690)

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.