This is an archive of the discontinued LLVM Phabricator instance.

[NFC] OptPassGate extracted from OptBisect
ClosedPublic

Authored by yrouban on Mar 23 2018, 2:44 AM.

Details

Summary

This is an NFC refactoring of the OptBisect class to split it into an optional pass gate interface used by LLVMContext and the Optional Pass Bisector (OptBisect) used for debugging of optional passes.

This refactoring is needed for D44464, which introduces setOptPassGate() method to allow implementations other than OptBisect.

Diff Detail

Repository
rL LLVM

Event Timeline

yrouban created this revision.Mar 23 2018, 2:44 AM
fedor.sergeev added inline comments.Mar 23 2018, 9:51 AM
include/llvm/IR/OptBisect.h
19–21 ↗(On Diff #139566)

Hmm.. can we avoid adding these headers and add forward declarations of all the necessary classes, please?
(the problem of layering has been just recently discussed on llvm-dev@)

include/llvm/IR/OptBisect.h
19–21 ↗(On Diff #139566)

Forward declaration doesn't really fix the layering problem. We really need to not use these classes.

The trouble is that at least in the OptBisect case we don't really want to build the description unless we're doing OptBisect, but we can't build the description for CallGraphSCC without having the layering problem.

fedor.sergeev added inline comments.Mar 23 2018, 12:01 PM
include/llvm/IR/OptBisect.h
19–21 ↗(On Diff #139566)

Okey, layering aside, forward declarations instead of Analysis headers would still be preferable here.

yrouban updated this revision to Diff 139765.Mar 26 2018, 12:18 AM
yrouban marked an inline comment as done.

changed 3 includes to forward declarations

fedor.sergeev accepted this revision.Mar 26 2018, 7:19 AM

LGTM. Though, please, wait for Andrew's feedback.

This revision is now accepted and ready to land.Mar 26 2018, 7:19 AM

I'm OK with this.

Thank you Andy. Do you mind looking at D44464 then?

LGTM. Though, please, wait for Andrew's feedback.

Thank you Fedor. Can you land the patch?

This revision was automatically updated to reflect the committed changes.