tra added inline comments. ================ Comment at: include/clang/Driver/Action.h:79 @@ +78,3 @@ + OFFLOAD_None = 0x00, + OFFLOAD_CUDA = 0x01, + }; ---------------- 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_?
================ Comment at: include/clang/Driver/Compilation.h:42 @@ +41,3 @@ + /// The tool chain of the offload host. + const ToolChain *OffloadHostToolChain; + ---------------- This could be generalized into yet another toolchain kind and handled the same way as offload toolchains. ================ Comment at: include/clang/Driver/Compilation.h:46 @@ +45,3 @@ + /// the host has to support. + unsigned OffloadHostKinds; + ---------------- This could use a more descriptive name. ActiveOffloadMask? ================ Comment at: include/clang/Driver/Compilation.h:51 @@ -43,1 +50,3 @@ + typedef std::pair<const ToolChain *, Action::OffloadKind> OffloadToolChainTy; + SmallVector<OffloadToolChainTy, 4> OrderedOffloadingToolchains; ---------------- Using std::multimap<OffloadKind, Toolchain> here would probably make specific_offload_kind_iterator unnecessary and would simplify toolchain lookup. ================ Comment at: lib/Driver/Driver.cpp:406 @@ +405,3 @@ + // We need to generate a CUDA toolchain if any of the inputs has a CUDA type. + for (auto &I : Inputs) + // Have we founs a CUDA file? If so generate the toolchain. ---------------- llvm::any_of() could be useful here: ``` if (llvm::any_of(Inputs, [](auto &I) { return types::isCuda(I.first)})) { ...addOffloadDeviceToolchain(); } ``` ================ Comment at: lib/Driver/Driver.cpp:419 @@ +418,3 @@ + // + // Add support for other offloading programming models here. + // ---------------- TODO: ================ Comment at: lib/Driver/Driver.cpp:543 @@ -520,1 +542,3 @@ + // Get the toolchains for the offloading devices, if any. + CreateOffloadingDeviceToolChains(*C, Inputs); ---------------- Get is somewhat misplaced here. Perhaps populate or initialize would work better. http://reviews.llvm.org/D18170 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits