[PATCH] D18170: [CUDA][OpenMP] Create generic offload toolchains

[Artem Belevich via cfe-commits]

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