Page MenuHomePhabricator

[PM] Introduce options to enable the (still experimental) new pass manager, and a code path to use it.
ClosedPublic

Authored by chandlerc on Dec 23 2016, 12:52 AM.

Details

Summary

The option is actually a top-level option but does contain
'experimental' in the name. This is the compromise suggested by Richard
in discussions. We expect this option will be around long enough and
have enough users towards the end that it merits not being relegated to
CC1, but it still needs to be clear that this option will go away at
some point.

The backend code is a fresh codepath dedicated to handling the flow with
the new pass manager. This was also Richard's suggested code structuring
to essentially leave a clean path for development rather than carrying
complexity or idiosyncracies of how we do things just to share code with
the parts of this in common with the legacy pass manager. And it turns
out, not much is really in common even though we use the legacy pass
manager for codegen at this point.

I've switched a couple of tests to run with the new pass manager, and
they appear to work. There are still plenty of bugs that need squashing
(just with basic experiments I've found two already!) but they aren't in
this code, and the whole point is to expose the necessary hooks to start
experimenting with the pass manager in more realistic scenarios.

That said, I want to *strongly caution* anyone itching to play with
this: it is still *very shaky*. Several large components have not yet
been shaken down. For example I have bugs in both the always inliner and
inliner that I have already spotted and will be fixing independently.

Still, this is a fun milestone. =D

One thing not in this patch (but that might be very reasonable to add)
is some level of support for raw textual pass pipelines such as what
Sean had a patch for some time ago. I'm mostly interested in the more
traditional flow of getting the IR out of Clang and then running it
through opt, but I can see other use cases so someone may want to add
it.

And of course, *many* features are not yet supported!

  • O1 is currently more like O2
  • None of the sanitizers are wired up
  • ObjC ARC optimizer isn't wired up
  • ...

So plenty of stuff still lef to do!

Depends on D28076.

Diff Detail

Repository
rL LLVM

Event Timeline

chandlerc updated this revision to Diff 82400.Dec 23 2016, 12:52 AM
chandlerc retitled this revision from to [PM] Introduce options to enable the (still experimental) new pass manager, and a code path to use it..
chandlerc updated this object.
chandlerc added reviewers: rsmith, mehdi_amini.
chandlerc added a subscriber: cfe-commits.
mehdi_amini accepted this revision.Dec 23 2016, 10:38 AM
mehdi_amini edited edge metadata.

LGTM! Exciting to see the new PM arriving in clang :)

lib/CodeGen/BackendUtil.cpp
757 ↗(On Diff #82400)

This is different from the legacy path, why?

762 ↗(On Diff #82400)

The legacy path accounts for the possibility of failing to create the TM

CreateTargetMachine(UsesCodeGen);

if (UsesCodeGen && !TM)
  return;
831 ↗(On Diff #82400)

No need to wrap in a block with a PrettyStackTraceString like the legacy PM?

test/Driver/clang_f_opts.c
475 ↗(On Diff #82400)

Not critical, but using the "CHECK" prefix isn't nice in a file intended to be shared for multiple tests.

This revision is now accepted and ready to land.Dec 23 2016, 10:38 AM
chandlerc marked 4 inline comments as done.Dec 23 2016, 12:50 PM

Thanks for the review, will land shortly with suggested fixes.

lib/CodeGen/BackendUtil.cpp
757 ↗(On Diff #82400)

Stale line. Meant to nuke it, we have it in the correct location below already.

762 ↗(On Diff #82400)

Good catch, done. I was too focused on the fact that TM was optional before.

831 ↗(On Diff #82400)

No, we should wrap it up similarly.

test/Driver/clang_f_opts.c
475 ↗(On Diff #82400)

Good point!

This revision was automatically updated to reflect the committed changes.
chandlerc marked 4 inline comments as done.