Re: [PATCH] D18172: [CUDA][OpenMP] Add a generic offload action builder

2016-09-28 Thread Hal Finkel via cfe-commits 
hfinkel accepted this revision.
hfinkel added a comment.
This revision is now accepted and ready to land.

A nice abstraction and cleanup. LGTM.



Comment at: lib/Driver/Driver.cpp:1625
@@ +1624,3 @@
+  // architecture. If we are in host-only mode we return 'success' so that
+  // the host use the CUDA offload kind.
+  if (auto *IA = dyn_cast(HostAction)) {

use -> uses


https://reviews.llvm.org/D18172



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D18172: [CUDA][OpenMP] Add a generic offload action builder

2016-09-23 Thread Alexey Bataev via cfe-commits 
ABataev added a comment.

LG for me.


https://reviews.llvm.org/D18172



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D18172: [CUDA][OpenMP] Add a generic offload action builder

2016-09-21 Thread Samuel Antao via cfe-commits 
sfantao updated this revision to Diff 72117.
sfantao added a comment.

- Rebase.


https://reviews.llvm.org/D18172

Files:
  include/clang/Driver/Compilation.h
  lib/Driver/Driver.cpp
  lib/Driver/Types.cpp
  test/Driver/cuda-bindings.cu
  test/Driver/cuda-phases.cu

Index: test/Driver/cuda-phases.cu
===
--- test/Driver/cuda-phases.cu
+++ test/Driver/cuda-phases.cu
@@ -13,194 +13,189 @@
 //
 // RUN: %clang -target powerpc64le-ibm-linux-gnu -ccc-print-phases --cuda-gpu-arch=sm_30 %s 2>&1 \
 // RUN: | FileCheck -check-prefix=BIN %s
-// BIN: 0: input, "{{.*}}cuda-phases.cu", cuda, (host-cuda)
-// BIN: 1: preprocessor, {0}, cuda-cpp-output, (host-cuda)
-// BIN: 2: compiler, {1}, ir, (host-cuda)
-// BIN: 3: input, "{{.*}}cuda-phases.cu", cuda, (device-cuda, sm_30)
-// BIN: 4: preprocessor, {3}, cuda-cpp-output, (device-cuda, sm_30)
-// BIN: 5: compiler, {4}, ir, (device-cuda, sm_30)
-// BIN: 6: backend, {5}, assembler, (device-cuda, sm_30)
-// BIN: 7: assembler, {6}, object, (device-cuda, sm_30)
-// BIN: 8: offload, "device-cuda (nvptx64-nvidia-cuda:sm_30)" {7}, object
-// BIN: 9: offload, "device-cuda (nvptx64-nvidia-cuda:sm_30)" {6}, assembler
-// BIN: 10: linker, {8, 9}, cuda-fatbin, (device-cuda)
-// BIN: 11: offload, "host-cuda (powerpc64le-ibm-linux-gnu)" {2}, "device-cuda (nvptx64-nvidia-cuda)" {10}, ir
-// BIN: 12: backend, {11}, assembler, (host-cuda)
-// BIN: 13: assembler, {12}, object, (host-cuda)
-// BIN: 14: linker, {13}, image, (host-cuda)
+// BIN-DAG: [[P0:[0-9]+]]: input, "{{.*}}cuda-phases.cu", cuda, (host-cuda)
+// BIN-DAG: [[P1:[0-9]+]]: preprocessor, {[[P0]]}, cuda-cpp-output, (host-cuda)
+// BIN-DAG: [[P2:[0-9]+]]: compiler, {[[P1]]}, ir, (host-cuda)
+// BIN-DAG: [[P3:[0-9]+]]: input, "{{.*}}cuda-phases.cu", cuda, (device-cuda, sm_30)
+// BIN-DAG: [[P4:[0-9]+]]: preprocessor, {[[P3]]}, cuda-cpp-output, (device-cuda, sm_30)
+// BIN-DAG: [[P5:[0-9]+]]: compiler, {[[P4]]}, ir, (device-cuda, sm_30)
+// BIN-DAG: [[P6:[0-9]+]]: backend, {[[P5]]}, assembler, (device-cuda, sm_30)
+// BIN-DAG: [[P7:[0-9]+]]: assembler, {[[P6]]}, object, (device-cuda, sm_30)
+// BIN-DAG: [[P8:[0-9]+]]: offload, "device-cuda (nvptx64-nvidia-cuda:sm_30)" {[[P7]]}, object
+// BIN-DAG: [[P9:[0-9]+]]: offload, "device-cuda (nvptx64-nvidia-cuda:sm_30)" {[[P6]]}, assembler
+// BIN-DAG: [[P10:[0-9]+]]: linker, {[[P8]], [[P9]]}, cuda-fatbin, (device-cuda)
+// BIN-DAG: [[P11:[0-9]+]]: offload, "host-cuda (powerpc64le-ibm-linux-gnu)" {[[P2]]}, "device-cuda (nvptx64-nvidia-cuda)" {[[P10]]}, ir
+// BIN-DAG: [[P12:[0-9]+]]: backend, {[[P11]]}, assembler, (host-cuda)
+// BIN-DAG: [[P13:[0-9]+]]: assembler, {[[P12]]}, object, (host-cuda)
+// BIN-DAG: [[P14:[0-9]+]]: linker, {[[P13]]}, image, (host-cuda)
 
 //
 // Test single gpu architecture up to the assemble phase.
 //
 // RUN: %clang -target powerpc64le-ibm-linux-gnu -ccc-print-phases --cuda-gpu-arch=sm_30 %s -S 2>&1 \
 // RUN: | FileCheck -check-prefix=ASM %s
-// ASM: 0: input, "{{.*}}cuda-phases.cu", cuda, (device-cuda, sm_30)
-// ASM: 1: preprocessor, {0}, cuda-cpp-output, (device-cuda, sm_30)
-// ASM: 2: compiler, {1}, ir, (device-cuda, sm_30)
-// ASM: 3: backend, {2}, assembler, (device-cuda, sm_30)
-// ASM: 4: offload, "device-cuda (nvptx64-nvidia-cuda:sm_30)" {3}, assembler
-// ASM: 5: input, "{{.*}}cuda-phases.cu", cuda, (host-cuda)
-// ASM: 6: preprocessor, {5}, cuda-cpp-output, (host-cuda)
-// ASM: 7: compiler, {6}, ir, (host-cuda)
-// ASM: 8: backend, {7}, assembler, (host-cuda)
+// ASM-DAG: [[P0:[0-9]+]]: input, "{{.*}}cuda-phases.cu", cuda, (device-cuda, sm_30)
+// ASM-DAG: [[P1:[0-9]+]]: preprocessor, {[[P0]]}, cuda-cpp-output, (device-cuda, sm_30)
+// ASM-DAG: [[P2:[0-9]+]]: compiler, {[[P1]]}, ir, (device-cuda, sm_30)
+// ASM-DAG: [[P3:[0-9]+]]: backend, {[[P2]]}, assembler, (device-cuda, sm_30)
+// ASM-DAG: [[P4:[0-9]+]]: offload, "device-cuda (nvptx64-nvidia-cuda:sm_30)" {[[P3]]}, assembler
+// ASM-DAG: [[P5:[0-9]+]]: input, "{{.*}}cuda-phases.cu", cuda, (host-cuda)
+// ASM-DAG: [[P6:[0-9]+]]: preprocessor, {[[P5]]}, cuda-cpp-output, (host-cuda)
+// ASM-DAG: [[P7:[0-9]+]]: compiler, {[[P6]]}, ir, (host-cuda)
+// ASM-DAG: [[P8:[0-9]+]]: backend, {[[P7]]}, assembler, (host-cuda)
 
 //
 // Test two gpu architectures with complete compilation.
 //
 // RUN: %clang -target powerpc64le-ibm-linux-gnu -ccc-print-phases --cuda-gpu-arch=sm_30 --cuda-gpu-arch=sm_35 %s 2>&1 \
 // RUN: | FileCheck -check-prefix=BIN2 %s
-// BIN2: 0: input, "{{.*}}cuda-phases.cu", cuda, (host-cuda)
-// BIN2: 1: preprocessor, {0}, cuda-cpp-output, (host-cuda)
-// BIN2: 2: compiler, {1}, ir, (host-cuda)
-// BIN2: 3: input, "{{.*}}cuda-phases.cu", cuda, (device-cuda, sm_30)
-// BIN2: 4: preprocessor, {3}, cuda-cpp-output, (device-cuda, sm_30)
-// BIN2: 5: compiler, {4}, ir, (device-cuda, sm_30)
-// BIN2: 6: backend, {5}, assembler, (device-cuda, sm_30)
-// BIN2: 7: assembler, {6}, object, (device-cuda, sm_30)
-// BIN2: 8: offload,

Re: [PATCH] D18172: [CUDA][OpenMP] Add a generic offload action builder

2016-08-05 Thread Jonas Hahnfeld via cfe-commits 
Hahnfeld added a subscriber: Hahnfeld.
Hahnfeld added a comment.

In https://reviews.llvm.org/D18172#500029, @sfantao wrote:

> Any more comments on this patch or depending ones?
>
> Thanks!
> Samuel


I can report that the latest patches are working for me and that they fix all 
points that I previously raised.
That said they look good from a user perspective but @jlebar and @ABataev have 
been driving the reviews so far.

Regards,
Jonas


https://reviews.llvm.org/D18172



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D18172: [CUDA][OpenMP] Add a generic offload action builder

2016-07-28 Thread Samuel Antao via cfe-commits 
sfantao added a comment.

Any more comments on this patch or depending ones?

Thanks!
Samuel


https://reviews.llvm.org/D18172



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D18172: [CUDA][OpenMP] Add a generic offload action builder

2016-07-28 Thread Samuel Antao via cfe-commits 
sfantao updated this revision to Diff 66016.
sfantao added a comment.

- Remove redundant phases from cuda-phases.cu and use DAG check.
- Rebase.


https://reviews.llvm.org/D18172

Files:
  include/clang/Driver/Compilation.h
  lib/Driver/Driver.cpp
  test/Driver/cuda-phases.cu

Index: test/Driver/cuda-phases.cu
===
--- test/Driver/cuda-phases.cu
+++ test/Driver/cuda-phases.cu
@@ -13,194 +13,189 @@
 //
 // RUN: %clang -target powerpc64le-ibm-linux-gnu -ccc-print-phases --cuda-gpu-arch=sm_30 %s 2>&1 \
 // RUN: | FileCheck -check-prefix=BIN %s
-// BIN: 0: input, "{{.*}}cuda-phases.cu", cuda, (host-cuda)
-// BIN: 1: preprocessor, {0}, cuda-cpp-output, (host-cuda)
-// BIN: 2: compiler, {1}, ir, (host-cuda)
-// BIN: 3: input, "{{.*}}cuda-phases.cu", cuda, (device-cuda, sm_30)
-// BIN: 4: preprocessor, {3}, cuda-cpp-output, (device-cuda, sm_30)
-// BIN: 5: compiler, {4}, ir, (device-cuda, sm_30)
-// BIN: 6: backend, {5}, assembler, (device-cuda, sm_30)
-// BIN: 7: assembler, {6}, object, (device-cuda, sm_30)
-// BIN: 8: offload, "device-cuda (nvptx64-nvidia-cuda:sm_30)" {7}, object
-// BIN: 9: offload, "device-cuda (nvptx64-nvidia-cuda:sm_30)" {6}, assembler
-// BIN: 10: linker, {8, 9}, cuda-fatbin, (device-cuda)
-// BIN: 11: offload, "host-cuda (powerpc64le-ibm-linux-gnu)" {2}, "device-cuda (nvptx64-nvidia-cuda)" {10}, ir
-// BIN: 12: backend, {11}, assembler, (host-cuda)
-// BIN: 13: assembler, {12}, object, (host-cuda)
-// BIN: 14: linker, {13}, image, (host-cuda)
+// BIN-DAG: [[P0:[0-9]+]]: input, "{{.*}}cuda-phases.cu", cuda, (host-cuda)
+// BIN-DAG: [[P1:[0-9]+]]: preprocessor, {[[P0]]}, cuda-cpp-output, (host-cuda)
+// BIN-DAG: [[P2:[0-9]+]]: compiler, {[[P1]]}, ir, (host-cuda)
+// BIN-DAG: [[P3:[0-9]+]]: input, "{{.*}}cuda-phases.cu", cuda, (device-cuda, sm_30)
+// BIN-DAG: [[P4:[0-9]+]]: preprocessor, {[[P3]]}, cuda-cpp-output, (device-cuda, sm_30)
+// BIN-DAG: [[P5:[0-9]+]]: compiler, {[[P4]]}, ir, (device-cuda, sm_30)
+// BIN-DAG: [[P6:[0-9]+]]: backend, {[[P5]]}, assembler, (device-cuda, sm_30)
+// BIN-DAG: [[P7:[0-9]+]]: assembler, {[[P6]]}, object, (device-cuda, sm_30)
+// BIN-DAG: [[P8:[0-9]+]]: offload, "device-cuda (nvptx64-nvidia-cuda:sm_30)" {[[P7]]}, object
+// BIN-DAG: [[P9:[0-9]+]]: offload, "device-cuda (nvptx64-nvidia-cuda:sm_30)" {[[P6]]}, assembler
+// BIN-DAG: [[P10:[0-9]+]]: linker, {[[P8]], [[P9]]}, cuda-fatbin, (device-cuda)
+// BIN-DAG: [[P11:[0-9]+]]: offload, "host-cuda (powerpc64le-ibm-linux-gnu)" {[[P2]]}, "device-cuda (nvptx64-nvidia-cuda)" {[[P10]]}, ir
+// BIN-DAG: [[P12:[0-9]+]]: backend, {[[P11]]}, assembler, (host-cuda)
+// BIN-DAG: [[P13:[0-9]+]]: assembler, {[[P12]]}, object, (host-cuda)
+// BIN-DAG: [[P14:[0-9]+]]: linker, {[[P13]]}, image, (host-cuda)
 
 //
 // Test single gpu architecture up to the assemble phase.
 //
 // RUN: %clang -target powerpc64le-ibm-linux-gnu -ccc-print-phases --cuda-gpu-arch=sm_30 %s -S 2>&1 \
 // RUN: | FileCheck -check-prefix=ASM %s
-// ASM: 0: input, "{{.*}}cuda-phases.cu", cuda, (device-cuda, sm_30)
-// ASM: 1: preprocessor, {0}, cuda-cpp-output, (device-cuda, sm_30)
-// ASM: 2: compiler, {1}, ir, (device-cuda, sm_30)
-// ASM: 3: backend, {2}, assembler, (device-cuda, sm_30)
-// ASM: 4: offload, "device-cuda (nvptx64-nvidia-cuda:sm_30)" {3}, assembler
-// ASM: 5: input, "{{.*}}cuda-phases.cu", cuda, (host-cuda)
-// ASM: 6: preprocessor, {5}, cuda-cpp-output, (host-cuda)
-// ASM: 7: compiler, {6}, ir, (host-cuda)
-// ASM: 8: backend, {7}, assembler, (host-cuda)
+// ASM-DAG: [[P0:[0-9]+]]: input, "{{.*}}cuda-phases.cu", cuda, (device-cuda, sm_30)
+// ASM-DAG: [[P1:[0-9]+]]: preprocessor, {[[P0]]}, cuda-cpp-output, (device-cuda, sm_30)
+// ASM-DAG: [[P2:[0-9]+]]: compiler, {[[P1]]}, ir, (device-cuda, sm_30)
+// ASM-DAG: [[P3:[0-9]+]]: backend, {[[P2]]}, assembler, (device-cuda, sm_30)
+// ASM-DAG: [[P4:[0-9]+]]: offload, "device-cuda (nvptx64-nvidia-cuda:sm_30)" {[[P3]]}, assembler
+// ASM-DAG: [[P5:[0-9]+]]: input, "{{.*}}cuda-phases.cu", cuda, (host-cuda)
+// ASM-DAG: [[P6:[0-9]+]]: preprocessor, {[[P5]]}, cuda-cpp-output, (host-cuda)
+// ASM-DAG: [[P7:[0-9]+]]: compiler, {[[P6]]}, ir, (host-cuda)
+// ASM-DAG: [[P8:[0-9]+]]: backend, {[[P7]]}, assembler, (host-cuda)
 
 //
 // Test two gpu architectures with complete compilation.
 //
 // RUN: %clang -target powerpc64le-ibm-linux-gnu -ccc-print-phases --cuda-gpu-arch=sm_30 --cuda-gpu-arch=sm_35 %s 2>&1 \
 // RUN: | FileCheck -check-prefix=BIN2 %s
-// BIN2: 0: input, "{{.*}}cuda-phases.cu", cuda, (host-cuda)
-// BIN2: 1: preprocessor, {0}, cuda-cpp-output, (host-cuda)
-// BIN2: 2: compiler, {1}, ir, (host-cuda)
-// BIN2: 3: input, "{{.*}}cuda-phases.cu", cuda, (device-cuda, sm_30)
-// BIN2: 4: preprocessor, {3}, cuda-cpp-output, (device-cuda, sm_30)
-// BIN2: 5: compiler, {4}, ir, (device-cuda, sm_30)
-// BIN2: 6: backend, {5}, assembler, (device-cuda, sm_30)
-// BIN2: 7: assembler, {6}, object, (device-cuda, sm_30)
-// BIN2: 8:

Re: [PATCH] D18172: [CUDA][OpenMP] Add a generic offload action builder

2016-07-12 Thread Samuel Antao via cfe-commits 
sfantao updated this revision to Diff 63681.
sfantao added a comment.

- Rebase.


http://reviews.llvm.org/D18172

Files:
  include/clang/Driver/Compilation.h
  lib/Driver/Driver.cpp

Index: lib/Driver/Driver.cpp
===
--- lib/Driver/Driver.cpp
+++ lib/Driver/Driver.cpp
@@ -1388,132 +1388,524 @@
   }
 }
 
