[PATCH] D21853: [Driver][OpenMP] Update actions builder to create unbundling action when necessary.

[Hal Finkel via cfe-commits]

hfinkel added inline comments.

================
Comment at: include/clang/Driver/Action.h:504
@@ +503,3 @@
+  /// unbundling action.
+  struct DependingActionInfoTy final {
+    /// \brief The tool chain of the depending action.
----------------
Don't need 'Ty' in the name of this struct.

================
Comment at: include/clang/Driver/Types.h:81
@@ +80,3 @@
+  /// isSrcFile - Is this a source file, i.e. something that still has to be
+  /// preprocessed. The logic behind this is the same that decides the first
+  /// compilation phase is a preprocessor one.
----------------
decided the first -> decides if the first

================
Comment at: include/clang/Driver/Types.h:82
@@ +81,3 @@
+  /// preprocessed. The logic behind this is the same that decides the first
+  /// compilation phase is a preprocessor one.
+  bool isSrcFile(ID Id);
----------------
preprocessor one -> preprocessing one

================
Comment at: lib/Driver/Driver.cpp:2091
@@ +2090,3 @@
+        InputArg->getOption().getKind() == llvm::opt::Option::InputClass &&
+        !types::isSrcFile(HostAction->getType())) {
+      auto UnbundlingHostAction =
----------------
This checks that the file needs to be preprocessed. What does preprocessing have to do with this? I don't imagine that providing a preprocessed source file as input should invoke the unbundler   .

================
Comment at: test/Driver/openmp-offload.c:274
@@ +273,3 @@
+/// Check separate compilation with offloading - unbundling actions
+// RUN:   touch %t.i
+// RUN:   %clang -### -ccc-print-phases -fopenmp -o %t.out -lsomelib -target powerpc64le-linux -fopenmp-targets=powerpc64le-ibm-linux-gnu,x86_64-pc-linux-gnu %t.i 2>&1 \
----------------
Oh, are you using .i to indicate a bundle instead of a preprocessed file? Don't do that. Please use a different suffix -- the bundler has its own file format.


https://reviews.llvm.org/D21853