This is an archive of the discontinued LLVM Phabricator instance.

[CUDA][OpenMP] Create generic offload toolchains
ClosedPublic

Authored by sfantao on Mar 14 2016, 6:26 PM.

Details

Summary

This patch introduces the concept of offloading tool chain and offloading kind. Each tool chain may have associated an offloading kind that marks it as used in a given programming model that requires offloading.

It also adds the logic to iterate on the tool chains based on the kind. Currently, only CUDA is supported, but in general a programming model (an offloading kind) may have associated multiple tool chains that require supporting offloading.

This patch does not add tests - its goal is to keep the existing functionality.

This patch is the first of a series of three that attempts to make the current support of CUDA more generic and easier to extend to other programming models, namely OpenMP. It tries to capture the suggestions/improvements/concerns on the initial proposal in http://lists.llvm.org/pipermail/cfe-dev/2016-February/047547.html. It only tackles the more consensual part of the proposal, i.e.does not address the problem of intermediate files bundling yet.

Diff Detail

Event Timeline

sfantao updated this revision to Diff 50688.Mar 14 2016, 6:26 PM
sfantao retitled this revision from to [CUDA][OpenMP] Create generic offload toolchains.
sfantao updated this object.
sfantao added reviewers: ABataev, jlebar, tra, echristo, hfinkel.
mkuron added a subscriber: mkuron.Mar 19 2016, 1:42 AM
tra added inline comments.Mar 22 2016, 1:45 PM
include/clang/Driver/Action.h
79

Nit: All-caps CUDA looks weird here. _Cuda may be better choice.
If you can shorten the prefix that would be nice, too. OK_ is already taken, though. OFK_?

include/clang/Driver/Compilation.h
42

This could be generalized into yet another toolchain kind and handled the same way as offload toolchains.

46

This could use a more descriptive name. ActiveOffloadMask?

51

Using std::multimap<OffloadKind, Toolchain> here would probably make specific_offload_kind_iterator unnecessary and would simplify toolchain lookup.

lib/Driver/Driver.cpp
406

llvm::any_of() could be useful here:

if (llvm::any_of(Inputs, [](auto &I) { return types::isCuda(I.first)})) {
  ...addOffloadDeviceToolchain();
}
419

TODO:

543

Get is somewhat misplaced here. Perhaps populate or initialize would work better.

sfantao updated this revision to Diff 52878.Apr 6 2016, 6:38 PM
sfantao marked 6 inline comments as done.

Address Art and Eric comments.

Hi Art,

Thanks for the the review!

include/clang/Driver/Action.h
79

Ok, sure. I'm using OFK_ instead of OFFLOAD_ as suggested.

include/clang/Driver/Compilation.h
42

Ok, I created the OFK_Host kind to designate the host toolchain.

46

Using ActiveOffloadMask now.

51

Ok, that makes sense. I was trying to avoid the clients of this hook to iterate over a pair - something that a map of vectors would probably do without the need of the iterator.

In any case, I did as you say as it is a more clean solution.

lib/Driver/Driver.cpp
406

Ok, using any_of now.

419

Done!

543

I am using populate now.

Is there any progress on this one?

@tra any more comments on this?

tra accepted this revision.Jun 10 2016, 9:52 AM
tra edited edge metadata.

LGTM.

This revision is now accepted and ready to land.Jun 10 2016, 9:52 AM
sfantao closed this revision.Jun 13 2016, 11:17 AM