Re: [PATCH] D11361: [OpenMP] Target directive host codegen

sfantao abandoned this revision. sfantao added a comment. Closing revision. It has been replaced by http://reviews.llvm.org/D12871 has suggested by John. Thanks! Samuel http://reviews.llvm.org/D11361 ___ cfe-commits mailing list

Re: [PATCH] D11361: [OpenMP] Target directive host codegen

rjmccall added a comment. Sorry for putting off the final review on this; I was heads-down trying to get the alignment patch done. It's looking good; obviously you'll need to update it to work with Addresses properly, but hopefully that won't be too much of a problem. When you do, maybe you

Re: [PATCH] D11361: [OpenMP] Target directive host codegen

Hahnfeld added a comment. Needs two small changes to work with current trunk Comment at: lib/CodeGen/CGStmtOpenMP.cpp:2135-2136 @@ +2134,4 @@ + const Expr *IfCond = nullptr; + if (auto C = S.getSingleClause(OMPC_if)) { +IfCond = cast(C)->getCondition(); + }

Re: [PATCH] D11361: [OpenMP] Target directive host codegen

ABataev added a comment. Seems good to me, but it would be good if John McCall could look at the patch. http://reviews.llvm.org/D11361 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

Re: [PATCH] D11361: [OpenMP] Target directive host codegen

sfantao updated this revision to Diff 33640. sfantao added a comment. Address last review comments. http://reviews.llvm.org/D11361 Files: lib/CodeGen/CGOpenMPRuntime.cpp lib/CodeGen/CGOpenMPRuntime.h lib/CodeGen/CGStmtOpenMP.cpp test/OpenMP/target_codegen.cpp Index:

Re: [PATCH] D11361: [OpenMP] Target directive host codegen

sfantao added inline comments. Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:2991-3005 @@ +2990,17 @@ + +/// \brief Values for bit flags used to specify the mapping type for +/// offloading. +enum OpenMPOffloadMappingFlags { + /// \brief Allocate memory on the device and move data

Re: [PATCH] D11361: [OpenMP] Target directive host codegen

sfantao updated this revision to Diff 33111. sfantao added a comment. Move map type and device id enums from CGOpenMPRuntime.h to CGOpenMPRuntime.cpp. http://reviews.llvm.org/D11361 Files: lib/CodeGen/CGOpenMPRuntime.cpp lib/CodeGen/CGOpenMPRuntime.h lib/CodeGen/CGStmtOpenMP.cpp

Re: [PATCH] D11361: [OpenMP] Target directive host codegen

sfantao added a comment. Thanks for the review! In http://reviews.llvm.org/D11361#232045, @ABataev wrote: Samuel, Yes, I thought about different files and different classes. Runtime for offloading codegen is not a part of libomp and it would be good to have separate runtime handler class

Re: [PATCH] D11361: [OpenMP] Target directive host codegen

sfantao added a comment. Two more inlined comments that I forgot to integrate in my previous response. Thanks! Samuel Comment at: lib/CodeGen/CGOpenMPRuntime.h:190-204 @@ -180,2 +189,17 @@ + /// \brief Values for bit flags used to specify the mapping type for + ///

Re: [PATCH] D11361: [OpenMP] Target directive host codegen

sfantao updated this revision to Diff 32843. sfantao added a comment. Address reviewer concerns. http://reviews.llvm.org/D11361 Files: lib/CodeGen/CGOpenMPRuntime.cpp lib/CodeGen/CGOpenMPRuntime.h lib/CodeGen/CGStmtOpenMP.cpp test/OpenMP/target_codegen.cpp Index:

Re: [PATCH] D11361: [OpenMP] Target directive host codegen

sfantao added a comment. In http://reviews.llvm.org/D11361#229744, @ABataev wrote: Another one thing I forget to mention. Current implementation of CGOpenMPRuntime is libomp-specific. You're trying to add functionality that is libtarget-specific. Maybe it is a good idea to separate support

Re: [PATCH] D11361: [OpenMP] Target directive host codegen

ABataev added inline comments. Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:2887 @@ +2886,3 @@ +llvm::Value * +CGOpenMPRuntime::emitTargetOutlinedFunction(CodeGenFunction CGF, +const OMPExecutableDirective D, I don't

Re: [PATCH] D11361: [OpenMP] Target directive host codegen

ABataev added a comment. Another one thing I forget to mention. Current implementation of CGOpenMPRuntime is libomp-specific. You're trying to add functionality that is libtarget-specific. Maybe it is a good idea to separate support for libomp and libtarget runtime libraries?

Re: [PATCH] D11361: [OpenMP] Target directive host codegen

sfantao added a comment. Alexey, Thanks for the review! Find my comments inlined. Thanks again! Samuel Comment at: lib/CodeGen/CGExpr.cpp:1969 @@ -1945,3 +1968,3 @@ else - return EmitCapturedFieldLValue(*this, CapturedStmtInfo-lookup(VD), -

Re: [PATCH] D11361: [OpenMP] Target directive host codegen

ABataev added inline comments. Comment at: lib/CodeGen/CGExpr.cpp:1969 @@ -1945,3 +1968,3 @@ else - return EmitCapturedFieldLValue(*this, CapturedStmtInfo-lookup(VD), - CapturedStmtInfo-getContextValue()); +

Re: [PATCH] D11361: [OpenMP] Target directive host codegen

sfantao updated this revision to Diff 32211. sfantao added a comment. This patch tries to avoid as much as possible changing the common infrastructure, by adapting the CapturedDecl creation in SEMA and by adding support to a second type of capture - ImplicitParamDecl (on top of the existent

Re: [PATCH] D11361: [OpenMP] Target directive host codegen

sfantao added a comment. Alexey, John, Thanks for the review! I've tried to address your concerns in the last diff. Please, check the inlined comments to find answers for the remarks of the previous diff. Thanks again! Samuel Comment at: