Re: [PATCH] D18171: [CUDA][OpenMP] Create generic offload action

2016-07-13 Thread Artem Belevich via cfe-commits
tra accepted this revision. tra added a comment. This revision is now accepted and ready to land. LGTM. Comment at: test/Driver/cuda_phases.cu:48 @@ +47,3 @@ +// +// Test two gpu architecture with complete compilation. +// architecture*s*. There are few more cop

Re: [PATCH] D18171: [CUDA][OpenMP] Create generic offload action

2016-07-13 Thread Samuel Antao via cfe-commits
sfantao marked an inline comment as done. Comment at: test/Driver/cuda_phases.cu:2 @@ +1,3 @@ +// Tests the phases generated for a CUDA offloading target for different +// combinations of: +// - Number of gpu architectures; Oh, ok. Thought that the registration of

Re: [PATCH] D18171: [CUDA][OpenMP] Create generic offload action

2016-07-13 Thread Samuel Antao via cfe-commits
sfantao updated this revision to Diff 63889. sfantao added a comment. - Add comments and REQUIRE directives to test. http://reviews.llvm.org/D18171 Files: include/clang/Driver/Action.h include/clang/Driver/Compilation.h include/clang/Driver/Driver.h lib/Driver/Action.cpp lib/Driver/Dr

Re: [PATCH] D18171: [CUDA][OpenMP] Create generic offload action

2016-07-13 Thread Artem Belevich via cfe-commits
tra added inline comments. Comment at: test/Driver/cuda_phases.cu:1 @@ +1,2 @@ +// RUN: %clang -target powerpc64le-ibm-linux-gnu -ccc-print-phases --cuda-gpu-arch=sm_30 %s 2>&1 \ +// RUN: | FileCheck -check-prefix=BIN %s Few words describing the test would be nic

Re: [PATCH] D18171: [CUDA][OpenMP] Create generic offload action

2016-07-13 Thread Samuel Antao via cfe-commits
sfantao updated this revision to Diff 63880. sfantao added a comment. - Add test to check the generated phases for CUDA. - Fix typo in comment. http://reviews.llvm.org/D18171 Files: include/clang/Driver/Action.h include/clang/Driver/Compilation.h include/clang/Driver/Driver.h lib/Driver

Re: [PATCH] D18171: [CUDA][OpenMP] Create generic offload action

2016-07-13 Thread Artem Belevich via cfe-commits
tra added a comment. The changes look good. Now we just need some tests. Something along the lines of test/Driver/phases.c should do. http://reviews.llvm.org/D18171 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi

Re: [PATCH] D18171: [CUDA][OpenMP] Create generic offload action

2016-07-13 Thread Samuel Antao via cfe-commits
sfantao added inline comments. Comment at: lib/Driver/Action.cpp:191-202 @@ +190,14 @@ + auto I = getInputs().begin(); + auto E = getInputs().end(); + if (I == E) +return; + + // Skip host action + if (HostTC) +++I; + + auto TI = DevToolChains.begin(); + for (; I !=

Re: [PATCH] D18171: [CUDA][OpenMP] Create generic offload action

2016-07-13 Thread Samuel Antao via cfe-commits
sfantao updated this revision to Diff 63865. sfantao marked an inline comment as done. sfantao added a comment. - Modify assertion to check that sizes of input dependences and device toolchains is consistent. http://reviews.llvm.org/D18171 Files: include/clang/Driver/Action.h include/clang

Re: [PATCH] D18171: [CUDA][OpenMP] Create generic offload action

2016-07-13 Thread Artem Belevich via cfe-commits
tra added inline comments. Comment at: lib/Driver/Action.cpp:191-202 @@ +190,14 @@ +const OffloadActionWorkTy &Work) const { + auto I = getInputs().begin(); + auto E = getInputs().end(); + if (I == E) +return; + + // Skip host action + if (HostTC) +++I; + + auto

Re: [PATCH] D18171: [CUDA][OpenMP] Create generic offload action

2016-07-13 Thread Samuel Antao via cfe-commits
sfantao added a comment. Hi Art, Thanks for the review! Addressed your comments in the last diff. Thanks again, Samuel Comment at: lib/Driver/Action.cpp:191-202 @@ +190,14 @@ +const OffloadActionWorkTy &Work) const { + auto I = getInputs().begin(); + auto E = getInputs()

Re: [PATCH] D18171: [CUDA][OpenMP] Create generic offload action

2016-07-13 Thread Samuel Antao via cfe-commits
sfantao updated this revision to Diff 63852. sfantao added a comment. - Add one more assert. http://reviews.llvm.org/D18171 Files: include/clang/Driver/Action.h include/clang/Driver/Compilation.h include/clang/Driver/Driver.h lib/Driver/Action.cpp lib/Driver/Driver.cpp lib/Driver/To

Re: [PATCH] D18171: [CUDA][OpenMP] Create generic offload action

2016-07-13 Thread Samuel Antao via cfe-commits
sfantao updated this revision to Diff 63850. sfantao marked 4 inline comments as done. sfantao added a comment. - Add assertions before accessing getInputs() when assumptions are made about the number of elements. - Add missing evaluations of end iterator in the beginning of loops. - Address othe

Re: [PATCH] D18171: [CUDA][OpenMP] Create generic offload action

2016-07-12 Thread Artem Belevich via cfe-commits
tra added a comment. Few minor nits and suggestions. Other than that I'm OK with the patch. Comment at: lib/Driver/Action.cpp:156 @@ +155,3 @@ + // Propagate info to the dependencies. + for (unsigned i = 0; i < getInputs().size(); ++i) +getInputs()[i]->propagateDeviceOfflo

Re: [PATCH] D18171: [CUDA][OpenMP] Create generic offload action

2016-07-12 Thread Samuel Antao via cfe-commits
sfantao added a comment. @tra, any more comments about this patch? Thanks! Samuel http://reviews.llvm.org/D18171 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D18171: [CUDA][OpenMP] Create generic offload action

2016-07-12 Thread Samuel Antao via cfe-commits
sfantao updated this revision to Diff 63680. sfantao added a comment. - Rebase. - Remove static function no longer necessary. http://reviews.llvm.org/D18171 Files: include/clang/Driver/Action.h include/clang/Driver/Compilation.h include/clang/Driver/Driver.h lib/Driver/Action.cpp lib/

Re: [PATCH] D18171: [CUDA][OpenMP] Create generic offload action

2016-07-01 Thread Samuel Antao via cfe-commits
sfantao updated this revision to Diff 62572. sfantao added a comment. - Rebase http://reviews.llvm.org/D18171 Files: include/clang/Driver/Action.h include/clang/Driver/Compilation.h include/clang/Driver/Driver.h lib/Driver/Action.cpp lib/Driver/Driver.cpp lib/Driver/ToolChain.cpp

Re: [PATCH] D18171: [CUDA][OpenMP] Create generic offload action

2016-06-30 Thread Samuel Antao via cfe-commits
sfantao updated this revision to Diff 62460. sfantao added a comment. - Remove \brief. http://reviews.llvm.org/D18171 Files: include/clang/Driver/Action.h include/clang/Driver/Compilation.h include/clang/Driver/Driver.h lib/Driver/Action.cpp lib/Driver/Driver.cpp lib/Driver/ToolChai

Re: [PATCH] D18171: [CUDA][OpenMP] Create generic offload action

2016-06-30 Thread Samuel Antao via cfe-commits
sfantao added a comment. In http://reviews.llvm.org/D18171#471824, @sfantao wrote: > Hi Alexey, > > Thanks for the review! I addressed your comments in the last diff. > > In http://reviews.llvm.org/D18171#471044, @ABataev wrote: > > > No '\brief's > > > When you say "no \brief" do you mean I am

Re: [PATCH] D18171: [CUDA][OpenMP] Create generic offload action

2016-06-30 Thread Samuel Antao via cfe-commits
sfantao added a comment. Hi Alexey, Thanks for the review! I addressed your comments in the last diff. In http://reviews.llvm.org/D18171#471044, @ABataev wrote: > No '\brief's When you say "no \brief" do you mean I am using that where I shouldn't or the other way around. I am only adding th

Re: [PATCH] D18171: [CUDA][OpenMP] Create generic offload action

2016-06-30 Thread Samuel Antao via cfe-commits
sfantao updated this revision to Diff 62435. sfantao marked 5 inline comments as done. sfantao added a comment. - Mark classes with final and fix comments. http://reviews.llvm.org/D18171 Files: include/clang/Driver/Action.h include/clang/Driver/Compilation.h include/clang/Driver/Driver.h

Re: [PATCH] D18171: [CUDA][OpenMP] Create generic offload action

2016-06-29 Thread Alexey Bataev via cfe-commits
ABataev added a comment. No '\brief's Comment at: include/clang/Driver/Action.h:204 @@ -159,1 +203,3 @@ +/// kind to its dependences. +class OffloadAction : public Action { virtual void anchor(); 'final' Comment at: include/clang/Driver/Acti

Re: [PATCH] D18171: [CUDA][OpenMP] Create generic offload action

2016-06-29 Thread Samuel Antao via cfe-commits
sfantao added a comment. @tra, any other comments/suggestions about this patch? Thanks! Samuel http://reviews.llvm.org/D18171 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D18171: [CUDA][OpenMP] Create generic offload action

2016-06-29 Thread Samuel Antao via cfe-commits
sfantao updated this revision to Diff 62230. sfantao added a comment. - Better organize how the offload action is used and add more comments to document what is going on. - Rebase. http://reviews.llvm.org/D18171 Files: include/clang/Driver/Action.h include/clang/Driver/Compilation.h incl

Re: [PATCH] D18171: [CUDA][OpenMP] Create generic offload action

2016-06-13 Thread Samuel Antao via cfe-commits
sfantao added a comment. Any more comments on this one? Thanks! Samuel http://reviews.llvm.org/D18171 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D18171: [CUDA][OpenMP] Create generic offload action

2016-06-13 Thread Samuel Antao via cfe-commits
sfantao updated this revision to Diff 60573. sfantao added a comment. Herald added a subscriber: mehdi_amini. - Rebase. http://reviews.llvm.org/D18171 Files: include/clang/Driver/Action.h include/clang/Driver/Compilation.h include/clang/Driver/Driver.h lib/Driver/Action.cpp lib/Driver

Re: [PATCH] D18171: [CUDA][OpenMP] Create generic offload action

2016-04-06 Thread Samuel Antao via cfe-commits
sfantao added a comment. Hi Art, Justin, Thanks for the review and feedback! Tried to address your concerns. Let me know other suggestion you may have. Thanks again, Samuel Comment at: include/clang/Driver/Action.h:95 @@ +94,3 @@ + /// same host. Therefore, the host offloadi

Re: [PATCH] D18171: [CUDA][OpenMP] Create generic offload action

2016-04-06 Thread Samuel Antao via cfe-commits
sfantao updated this revision to Diff 52879. sfantao marked 15 inline comments as done. sfantao added a comment. Address Art, Justin and Eric comments. http://reviews.llvm.org/D18171 Files: include/clang/Driver/Action.h include/clang/Driver/Compilation.h include/clang/Driver/Driver.h li

Re: [PATCH] D18171: [CUDA][OpenMP] Create generic offload action

2016-03-23 Thread Justin Lebar via cfe-commits
jlebar added a comment. > @jlebar: Justin, any suggestions on what we can/should do regarding [const > correctness issues]? Using "mutable" to work around const-incorrectness should be a last resort. I've been frustrated by the const-incorrectness of Action as well, but if it's an issue here

Re: [PATCH] D18171: [CUDA][OpenMP] Create generic offload action

2016-03-23 Thread Artem Belevich via cfe-commits
tra added a comment. Thank you for making these changes. They don't solve all the shortcomings, but they improve things quite a bit, IMO. Overall I'm happy with the changes, though use of mutable and changing action state from const functions may need a look from someone with better C++-fu ski

[PATCH] D18171: [CUDA][OpenMP] Create generic offload action

2016-03-14 Thread Samuel Antao via cfe-commits
sfantao created this revision. sfantao added reviewers: ABataev, jlebar, tra, echristo, hfinkel. sfantao added subscribers: caomhin, carlo.bertolli, arpith-jacob, cfe-commits. This patch replaces the CUDA specific action by a generic offload action. The offload action may have multiple dependence