This is an archive of the discontinued LLVM Phabricator instance.

[OpenMP] Make the new offloading driver the default
ClosedPublic

Authored by jhuber6 on Mar 31 2022, 8:46 AM.

Details

Summary

Previously an opt-in flag -fopenmp-new-driver was used to enable the
new offloading driver. After passing tests for a few months it should be
sufficiently mature to flip the switch and make it the default. The new
offloading driver is now enabled if there is OpenMP and OpenMP
offloading present and the new -fno-openmp-new-driver is not present.

The new offloading driver has three main benefits over the old method:

  • Static library support
  • Device-side LTO
  • Unified clang driver stages

Depends on D122683

Diff Detail

Event Timeline

jhuber6 created this revision.Mar 31 2022, 8:46 AM
Herald added a project: Restricted Project. · View Herald TranscriptMar 31 2022, 8:46 AM
jhuber6 requested review of this revision.Mar 31 2022, 8:46 AM
Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptMar 31 2022, 8:46 AM

I feel we should port some of the clang driver tests rather than running them only for the old one.
The old one is on its way out, so ...

I feel we should port some of the clang driver tests rather than running them only for the old one.
The old one is on its way out, so ...

I tried to keep it where it worked, most of the ones changed were for checking things like nvlink specifically. One problem is the test that does -E which doesn't work right now because we won't generate offloading code without hitting a compile phase on the host. Will need to restructure some stuff to get that to work but I'm not sure how high the priority is when save-temps works fine.

Editing the tests in place to check for new driver properties would mean we lose coverage for the old driver and get a lot of churn if we need to back out the change.

How about taking some of the more interesting driver tests, duplicating them to only run the new driver, changing the content so it actually matches the new driver and making sure they make sense. We could / should do that before toggling the default.

Then when the old pathway is erased, we also erase the tests which only check the behaviour of the old path, and the transient test near-duplication is gone.

Same effect as changing the tests in place, just more resistant to reversion churn and has the nice property of continuing to check we haven't accidentally broken the old toolchain between now and when it is removed.

jhuber6 updated this revision to Diff 423416.Apr 18 2022, 9:59 AM

Splitting major changes into two files as per suggestion.

JonChesterfield accepted this revision.Apr 18 2022, 10:07 AM

LGTM, thanks! If it fails CI we might want to submit the tests and the code change separately, but I'm fine with trying it as one block in the first instance.

This revision is now accepted and ready to land.Apr 18 2022, 10:07 AM
This revision was landed with ongoing or failed builds.Apr 18 2022, 12:05 PM
This revision was automatically updated to reflect the committed changes.