This is an archive of the discontinued LLVM Phabricator instance.

[SimplifyCFG] Fix prof branch_weights MD while removing unreachable switch cases
ClosedPublic

Authored by yrouban on May 21 2019, 4:00 AM.

Details

Summary

SimplifyCFG has a bug that results in inconsistent prof branch_weights metadata if unreachable switch cases are removed.
This patch fixes this bug by making use of the newly introduced SwitchInstProfBranchWeightsWrapper class (see D62122).
A new test is created.

Diff Detail

Repository
rL LLVM

Event Timeline

yrouban created this revision.May 21 2019, 4:00 AM
Herald added a project: Restricted Project. · View Herald TranscriptMay 21 2019, 4:00 AM
Herald added a subscriber: hiraditya. · View Herald Transcript
reames accepted this revision.Jun 12 2019, 3:27 PM

LGTM w/two required changes.

The second is that the updater itself should check for the right number of metadata nodes and do nothing if already corrupt (as mentioned in another review thread).

llvm/lib/Transforms/Utils/SimplifyCFG.cpp
4217 ↗(On Diff #200455)

Naming here is non-idiomatic. Please leave SI for the SwitchInst and use SU or something for the updater class.

4219 ↗(On Diff #200455)

Not related to your change, but there's no reason a switch can't have several cases leading to the same unreachable block. A separate change to handle that would be nice if you wanted.

This revision is now accepted and ready to land.Jun 12 2019, 3:27 PM
This revision was automatically updated to reflect the committed changes.