-// For each unique --cuda-gpu-arch= argument creates a TY_CUDA_DEVICE
-// input action and then wraps each in CudaDeviceAction paired with
-// appropriate GPU arch name. In case of partial (i.e preprocessing
-// only) or device-only compilation, each device action is added to /p
-// Actions and /p Current is released. Otherwise the function creates
-// and returns a new CudaHostAction which wraps /p Current and device
-// side actions.
-static Action *buildCudaActions(Compilation , DerivedArgList ,
-const Arg *InputArg, Action *HostAction,
-ActionList ) {
-  Arg *PartialCompilationArg = Args.getLastArg(
-  options::OPT_cuda_host_only, options::OPT_cuda_device_only,
-  options::OPT_cuda_compile_host_device);
-  bool CompileHostOnly =
-  PartialCompilationArg &&
-  PartialCompilationArg->getOption().matches(options::OPT_cuda_host_only);
-  bool CompileDeviceOnly =
-  PartialCompilationArg &&
-  PartialCompilationArg->getOption().matches(options::OPT_cuda_device_only);
-
-  if (CompileHostOnly) {
+namespace {
+/// Provides a convenient interface for different programming models to generate
+/// the required device actions.
+class OffloadingActionBuilder final {
+  /// Flag used to trace errors in the builder.
+  bool IsValid = false;
+
+  /// The compilation that is using this builder.
+  Compilation 
+
+  /// The derived arguments associated with this builder.
+  DerivedArgList 
+
+  /// Map between an input argument and the offload kinds used to process it.
+  std::map InputArgToOffloadKindMap;
+
+  /// Builder interface. It doesn't build anything or keep any state.
+  class DeviceActionBuilder {
+  public:
+typedef llvm::SmallVector PhasesTy;
+
+enum ActionBuilderReturnCode {
+  // The builder acted successfully on the current action.
+  ABRT_Success,
+  // The builder didn't have to act on the current action.
+  ABRT_Inactive,
+  // The builder was successful and requested the host action to not be
+  // generated.
+  ABRT_Ignore_Host,
+};
+
+  protected:
+/// Compilation associated with this builder.
+Compilation 
+
+/// Tool chains associated with this builder. The same programming
+/// model may have associated one or more tool chains.
+SmallVector ToolChains;
+
+/// The derived arguments associated with this builder.
+DerivedArgList 
+
+/// The inputs associated with this builder.
+const Driver::InputList 
+
+/// The associated offload kind.
+Action::OffloadKind AssociatedOffloadKind = Action::OFK_None;
+
+  public:
+DeviceActionBuilder(Compilation , DerivedArgList ,
+const Driver::InputList ,
+Action::OffloadKind AssociatedOffloadKind)
+: C(C), Args(Args), Inputs(Inputs),
+  AssociatedOffloadKind(AssociatedOffloadKind) {}
+virtual ~DeviceActionBuilder() {}
+
+/// Fill up the array \a DA with all the device dependences that should be
+/// added to the provided host action \a HostAction. By default it is
+/// inactive.
+virtual ActionBuilderReturnCode
+getDeviceDepences(OffloadAction::DeviceDependences , phases::ID CurPhase,
+  phases::ID FinalPhase, PhasesTy ) {
+  return ABRT_Inactive;
+}
+
+/// Update the state to include the provided host action \a HostAction as a
+/// dependency of the current device action. By default it is inactive.
+virtual ActionBuilderReturnCode addDeviceDepences(Action *HostAction) {
+  return ABRT_Inactive;
+}
+
+/// Append top level actions generated by the builder. Return true if errors
+/// were found.
+virtual void appendTopLevelActions(ActionList ) {}
+
+/// Append linker actions generated by the builder. Return true if errors
+/// were found.
+virtual void appendLinkDependences(OffloadAction::DeviceDependences ) {}
+
+/// Initialize the builder. Return true if any initialization errors are
+/// found.
+virtual bool initialize() { return false; }
+
+/// Return true if this builder is valid. We have a valid builder if we have
+/// associated device tool chains.
+bool isValid() { return !ToolChains.empty(); }
+
+/// Return the associated offload kind.
+Action::OffloadKind getAssociatedOffloadKind() {
+  return AssociatedOffloadKind;
+}
+  };
+
+  /// \brief CUDA action builder. It injects device code in the host backend
+  /// action.
+  class CudaActionBuilder final : public DeviceActionBuilder {
+

Re: [PATCH] D18172: [CUDA][OpenMP] Add a generic offload action builder

2016-07-01 Thread Samuel Antao via cfe-commits 
sfantao updated this revision to Diff 62573.
sfantao added a comment.

- Rebase


http://reviews.llvm.org/D18172

Files:
  include/clang/Driver/Compilation.h
  lib/Driver/Driver.cpp

Index: lib/Driver/Driver.cpp
===
--- lib/Driver/Driver.cpp
+++ lib/Driver/Driver.cpp
@@ -1387,131 +1387,521 @@
   }
 }
 
-// For each unique --cuda-gpu-arch= argument creates a TY_CUDA_DEVICE
-// input action and then wraps each in CudaDeviceAction paired with
-// appropriate GPU arch name. In case of partial (i.e preprocessing
-// only) or device-only compilation, each device action is added to /p
-// Actions and /p Current is released. Otherwise the function creates
-// and returns a new CudaHostAction which wraps /p Current and device
-// side actions.
-static Action *buildCudaActions(Compilation , DerivedArgList ,
-const Arg *InputArg, Action *HostAction,
-ActionList ) {
-  Arg *PartialCompilationArg = Args.getLastArg(
-  options::OPT_cuda_host_only, options::OPT_cuda_device_only,
-  options::OPT_cuda_compile_host_device);
-  bool CompileHostOnly =
-  PartialCompilationArg &&
-  PartialCompilationArg->getOption().matches(options::OPT_cuda_host_only);
-  bool CompileDeviceOnly =
-  PartialCompilationArg &&
-  PartialCompilationArg->getOption().matches(options::OPT_cuda_device_only);
-
-  if (CompileHostOnly) {
+namespace {
+/// Provides a convenient interface for different programming models to generate
+/// the required device actions.
+class OffloadingActionBuilder final {
+  /// Flag used to trace errors in the builder.
+  bool IsValid = false;
+
+  /// The compilation that is using this builder.
+  Compilation 
+
+  /// The derived arguments associated with this builder.
+  DerivedArgList 
+
+  /// Map between an input argument and the offload kinds used to process it.
+  std::map InputArgToOffloadKindMap;
+
+  /// Builder interface. It doesn't build anything or keep any state.
+  class DeviceActionBuilder {
+  public:
+typedef llvm::SmallVector PhasesTy;
+
+enum ActionBuilderReturnCode {
+  // The builder acted successfully on the current action.
+  ABRT_Success,
+  // The builder didn't have to act on the current action.
+  ABRT_Inactive,
+  // The builder was successful and requested the host action to not be
+  // generated.
+  ABRT_Ignore_Host,
+};
+
+  protected:
+/// Compilation associated with this builder.
+Compilation 
+
+/// Tool chains associated with this builder. The same programming
+/// model may have associated one or more tool chains.
+SmallVector ToolChains;
+
+/// The derived arguments associated with this builder.
+DerivedArgList 
+
+/// The inputs associated with this builder.
+const Driver::InputList 
+
+/// The associated offload kind.
+Action::OffloadKind AssociatedOffloadKind = Action::OFK_None;
+
+  public:
+DeviceActionBuilder(Compilation , DerivedArgList ,
+const Driver::InputList ,
+Action::OffloadKind AssociatedOffloadKind)
+: C(C), Args(Args), Inputs(Inputs),
+  AssociatedOffloadKind(AssociatedOffloadKind) {}
+virtual ~DeviceActionBuilder() {}
+
+/// Fill up the array \a DA with all the device dependences that should be
+/// added to the provided host action \a HostAction. By default it is
+/// inactive.
+virtual ActionBuilderReturnCode
+getDeviceDepences(OffloadAction::DeviceDependences , phases::ID CurPhase,
+  phases::ID FinalPhase, PhasesTy ) {
+  return ABRT_Inactive;
+}
+
+/// Update the state to include the provided host action \a HostAction as a
+/// dependency of the current device action. By default it is inactive.
+virtual ActionBuilderReturnCode addDeviceDepences(Action *HostAction) {
+  return ABRT_Inactive;
+}
+
+/// Append top level actions generated by the builder. Return true if errors
+/// were found.
+virtual void appendTopLevelActions(ActionList ) {}
+
+/// Append linker actions generated by the builder. Return true if errors
+/// were found.
+virtual void appendLinkDependences(OffloadAction::DeviceDependences ) {}
+
+/// Initialize the builder. Return true if any initialization errors are
+/// found.
+virtual bool initialize() { return false; }
+
+/// Return true if this builder is valid. We have a valid builder if we have
+/// associated device tool chains.
+bool isValid() { return !ToolChains.empty(); }
+
+/// Return the associated offload kind.
+Action::OffloadKind getAssociatedOffloadKind() {
+  return AssociatedOffloadKind;
+}
+  };
+
+  /// \brief CUDA action builder. It injects device code in the host backend
+  /// action.
+  class CudaActionBuilder final : public DeviceActionBuilder {
+

Re: [PATCH] D18172: [CUDA][OpenMP] Add a generic offload action builder

2016-07-01 Thread Samuel Antao via cfe-commits 
sfantao added a comment.

In http://reviews.llvm.org/D18172#472152, @jlebar wrote:

> Yeah, I'd say that in the absence of a rule, consistency with surrounding
>  code is king.  Otherwise we're sending a message when we don't mean to be.
>
> I'm not at my machine, but my recollection is that most of the driver uses
>  final sparingly.  But whatever the convention is we should do that, I think.


True, `final` is not widely used in the driver. Again, I don't feel strongly 
about doing this one way or the other. For the moment, I'll update the diffs 
according to what Alexey requested. If you guys decide that I should do that 
differently, I'm happy to go to back to each review and update it accordingly.

Thanks again!
Samuel


http://reviews.llvm.org/D18172



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D18172: [CUDA][OpenMP] Add a generic offload action builder

2016-07-01 Thread Samuel Antao via cfe-commits 
sfantao updated this revision to Diff 62506.
sfantao added a comment.

- Use double instead of triple slash in one comment.


http://reviews.llvm.org/D18172

Files:
  include/clang/Driver/Compilation.h
  lib/Driver/Driver.cpp

Index: lib/Driver/Driver.cpp
===
--- lib/Driver/Driver.cpp
+++ lib/Driver/Driver.cpp
@@ -1387,131 +1387,521 @@
   }
 }
 
-// For each unique --cuda-gpu-arch= argument creates a TY_CUDA_DEVICE
-// input action and then wraps each in CudaDeviceAction paired with
-// appropriate GPU arch name. In case of partial (i.e preprocessing
-// only) or device-only compilation, each device action is added to /p
-// Actions and /p Current is released. Otherwise the function creates
-// and returns a new CudaHostAction which wraps /p Current and device
-// side actions.
-static Action *buildCudaActions(Compilation , DerivedArgList ,
-const Arg *InputArg, Action *HostAction,
-ActionList ) {
-  Arg *PartialCompilationArg = Args.getLastArg(
-  options::OPT_cuda_host_only, options::OPT_cuda_device_only,
-  options::OPT_cuda_compile_host_device);
-  bool CompileHostOnly =
-  PartialCompilationArg &&
-  PartialCompilationArg->getOption().matches(options::OPT_cuda_host_only);
-  bool CompileDeviceOnly =
-  PartialCompilationArg &&
-  PartialCompilationArg->getOption().matches(options::OPT_cuda_device_only);
-
-  if (CompileHostOnly) {
+namespace {
+/// Provides a convenient interface for different programming models to generate
+/// the required device actions.
+class OffloadingActionBuilder final {
+  /// Flag used to trace errors in the builder.
+  bool IsValid = false;
+
+  /// The compilation that is using this builder.
+  Compilation 
+
+  /// The derived arguments associated with this builder.
+  DerivedArgList 
+
+  /// Map between an input argument and the offload kinds used to process it.
+  std::map InputArgToOffloadKindMap;
+
+  /// Builder interface. It doesn't build anything or keep any state.
+  class DeviceActionBuilder {
+  public:
+typedef llvm::SmallVector PhasesTy;
+
+enum ActionBuilderReturnCode {
+  // The builder acted successfully on the current action.
+  ABRT_Success,
+  // The builder didn't have to act on the current action.
+  ABRT_Inactive,
+  // The builder was successful and requested the host action to not be
+  // generated.
+  ABRT_Ignore_Host,
+};
+
+  protected:
+/// Compilation associated with this builder.
+Compilation 
+
+/// Tool chains associated with this builder. The same programming
+/// model may have associated one or more tool chains.
+SmallVector ToolChains;
+
+/// The derived arguments associated with this builder.
+DerivedArgList 
+
+/// The inputs associated with this builder.
+const Driver::InputList 
+
+/// The associated offload kind.
+Action::OffloadKind AssociatedOffloadKind = Action::OFK_None;
+
+  public:
+DeviceActionBuilder(Compilation , DerivedArgList ,
+const Driver::InputList ,
+Action::OffloadKind AssociatedOffloadKind)
+: C(C), Args(Args), Inputs(Inputs),
+  AssociatedOffloadKind(AssociatedOffloadKind) {}
+virtual ~DeviceActionBuilder() {}
+
+/// Fill up the array \a DA with all the device dependences that should be
+/// added to the provided host action \a HostAction. By default it is
+/// inactive.
+virtual ActionBuilderReturnCode
+getDeviceDepences(OffloadAction::DeviceDependences , phases::ID CurPhase,
+  phases::ID FinalPhase, PhasesTy ) {
+  return ABRT_Inactive;
+}
+
+/// Update the state to include the provided host action \a HostAction as a
+/// dependency of the current device action. By default it is inactive.
+virtual ActionBuilderReturnCode addDeviceDepences(Action *HostAction) {
+  return ABRT_Inactive;
+}
+
+/// Append top level actions generated by the builder. Return true if errors
+/// were found.
+virtual void appendTopLevelActions(ActionList ) {}
+
+/// Append linker actions generated by the builder. Return true if errors
+/// were found.
+virtual void appendLinkDependences(OffloadAction::DeviceDependences ) {}
+
+/// Initialize the builder. Return true if any initialization errors are
+/// found.
+virtual bool initialize() { return false; }
+
+/// Return true if this builder is valid. We have a valid builder if we have
+/// associated device tool chains.
+bool isValid() { return !ToolChains.empty(); }
+
+/// Return the associated offload kind.
+Action::OffloadKind getAssociatedOffloadKind() {
+  return AssociatedOffloadKind;
+}
+  };
+
+  /// \brief CUDA action builder. It injects device code in the host backend
+  /// action.
+  class

Re: [PATCH] D18172: [CUDA][OpenMP] Add a generic offload action builder

2016-07-01 Thread Justin Lebar via cfe-commits 
jlebar added a subscriber: jlebar.
jlebar added a comment.

Yeah, I'd say that in the absence of a rule, consistency with surrounding
code is king.  Otherwise we're sending a message when we don't mean to be.

I'm not at my machine, but my recollection is that most of the driver uses
final sparingly.  But whatever the convention is we should do that, I think.


http://reviews.llvm.org/D18172



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D18172: [CUDA][OpenMP] Add a generic offload action builder

2016-07-01 Thread Justin Lebar via cfe-commits 
Yeah, I'd say that in the absence of a rule, consistency with surrounding
code is king.  Otherwise we're sending a message when we don't mean to be.

I'm not at my machine, but my recollection is that most of the driver uses
final sparingly.  But whatever the convention is we should do that, I think.
On Jun 30, 2016 9:16 PM, "Samuel Antao"  wrote:

> sfantao added a comment.
>
> In http://reviews.llvm.org/D18172#471861, @jlebar wrote:
>
> > Alexey, it seems that you're asking for "final" on all classes that are
> not inherited from.  Forgive my ignorance, but would you mind pointing me
> to the document that talks about our position on "final" in LLVM source?  I
> don't see it in the style guide, but I may be missing something.
> >
> > The style guide does talk a good bit about writing concise and generally
> not-misleading code.  My concern is that adding "final" everywhere paints
> an inaccurate picture and will mislead readers.  Specifically, "final" is
> useful as a signal to readers that a class cannot safely be inherited
> from.  "Don't even think about it, buster."  But here we're adding "final"
> to a lot of classes that, as far as I can tell, aren't distinctive except
> in that they have no subclasses today.  The problem with this is that, if
> we use "final" in this way, it dilutes the first "don't even try" meaning.
> Now when I see a class with "final" that I want to subclass, I'm just going
> to rip the "final" off, because chances are, I can do so safely.  Now
> "final" does not serve as a warning to me that I shouldn't do this.
> >
> > Sorry to focus on a superficial issue, but I think this really does
> matter for usability.
>
>
> Hi Justin,
>
> Thanks for the comment!
>
> I understand both interpretations can be useful for different purposes,
> i.e. "don't inherit, it is not safe" vs "feel free to change this class
> without worrying with subclasses". I tend to think that the latter (which I
> believe is what Alexey has in mind) can, in general, be more useful in the
> sense that a class is usually safe to extend - you can always add features
> that are somewhat orthogonal to what the parent class is doing. I can, of
> course, imagine cases where your argument is valid - you can have an
> implementation that allocates memory based on the type of the parent
> therefore it does not observe the extra storage required by derived
> classes. However, I think these cases are less common.
>
> I also look at the programming guidelines and couldn't find a clear answer
> to the issue you raised.
>
> I don't have a strong opinion about this, just giving my two cents.
>
> Thanks again!
> Samuel
>
>
> http://reviews.llvm.org/D18172
>
>
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D18172: [CUDA][OpenMP] Add a generic offload action builder

2016-06-30 Thread Samuel Antao via cfe-commits 
sfantao added a comment.

In http://reviews.llvm.org/D18172#471861, @jlebar wrote:

> Alexey, it seems that you're asking for "final" on all classes that are not 
> inherited from.  Forgive my ignorance, but would you mind pointing me to the 
> document that talks about our position on "final" in LLVM source?  I don't 
> see it in the style guide, but I may be missing something.
>
> The style guide does talk a good bit about writing concise and generally 
> not-misleading code.  My concern is that adding "final" everywhere paints an 
> inaccurate picture and will mislead readers.  Specifically, "final" is useful 
> as a signal to readers that a class cannot safely be inherited from.  "Don't 
> even think about it, buster."  But here we're adding "final" to a lot of 
> classes that, as far as I can tell, aren't distinctive except in that they 
> have no subclasses today.  The problem with this is that, if we use "final" 
> in this way, it dilutes the first "don't even try" meaning.  Now when I see a 
> class with "final" that I want to subclass, I'm just going to rip the "final" 
> off, because chances are, I can do so safely.  Now "final" does not serve as 
> a warning to me that I shouldn't do this.
>
> Sorry to focus on a superficial issue, but I think this really does matter 
> for usability.


Hi Justin,

Thanks for the comment!

I understand both interpretations can be useful for different purposes, i.e. 
"don't inherit, it is not safe" vs "feel free to change this class without 
worrying with subclasses". I tend to think that the latter (which I believe is 
what Alexey has in mind) can, in general, be more useful in the sense that a 
class is usually safe to extend - you can always add features that are somewhat 
orthogonal to what the parent class is doing. I can, of course, imagine cases 
where your argument is valid - you can have an implementation that allocates 
memory based on the type of the parent therefore it does not observe the extra 
storage required by derived classes. However, I think these cases are less 
common.

I also look at the programming guidelines and couldn't find a clear answer to 
the issue you raised.

I don't have a strong opinion about this, just giving my two cents.

Thanks again!
Samuel


http://reviews.llvm.org/D18172



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D18172: [CUDA][OpenMP] Add a generic offload action builder

2016-06-30 Thread Alexey Bataev via cfe-commits 
ABataev added a comment.

In http://reviews.llvm.org/D18172#471861, @jlebar wrote:

> Alexey, it seems that you're asking for "final" on all classes that are not 
> inherited from.  Forgive my ignorance, but would you mind pointing me to the 
> document that talks about our position on "final" in LLVM source?  I don't 
> see it in the style guide, but I may be missing something.
>
> The style guide does talk a good bit about writing concise and generally 
> not-misleading code.  My concern is that adding "final" everywhere paints an 
> inaccurate picture and will mislead readers.  Specifically, "final" is useful 
> as a signal to readers that a class cannot safely be inherited from.  "Don't 
> even think about it, buster."  But here we're adding "final" to a lot of 
> classes that, as far as I can tell, aren't distinctive except in that they 
> have no subclasses today.  The problem with this is that, if we use "final" 
> in this way, it dilutes the first "don't even try" meaning.  Now when I see a 
> class with "final" that I want to subclass, I'm just going to rip the "final" 
> off, because chances are, I can do so safely.  Now "final" does not serve as 
> a warning to me that I shouldn't do this.
>
> Sorry to focus on a superficial issue, but I think this really does matter 
> for usability.


Hi Justin,
There are no strict rules on using 'final'. But I believe that it is better to 
protect developers from unintended use of code and if language allows to do it 
we should do it. If some class is not intended to be used as a base class, it 
is better to mark it as 'final' to be sure that nobody will inherit from it for 
sure. Consider it as an additional contract of the class.


http://reviews.llvm.org/D18172



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D18172: [CUDA][OpenMP] Add a generic offload action builder

2016-06-30 Thread Justin Lebar via cfe-commits 
jlebar added a comment.

Alexey, it seems that you're asking for "final" on all classes that are not 
inherited from.  Forgive my ignorance, but would you mind pointing me to the 
document that talks about our position on "final" in LLVM source?  I don't see 
it in the style guide, but I may be missing something.

The style guide does talk a good bit about writing concise and generally 
not-misleading code.  My concern is that adding "final" everywhere paints an 
inaccurate picture and will mislead readers.  Specifically, "final" is useful 
as a signal to readers that a class cannot safely be inherited from.  "Don't 
even think about it, buster."  But here we're adding "final" to a lot of 
classes that, as far as I can tell, aren't distinctive except in that they have 
no subclasses today.  The problem with this is that, if we use "final" in this 
way, it dilutes the first "don't even try" meaning.  Now when I see a class 
with "final" that I want to subclass, I'm just going to rip the "final" off, 
because chances are, I can do so safely.  Now "final" does not serve as a 
warning to me that I shouldn't do this.

Sorry to focus on a superficial issue, but I think this really does matter for 
usability.


http://reviews.llvm.org/D18172



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D18172: [CUDA][OpenMP] Add a generic offload action builder

2016-06-30 Thread Samuel Antao via cfe-commits 
sfantao added a comment.

Hi Alexey,

Thanks for the review! Addressed your comments in the new diff.

I'll wait for your response on my question in http://reviews.llvm.org/D18172 to 
address the issue with \brief properly.

Thanks again,
Samuel


http://reviews.llvm.org/D18172



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D18172: [CUDA][OpenMP] Add a generic offload action builder

2016-06-30 Thread Samuel Antao via cfe-commits 
sfantao updated this revision to Diff 62438.
sfantao marked 3 inline comments as done.
sfantao added a comment.

- Fix comments, annotate classes with final, and add default initializers.


http://reviews.llvm.org/D18172

Files:
  include/clang/Driver/Compilation.h
  lib/Driver/Driver.cpp

Index: lib/Driver/Driver.cpp
===
--- lib/Driver/Driver.cpp
+++ lib/Driver/Driver.cpp
@@ -1387,131 +1387,523 @@
   }
 }
 
-// For each unique --cuda-gpu-arch= argument creates a TY_CUDA_DEVICE
-// input action and then wraps each in CudaDeviceAction paired with
-// appropriate GPU arch name. In case of partial (i.e preprocessing
-// only) or device-only compilation, each device action is added to /p
-// Actions and /p Current is released. Otherwise the function creates
-// and returns a new CudaHostAction which wraps /p Current and device
-// side actions.
-static Action *buildCudaActions(Compilation , DerivedArgList ,
-const Arg *InputArg, Action *HostAction,
-ActionList ) {
-  Arg *PartialCompilationArg = Args.getLastArg(
-  options::OPT_cuda_host_only, options::OPT_cuda_device_only,
-  options::OPT_cuda_compile_host_device);
-  bool CompileHostOnly =
-  PartialCompilationArg &&
-  PartialCompilationArg->getOption().matches(options::OPT_cuda_host_only);
-  bool CompileDeviceOnly =
-  PartialCompilationArg &&
-  PartialCompilationArg->getOption().matches(options::OPT_cuda_device_only);
-
-  if (CompileHostOnly) {
+namespace {
+/// \brief Provides a convenient interface for different programming models to
+/// generate the required device actions.
+class OffloadingActionBuilder final {
+  /// \brief Flag used to trace errors in the builder.
+  bool IsValid = false;
+
+  /// \brief The compilation that is using this builder.
+  Compilation 
+
+  /// \brief The derived arguments associated with this builder.
+  DerivedArgList 
+
+  /// \brief Map between an input argument and the offload kinds used to process
+  /// it.
+  std::map InputArgToOffloadKindMap;
+
+  /// \brief Builder interface. It doesn't build anything or keep any state.
+  class DeviceActionBuilder {
+  public:
+typedef llvm::SmallVector PhasesTy;
+
+enum ActionBuilderReturnCode {
+  // The builder acted successfully on the current action.
+  ABRT_Success,
+  // The builder didn't have to act on the current action.
+  ABRT_Inactive,
+  // The builder was successful and requested the host action to not be
+  // generated.
+  ABRT_Ignore_Host,
+};
+
+  protected:
+/// \brief Compilation associated with this builder.
+Compilation 
+
+/// \brief Tool chains associated with this builder. The same programming
+/// model may have associated one or more tool chains.
+SmallVector ToolChains;
+
+/// \brief The derived arguments associated with this builder.
+DerivedArgList 
+
+/// \brief The inputs associated with this builder.
+const Driver::InputList 
+
+/// \brief The associated offload kind.
+Action::OffloadKind AssociatedOffloadKind = Action::OFK_None;
+
+  public:
+DeviceActionBuilder(Compilation , DerivedArgList ,
+const Driver::InputList ,
+Action::OffloadKind AssociatedOffloadKind)
+: C(C), Args(Args), Inputs(Inputs),
+  AssociatedOffloadKind(AssociatedOffloadKind) {}
+virtual ~DeviceActionBuilder() {}
+
+// \brief Fill up the array \a DA with all the device dependences that
+// should be added to the provided host action \a HostAction. By default it
+// is inactive.
+virtual ActionBuilderReturnCode
+getDeviceDepences(OffloadAction::DeviceDependences , phases::ID CurPhase,
+  phases::ID FinalPhase, PhasesTy ) {
+  return ABRT_Inactive;
+}
+
+// \brief Update the state to include the provided host action \a HostAction
+// as a dependency of the current device action. By default it is inactive.
+virtual ActionBuilderReturnCode addDeviceDepences(Action *HostAction) {
+  return ABRT_Inactive;
+}
+
+// \brief Append top level actions generated by the builder. Return true if
+// errors were found.
+virtual void appendTopLevelActions(ActionList ) {}
+
+// \brief Append linker actions generated by the builder. Return true if
+// errors were found.
+virtual void appendLinkDependences(OffloadAction::DeviceDependences ) {}
+
+// \brief Initialize the builder. Return true if any initialization errors
+// are found.
+virtual bool initialize() { return false; }
+
+// \brief Return true if this builder is valid. We have a valid builder if
+// we have associated device tool chains.
+bool isValid() { return !ToolChains.empty(); }
+
+// \brief Return the associated offload kind.
+Action::OffloadKind

Re: [PATCH] D18172: [CUDA][OpenMP] Add a generic offload action builder

2016-06-29 Thread Alexey Bataev via cfe-commits 
ABataev added a comment.

No '\brief's



Comment at: lib/Driver/Driver.cpp:1393
@@ +1392,3 @@
+/// generate the required device actions.
+class OffloadingActionBuilder {
+  /// \brief Flag used to trace errors in the builder.

1. 'final'
2. default initializers for fields


Comment at: lib/Driver/Driver.cpp:1437
@@ +1436,3 @@
+/// \brief The associated offload kind.
+Action::OffloadKind AssociatedOffloadKind;
+

default initializer


Comment at: lib/Driver/Driver.cpp:1486
@@ +1485,3 @@
+  /// action.
+  class CudaActionBuilder : public DeviceActionBuilder {
+/// \brief Flags to signal if the user requested host-only or device-only

'final'


http://reviews.llvm.org/D18172



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D18172: [CUDA][OpenMP] Add a generic offload action builder

2016-06-29 Thread Samuel Antao via cfe-commits 
sfantao updated this revision to Diff 62231.
sfantao added a comment.

- Rebase.


http://reviews.llvm.org/D18172

Files:
  include/clang/Driver/Compilation.h
  lib/Driver/Driver.cpp

Index: lib/Driver/Driver.cpp
===
--- lib/Driver/Driver.cpp
+++ lib/Driver/Driver.cpp
@@ -1387,131 +1387,523 @@
   }
 }
 
-// For each unique --cuda-gpu-arch= argument creates a TY_CUDA_DEVICE
-// input action and then wraps each in CudaDeviceAction paired with
-// appropriate GPU arch name. In case of partial (i.e preprocessing
-// only) or device-only compilation, each device action is added to /p
-// Actions and /p Current is released. Otherwise the function creates
-// and returns a new CudaHostAction which wraps /p Current and device
-// side actions.
-static Action *buildCudaActions(Compilation , DerivedArgList ,
-const Arg *InputArg, Action *HostAction,
-ActionList ) {
-  Arg *PartialCompilationArg = Args.getLastArg(
-  options::OPT_cuda_host_only, options::OPT_cuda_device_only,
-  options::OPT_cuda_compile_host_device);
-  bool CompileHostOnly =
-  PartialCompilationArg &&
-  PartialCompilationArg->getOption().matches(options::OPT_cuda_host_only);
-  bool CompileDeviceOnly =
-  PartialCompilationArg &&
-  PartialCompilationArg->getOption().matches(options::OPT_cuda_device_only);
-
-  if (CompileHostOnly) {
-  OffloadAction::HostDependence HDep(
+namespace {
+/// \brief Provides a convenient interface for different programming models to
+/// generate the required device actions.
+class OffloadingActionBuilder {
+  /// \brief Flag used to trace errors in the builder.
+  bool IsValid;
+
+  /// \brief The compilation that is using this builder.
+  Compilation 
+
+  /// \brief The derived arguments associated with this builder.
+  DerivedArgList 
+
+  /// \brief Map between an input argument and the offload kinds used to process
+  /// it.
+  std::map InputArgToOffloadKindMap;
+
+  /// \brief Builder interface. It doesn't build anything or keep any state.
+  class DeviceActionBuilder {
+  public:
+typedef llvm::SmallVector PhasesTy;
+
+enum ActionBuilderReturnCode {
+  // The builder acted successfully on the current action.
+  ABRT_Success,
+  // The builder didn't have to act on the current action.
+  ABRT_Inactive,
+  // The builder was successful and requested the host action to not be
+  // generated.
+  ABRT_Ignore_Host,
+};
+
+  protected:
+/// \brief Compilation associated with this builder.
+Compilation 
+
+/// \brief Tool chains associated with this builder. The same programming
+/// model may have associated one or more tool chains.
+SmallVector ToolChains;
+
+/// \brief The derived arguments associated with this builder.
+DerivedArgList 
+
+/// \brief The inputs associated with this builder.
+const Driver::InputList 
+
+/// \brief The associated offload kind.
+Action::OffloadKind AssociatedOffloadKind;
+
+  public:
+DeviceActionBuilder(Compilation , DerivedArgList ,
+const Driver::InputList ,
+Action::OffloadKind AssociatedOffloadKind)
+: C(C), Args(Args), Inputs(Inputs),
+  AssociatedOffloadKind(AssociatedOffloadKind) {}
+virtual ~DeviceActionBuilder() {}
+
+// \brief Fill up the array \a DA with all the device dependences that
+// should be added to the provided host action \a HostAction. By default it
+// is inactive.
+virtual ActionBuilderReturnCode
+getDeviceDepences(OffloadAction::DeviceDependences , phases::ID CurPhase,
+  phases::ID FinalPhase, PhasesTy ) {
+  return ABRT_Inactive;
+}
+
+// \brief Update the state to include the provided host action \a HostAction
+// as a dependency of the current device action. By default it is inactive.
+virtual ActionBuilderReturnCode addDeviceDepences(Action *HostAction) {
+  return ABRT_Inactive;
+}
+
+// \brief Append top level actions generated by the builder. Return true if
+// errors were found.
+virtual void appendTopLevelActions(ActionList ) {}
+
+// \brief Append linker actions generated by the builder. Return true if
+// errors were found.
+virtual void appendLinkDependences(OffloadAction::DeviceDependences ) {}
+
+// \brief Initialize the builder. Return true if any initialization errors
+// are found.
+virtual bool initialize() { return false; }
+
+// \brief Return true if this builder is valid. We have a valid builder if
+// we have associated device tool chains.
+bool isValid() { return !ToolChains.empty(); }
+
+// \brief Return the associated offload kind.
+Action::OffloadKind getAssociatedOffloadKind() {
+  return AssociatedOffloadKind;
+}
+  };
+
+  /// \brief CUDA action

Re: [PATCH] D18172: [CUDA][OpenMP] Add a generic offload action builder

2016-04-06 Thread Samuel Antao via cfe-commits 
sfantao updated this revision to Diff 52881.
sfantao updated the summary for this revision.
sfantao added a comment.

Rebase.


http://reviews.llvm.org/D18172

Files:
  include/clang/Driver/Compilation.h
  lib/Driver/Driver.cpp

Index: lib/Driver/Driver.cpp
===
--- lib/Driver/Driver.cpp
+++ lib/Driver/Driver.cpp
@@ -1343,126 +1343,503 @@
   }
 }
 
-// For each unique --cuda-gpu-arch= argument creates a TY_CUDA_DEVICE
-// input action and then wraps each in CudaDeviceAction paired with
-// appropriate GPU arch name. In case of partial (i.e preprocessing
-// only) or device-only compilation, each device action is added to /p
-// Actions and /p Current is released. Otherwise the function creates
-// and returns a new CudaHostAction which wraps /p Current and device
-// side actions.
-static Action *buildCudaActions(Compilation , DerivedArgList ,
-const Arg *InputArg, Action *HostAction,
-ActionList ) {
-  Arg *PartialCompilationArg = Args.getLastArg(options::OPT_cuda_host_only,
-   options::OPT_cuda_device_only);
-  // Host-only compilation case.
-  if (PartialCompilationArg &&
-  PartialCompilationArg->getOption().matches(options::OPT_cuda_host_only)) {
+namespace {
+/// \brief Provides a convenient interface for different programming models to
+/// generate the required device actions.
+class OffloadingActionBuilder {
+  /// \brief Flag used to trace errors in the builder.
+  bool IsValid;
+
+  /// \brief The compilation that is using this builder.
+  Compilation 
+
+  /// \brief The derived arguments associated with this builder.
+  DerivedArgList 
+
+  /// \brief Map between an input argument and the offload kinds used to process
+  /// it.
+  std::map InputArgToOffloadKindMap;
+
+  /// \brief Builder interface. It doesn't build anything or keep any state.
+  class DeviceActionBuilder {
+  public:
+typedef llvm::SmallVector PhasesTy;
+
+enum ActionBuilderReturnCode {
+  // The builder acted successfully on the current action.
+  ABRT_Success,
+  // The builder didn't have to act on the current action.
+  ABRT_Inactive,
+  // The builder was successful and requested the host action to not be
+  // generated.
+  ABRT_Ignore_Host,
+};
+
+  protected:
+/// \brief Compilation associated with this builder.
+Compilation 
+
+/// \brief Tool chains associated with this builder. The same programming
+/// model may have associated one or more tool chains.
+SmallVector ToolChains;
+
+/// \brief The derived arguments associated with this builder.
+DerivedArgList 
+
+/// \brief The inputs associated with this builder.
+const Driver::InputList 
+
+/// \brief The associated offload kind.
+Action::OffloadKind AssociatedOffloadKind;
+
+  public:
+DeviceActionBuilder(Compilation , DerivedArgList ,
+const Driver::InputList ,
+Action::OffloadKind AssociatedOffloadKind)
+: C(C), Args(Args), Inputs(Inputs),
+  AssociatedOffloadKind(AssociatedOffloadKind) {}
+virtual ~DeviceActionBuilder() {}
+
+// \brief Fill up the array \a DA with all the device dependences that
+// should be added to the provided host action \a HostAction. By default it
+// is inactive.
+virtual ActionBuilderReturnCode
+getDeviceDepences(OffloadAction::DeviceDependences , Action *HostAction,
+  types::ID InputType, const Arg *InputArg,
+  phases::ID CurPhase, phases::ID FinalPhase,
+  PhasesTy ) {
+  return ABRT_Inactive;
+}
+
+// \brief Update the state to include the provided host action \a HostAction
+// as a dependency of the current device action. By default it is inactive.
+virtual ActionBuilderReturnCode addDeviceDepences(Action *HostAction,
+  types::ID InputType,
+  const Arg *InputArg) {
+  return ABRT_Inactive;
+}
+
+// \brief Append top level actions generated by the builder. Return true if
+// errors were found.
+virtual bool appendTopLevelActions(ActionList ) { return false; }
+
+// \brief Initialize the builder. Return true if any initialization errors
+// are found.
+virtual bool initialize() { return false; }
+
+// \brief Return true if this builder is valid. We have a valid builder if
+// we have associated device tool chains.
+bool isValid() { return !ToolChains.empty(); }
+
+// \brief Return the associated offload kind.
+Action::OffloadKind getAssociatedOffloadKind() {
+  return AssociatedOffloadKind;
+}
+  };
+
+  /// \brief CUDA action builder. It injects device code in the host backend
+  /// action.
+  class