This is an archive of the discontinued LLVM Phabricator instance.

[Clang] Do not build the OffloadActionBuilder when using the new driver
ClosedPublic

Authored by jhuber6 on Oct 11 2022, 2:21 PM.

Details

Summary

The Offloading toolchain currently has two methods for construction the
requires actions. The "new" driver and the old OffloadActionBuilder.
Using either one is mutually exclusive, so we should not initialize this
when using the new driver. This was causing some error messages to be
printed multiple times because we were checking them in both the old and
the new driver.

Diff Detail

Event Timeline

jhuber6 created this revision.Oct 11 2022, 2:21 PM
Herald added a project: Restricted Project. · View Herald TranscriptOct 11 2022, 2:21 PM
jhuber6 requested review of this revision.Oct 11 2022, 2:21 PM
Herald added a project: Restricted Project. · View Herald TranscriptOct 11 2022, 2:21 PM
tra accepted this revision.Oct 11 2022, 2:33 PM
tra added inline comments.
clang/lib/Driver/Driver.cpp
3987–3988

Nit: Having to prefix every use of OffloadBuilder looks a bit cumbersome.

It would look a bit cleaner if we had some sort of no-op builder we'd populate OffloadBuilder with. Does not buy us anything functionally, though, so it may not be worth complicating things just for this. Up to you.

This revision is now accepted and ready to land.Oct 11 2022, 2:33 PM

Thanks for the quick review.

clang/lib/Driver/Driver.cpp
3987–3988

The main difficulty is that the OffloadBuilder requires the input to be checked at different points, so we wouldn't be able to merge anything. My hope one day is to delete the OffloadBuilder entirely, so I'm also not too concerned with making it look nice in the meantime.

This revision was landed with ongoing or failed builds.Oct 11 2022, 3:46 PM
This revision was automatically updated to reflect the committed changes.