This is an archive of the discontinued LLVM Phabricator instance.

[PM] Port UnreachableBlockElim to the new Pass Manager
ClosedPublic

Authored by wmi on Jul 7 2016, 5:02 PM.

Diff Detail

Repository
rL LLVM

Event Timeline

wmi updated this revision to Diff 63169.Jul 7 2016, 5:02 PM
wmi retitled this revision from to [PM] Port UnreachableBlockElim to the new Pass Manager.
wmi updated this object.
wmi added reviewers: davidxl, silvas, davide.
wmi set the repository for this revision to rL LLVM.
wmi added a subscriber: llvm-commits.
davide accepted this revision.Jul 7 2016, 5:18 PM
davide edited edge metadata.

LGTM with nits.

lib/CodeGen/UnreachableBlockElim.cpp
44 ↗(On Diff #63169)

The body of runOnFunction() can probably be inlined here.

102 ↗(On Diff #63169)

AnalysisManager<Function>

102 ↗(On Diff #63169)

Maybe AnalysisManager<Function> ?

test/CodeGen/X86/unreachableblockelim.ll
1 ↗(On Diff #63169)

So this pass wasn't tested before?

This revision is now accepted and ready to land.Jul 7 2016, 5:18 PM
silvas edited edge metadata.Jul 7 2016, 5:26 PM

LGTM.

btw, I don't think we have a strong precedent for AnalysisManager<Function> vs FunctionAnalysisManager so either is fine.

silvas accepted this revision.Jul 7 2016, 5:26 PM
silvas edited edge metadata.

Marking explicitly as "accepted".

wmi added a comment.Jul 7 2016, 5:40 PM

Thanks for the review. Addressed davide's comments.

lib/CodeGen/UnreachableBlockElim.cpp
44 ↗(On Diff #63169)

Right, fixed.

102 ↗(On Diff #63169)

Sean says no preference so I leave it as it is.

test/CodeGen/X86/unreachableblockelim.ll
1 ↗(On Diff #63169)

It was tested but only by llc instead of opt. We cannot test the new pass using llc.

wmi updated this revision to Diff 63171.Jul 7 2016, 5:41 PM
wmi edited edge metadata.
This revision was automatically updated to reflect the committed changes.