[PATCH] D60568: [OpenMP] Add support for registering requires directives with the runtime

This revision was automatically updated to reflect the committed changes. Closed by commit rL361298: [OpenMP] Add support for registering requires directives with the runtime (authored by gbercea, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Repository:

[PATCH] D60568: [OpenMP] Add support for registering requires directives with the runtime

ABataev accepted this revision. ABataev added a comment. LG. Please, commit it only after the runtime part is committed. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60568/new/ https://reviews.llvm.org/D60568 ___

[PATCH] D60568: [OpenMP] Add support for registering requires directives with the runtime

gtbercea updated this revision to Diff 200504. gtbercea added a comment. - Remove new option. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60568/new/ https://reviews.llvm.org/D60568 Files: lib/CodeGen/CGOpenMPRuntime.cpp lib/CodeGen/CGOpenMPRuntime.h

[PATCH] D60568: [OpenMP] Add support for registering requires directives with the runtime

gtbercea marked an inline comment as done. gtbercea added inline comments. Comment at: lib/Frontend/CompilerInvocation.cpp:2840 Opts.OpenMP && Args.hasArg(options::OPT_fopenmp_is_device); + Opts.OpenMPHasTargets = + Opts.OpenMP &&

[PATCH] D60568: [OpenMP] Add support for registering requires directives with the runtime

ABataev added inline comments. Comment at: lib/Frontend/CompilerInvocation.cpp:2840 Opts.OpenMP && Args.hasArg(options::OPT_fopenmp_is_device); + Opts.OpenMPHasTargets = + Opts.OpenMP && Args.hasArg(options::OPT_fopenmp_targets_EQ); We already have

[PATCH] D60568: [OpenMP] Add support for registering requires directives with the runtime

gtbercea added a comment. In D60568#1508909 , @ABataev wrote: > Looks good, in general. Just one more question: will __tgt_register_requires > be emitted if only -fopenmp is used? If -fopenmp-targets is not provided, > this function should not be

[PATCH] D60568: [OpenMP] Add support for registering requires directives with the runtime

gtbercea updated this revision to Diff 200355. gtbercea added a comment. - Avoid requires registration when no targets are specified. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60568/new/ https://reviews.llvm.org/D60568 Files:

[PATCH] D60568: [OpenMP] Add support for registering requires directives with the runtime

ABataev added a comment. Looks good, in general. Just one more question: will __tgt_register_requires be emitted if only -fopenmp is used? If -fopenmp-targets is not provided, this function should not be called, I think, because we're not going to use offloading at all in this case and

[PATCH] D60568: [OpenMP] Add support for registering requires directives with the runtime

gtbercea added a comment. ping Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60568/new/ https://reviews.llvm.org/D60568 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D60568: [OpenMP] Add support for registering requires directives with the runtime

gtbercea updated this revision to Diff 200091. gtbercea added a comment. - Fix braces. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60568/new/ https://reviews.llvm.org/D60568 Files: lib/CodeGen/CGOpenMPRuntime.cpp lib/CodeGen/CGOpenMPRuntime.h

[PATCH] D60568: [OpenMP] Add support for registering requires directives with the runtime

gtbercea updated this revision to Diff 200089. gtbercea marked 3 inline comments as done. gtbercea added a comment. - Fix format. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60568/new/ https://reviews.llvm.org/D60568 Files: lib/CodeGen/CGOpenMPRuntime.cpp

[PATCH] D60568: [OpenMP] Add support for registering requires directives with the runtime

gtbercea updated this revision to Diff 200088. gtbercea added a comment. - Fix conditions. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60568/new/ https://reviews.llvm.org/D60568 Files: lib/CodeGen/CGOpenMPRuntime.cpp lib/CodeGen/CGOpenMPRuntime.h

[PATCH] D60568: [OpenMP] Add support for registering requires directives with the runtime

ABataev added inline comments. Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:9291 + if (CGM.getLangOpts().OpenMPSimd || CGM.getLangOpts().OpenMPIsDevice || + (OffloadEntriesInfoManager.empty() && !HasEmittedDeclareTargetRegion)) +return nullptr; Missed

[PATCH] D60568: [OpenMP] Add support for registering requires directives with the runtime

gtbercea marked an inline comment as done. gtbercea added inline comments. Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:10364 + if (const auto *FD = dyn_cast(D)) { +if (OMPDeclareTargetDeclAttr::isDeclareTargetDeclaration(FD)) { + HasEmittedDeclareTargetRegion = true;

[PATCH] D60568: [OpenMP] Add support for registering requires directives with the runtime

ABataev added inline comments. Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:10364 + if (const auto *FD = dyn_cast(D)) { +if (OMPDeclareTargetDeclAttr::isDeclareTargetDeclaration(FD)) { + HasEmittedDeclareTargetRegion = true; gtbercea wrote: > ABataev

[PATCH] D60568: [OpenMP] Add support for registering requires directives with the runtime

gtbercea marked an inline comment as done. gtbercea added inline comments. Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:10364 + if (const auto *FD = dyn_cast(D)) { +if (OMPDeclareTargetDeclAttr::isDeclareTargetDeclaration(FD)) { + HasEmittedDeclareTargetRegion = true;

[PATCH] D60568: [OpenMP] Add support for registering requires directives with the runtime

ABataev added inline comments. Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:10364 + if (const auto *FD = dyn_cast(D)) { +if (OMPDeclareTargetDeclAttr::isDeclareTargetDeclaration(FD)) { + HasEmittedDeclareTargetRegion = true; gtbercea wrote: > ABataev

[PATCH] D60568: [OpenMP] Add support for registering requires directives with the runtime

gtbercea marked an inline comment as done. gtbercea added inline comments. Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:10364 + if (const auto *FD = dyn_cast(D)) { +if (OMPDeclareTargetDeclAttr::isDeclareTargetDeclaration(FD)) { + HasEmittedDeclareTargetRegion = true;

[PATCH] D60568: [OpenMP] Add support for registering requires directives with the runtime

ABataev added inline comments. Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:10362 +void CGOpenMPRuntime::emitFunctionProlog(CodeGenFunction , const Decl *D){ + if (const auto *FD = dyn_cast(D)) { Bad formatting. Comment at:

[PATCH] D60568: [OpenMP] Add support for registering requires directives with the runtime

gtbercea updated this revision to Diff 196945. gtbercea marked 2 inline comments as done. gtbercea added a comment. - Add tests. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60568/new/ https://reviews.llvm.org/D60568 Files: lib/CodeGen/CGOpenMPRuntime.cpp

[PATCH] D60568: [OpenMP] Add support for registering requires directives with the runtime

ABataev added inline comments. Comment at: test/OpenMP/openmp_offload_registration.cpp:40 +// CHECK: ret void +// CHECK: declare void @__tgt_register_requires(i64) + ABataev wrote: > Why do you need this check? Again, this check is not required

[PATCH] D60568: [OpenMP] Add support for registering requires directives with the runtime

gtbercea updated this revision to Diff 195775. gtbercea added a comment. - Move error check in sema. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60568/new/ https://reviews.llvm.org/D60568 Files: lib/CodeGen/CGOpenMPRuntime.cpp

[PATCH] D60568: [OpenMP] Add support for registering requires directives with the runtime

ABataev added inline comments. Comment at: include/clang/Basic/DiagnosticSemaKinds.td:9135 "%0 clause previously used here">; +def err_omp_target_before_requires : Error < + "Target region encountered before requires directive with %0 clause.">; Split the

[PATCH] D60568: [OpenMP] Add support for registering requires directives with the runtime

gtbercea updated this revision to Diff 195739. gtbercea added a comment. - Add const. - Address comments. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60568/new/ https://reviews.llvm.org/D60568 Files: include/clang/Basic/DiagnosticSemaKinds.td

[PATCH] D60568: [OpenMP] Add support for registering requires directives with the runtime

ABataev added inline comments. Comment at: lib/Sema/SemaOpenMP.cpp:476 + isa(CNew))) { +SemaRef.Diag(Loc, + diag::err_omp_target_before_requires) gtbercea wrote: > ABataev wrote: > > DSAStack usually is not used for

[PATCH] D60568: [OpenMP] Add support for registering requires directives with the runtime

gtbercea marked an inline comment as done. gtbercea added inline comments. Comment at: lib/Sema/SemaOpenMP.cpp:476 + isa(CNew))) { +SemaRef.Diag(Loc, + diag::err_omp_target_before_requires) ABataev wrote: > DSAStack usually

[PATCH] D60568: [OpenMP] Add support for registering requires directives with the runtime

ABataev added inline comments. Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:7956 // Map other list items in the map clause which are not captured variables -// but "declare target link" global variables., +// but "declare target link" global variables. for

[PATCH] D60568: [OpenMP] Add support for registering requires directives with the runtime

gtbercea updated this revision to Diff 195644. gtbercea added a comment. - Update tests. - Move error check in sema. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60568/new/ https://reviews.llvm.org/D60568 Files: include/clang/Basic/DiagnosticSemaKinds.td

[PATCH] D60568: [OpenMP] Add support for registering requires directives with the runtime

ABataev added a comment. Other tests in presence of the requires directive with the clause? Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:8975 +CGM.Error(Clause->getBeginLoc(), + "Target region emitted before requires directive."); +

[PATCH] D60568: [OpenMP] Add support for registering requires directives with the runtime

gtbercea marked 2 inline comments as done. gtbercea added inline comments. Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:8975 +CGM.Error(Clause->getBeginLoc(), + "Target region emitted before requires directive."); + HasRequiresUnifiedSharedMemory =

[PATCH] D60568: [OpenMP] Add support for registering requires directives with the runtime

gtbercea updated this revision to Diff 195609. gtbercea marked an inline comment as done. gtbercea added a comment. - Update error message. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60568/new/ https://reviews.llvm.org/D60568 Files:

[PATCH] D60568: [OpenMP] Add support for registering requires directives with the runtime

gtbercea updated this revision to Diff 195608. gtbercea marked an inline comment as done. gtbercea added a comment. - Add support for declare target routines. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60568/new/ https://reviews.llvm.org/D60568 Files:

[PATCH] D60568: [OpenMP] Add support for registering requires directives with the runtime

ABataev added inline comments. Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:8975 +CGM.Error(Clause->getBeginLoc(), + "Target region emitted before requires directive."); + HasRequiresUnifiedSharedMemory = true; The message speaks

[PATCH] D60568: [OpenMP] Add support for registering requires directives with the runtime

gtbercea marked an inline comment as done. gtbercea added inline comments. Comment at: lib/CodeGen/CGOpenMPRuntime.h:644 + /// Flag for keeping track of weather a target region has been emitted. + bool hasEmittedTargetRegion = false; + ABataev wrote: >

[PATCH] D60568: [OpenMP] Add support for registering requires directives with the runtime

AlexEichenberger accepted this revision. AlexEichenberger added a comment. This revision is now accepted and ready to land. fantastic Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60568/new/ https://reviews.llvm.org/D60568

[PATCH] D60568: [OpenMP] Add support for registering requires directives with the runtime

ABataev added inline comments. Comment at: lib/CodeGen/CGOpenMPRuntime.h:644 + /// Flag for keeping track of weather a target region has been emitted. + bool hasEmittedTargetRegion = false; + gtbercea wrote: > ABataev wrote: > > gtbercea wrote: > > > gtbercea

[PATCH] D60568: [OpenMP] Add support for registering requires directives with the runtime

gtbercea marked an inline comment as done. gtbercea added inline comments. Comment at: lib/CodeGen/CGOpenMPRuntime.h:644 + /// Flag for keeping track of weather a target region has been emitted. + bool hasEmittedTargetRegion = false; + ABataev wrote: >

[PATCH] D60568: [OpenMP] Add support for registering requires directives with the runtime

ABataev added inline comments. Comment at: lib/CodeGen/CGOpenMPRuntime.h:644 + /// Flag for keeping track of weather a target region has been emitted. + bool hasEmittedTargetRegion = false; + gtbercea wrote: > gtbercea wrote: > > ABataev wrote: > > > gtbercea

[PATCH] D60568: [OpenMP] Add support for registering requires directives with the runtime

gtbercea marked an inline comment as done. gtbercea added inline comments. Comment at: lib/CodeGen/CGOpenMPRuntime.h:644 + /// Flag for keeping track of weather a target region has been emitted. + bool hasEmittedTargetRegion = false; + gtbercea wrote: >

[PATCH] D60568: [OpenMP] Add support for registering requires directives with the runtime

gtbercea marked an inline comment as done. gtbercea added inline comments. Comment at: lib/CodeGen/CGOpenMPRuntime.h:644 + /// Flag for keeping track of weather a target region has been emitted. + bool hasEmittedTargetRegion = false; + ABataev wrote: >

[PATCH] D60568: [OpenMP] Add support for registering requires directives with the runtime

ABataev added inline comments. Comment at: lib/CodeGen/CGOpenMPRuntime.h:644 + /// Flag for keeping track of weather a target region has been emitted. + bool hasEmittedTargetRegion = false; + gtbercea wrote: > ABataev wrote: > > Why do you need this? I think

[PATCH] D60568: [OpenMP] Add support for registering requires directives with the runtime

gtbercea marked an inline comment as done. gtbercea added inline comments. Comment at: lib/CodeGen/CGOpenMPRuntime.h:644 + /// Flag for keeping track of weather a target region has been emitted. + bool hasEmittedTargetRegion = false; + ABataev wrote: > Why do

[PATCH] D60568: [OpenMP] Add support for registering requires directives with the runtime

ABataev added inline comments. Comment at: lib/CodeGen/CGOpenMPRuntime.h:644 + /// Flag for keeping track of weather a target region has been emitted. + bool hasEmittedTargetRegion = false; + Why do you need this? I think your function should be called without

[PATCH] D60568: [OpenMP] Add support for registering requires directives with the runtime

gtbercea updated this revision to Diff 195394. gtbercea added a comment. - Fix checks for use of requires directive. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60568/new/ https://reviews.llvm.org/D60568 Files: lib/CodeGen/CGOpenMPRuntime.cpp

[PATCH] D60568: [OpenMP] Add support for registering requires directives with the runtime

gtbercea marked an inline comment as done. gtbercea added inline comments. Comment at: test/OpenMP/openmp_offload_registration.cpp:38 +// CHECK: define internal void @.omp_offloading.requires_reg() +// CHECK: call void @__tgt_register_requires(i64 0) +// CHECK: ret

[PATCH] D60568: [OpenMP] Add support for registering requires directives with the runtime

ABataev added inline comments. Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:8978 +if (Clause->getClauseKind() == OMPC_unified_shared_memory) { + CGM.getOpenMPRuntime().HasRequiresUnifiedSharedMemory = true; + break; gtbercea wrote: > gtbercea wrote:

[PATCH] D60568: [OpenMP] Add support for registering requires directives with the runtime

gtbercea updated this revision to Diff 195200. gtbercea added a comment. - Remove const. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60568/new/ https://reviews.llvm.org/D60568 Files: lib/CodeGen/CGOpenMPRuntime.cpp lib/CodeGen/CGOpenMPRuntime.h

[PATCH] D60568: [OpenMP] Add support for registering requires directives with the runtime

gtbercea marked 2 inline comments as done. gtbercea added inline comments. Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:8978 +if (Clause->getClauseKind() == OMPC_unified_shared_memory) { + CGM.getOpenMPRuntime().HasRequiresUnifiedSharedMemory = true; + break;

[PATCH] D60568: [OpenMP] Add support for registering requires directives with the runtime

gtbercea marked 2 inline comments as done. gtbercea added inline comments. Comment at: lib/CodeGen/CGOpenMPRuntime.h:1438 + /// requires directives was used in the current module. + virtual llvm::Function *emitRequiresDirectiveRegFun(); + ABataev wrote: >

[PATCH] D60568: [OpenMP] Add support for registering requires directives with the runtime

gtbercea updated this revision to Diff 195195. gtbercea added a comment. - Add break. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60568/new/ https://reviews.llvm.org/D60568 Files: lib/CodeGen/CGOpenMPRuntime.cpp lib/CodeGen/CGOpenMPRuntime.h

[PATCH] D60568: [OpenMP] Add support for registering requires directives with the runtime

gtbercea updated this revision to Diff 195190. gtbercea added a comment. - Fix type. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60568/new/ https://reviews.llvm.org/D60568 Files: lib/CodeGen/CGOpenMPRuntime.cpp lib/CodeGen/CGOpenMPRuntime.h

[PATCH] D60568: [OpenMP] Add support for registering requires directives with the runtime

ABataev added inline comments. Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:8970 +if (Clause->getClauseKind() == OMPC_unified_shared_memory) + CGM.getOpenMPRuntime().HasRequiresUnifiedSharedMemory = true; + } You can do `break;` here, no need for

[PATCH] D60568: [OpenMP] Add support for registering requires directives with the runtime

gtbercea updated this revision to Diff 195186. gtbercea added a comment. - Remove atomic flags. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60568/new/ https://reviews.llvm.org/D60568 Files: lib/CodeGen/CGOpenMPRuntime.cpp lib/CodeGen/CGOpenMPRuntime.h

[PATCH] D60568: [OpenMP] Add support for registering requires directives with the runtime

gtbercea updated this revision to Diff 195185. gtbercea added a comment. - Address comments. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60568/new/ https://reviews.llvm.org/D60568 Files: lib/CodeGen/CGOpenMPRuntime.cpp lib/CodeGen/CGOpenMPRuntime.h

[PATCH] D60568: [OpenMP] Add support for registering requires directives with the runtime

ABataev added inline comments. Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:473 + /// atomic_default_mem_order seq_cst clause. + OMP_REQ_ATOMIC_DEFAULT_SEQ_CST = 0x008, + /// atomic_default_mem_order acq_rel clause. YOu don't need al these flags, add only

[PATCH] D60568: [OpenMP] Add support for registering requires directives with the runtime

AlexEichenberger requested changes to this revision. AlexEichenberger added a comment. This revision now requires changes to proceed. see note above; apologies if it is already done and hiding somewhere I did not see Comment at: lib/CodeGen/CGOpenMPRuntime.h:641 + ///

[PATCH] D60568: [OpenMP] Add support for registering requires directives with the runtime

gtbercea updated this revision to Diff 194781. gtbercea added a comment. - Handle OpenMP simd case. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60568/new/ https://reviews.llvm.org/D60568 Files: lib/CodeGen/CGOpenMPRuntime.cpp lib/CodeGen/CGOpenMPRuntime.h

[PATCH] D60568: [OpenMP] Add support for registering requires directives with the runtime

gtbercea updated this revision to Diff 194760. gtbercea added a comment. - Split patch. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60568/new/ https://reviews.llvm.org/D60568 Files: lib/CodeGen/CGOpenMPRuntime.cpp lib/CodeGen/CGOpenMPRuntime.h

[PATCH] D60568: [OpenMP] Add support for registering requires directives with the runtime

gtbercea updated this revision to Diff 194758. gtbercea marked 2 inline comments as done. gtbercea added a comment. - Fix enum. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60568/new/ https://reviews.llvm.org/D60568 Files: lib/CodeGen/CGOpenMPRuntime.cpp

[PATCH] D60568: [OpenMP] Add support for registering requires directives with the runtime

gtbercea marked 4 inline comments as done. gtbercea added inline comments. Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:3870 llvm::Function * +CGOpenMPRuntime::createRequiresDirectiveRegistration() { + // If we don't have entries or if we are emitting code for the device, we

[PATCH] D60568: [OpenMP] Add support for registering requires directives with the runtime

gtbercea updated this revision to Diff 194743. gtbercea marked 6 inline comments as done. gtbercea added a comment. - Address comments. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60568/new/ https://reviews.llvm.org/D60568 Files:

[PATCH] D60568: [OpenMP] Add support for registering requires directives with the runtime

gtbercea updated this revision to Diff 194741. gtbercea added a comment. - Address comments. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60568/new/ https://reviews.llvm.org/D60568 Files: lib/CodeGen/CGOpenMPRuntime.cpp lib/CodeGen/CGOpenMPRuntime.h

[PATCH] D60568: [OpenMP] Add support for registering requires directives with the runtime

ABataev added inline comments. Comment at: lib/CodeGen/CGDecl.cpp:2576 void CodeGenModule::EmitOMPRequiresDecl(const OMPRequiresDecl *D) { - getOpenMPRuntime().checkArchForUnifiedAddressing(D); } You don't need to pass the reference to CodeGenModule to

[PATCH] D60568: [OpenMP] Add support for registering requires directives with the runtime

gtbercea created this revision. gtbercea added reviewers: ABataev, AlexEichenberger, caomhin. Herald added subscribers: cfe-commits, jdoerfert, jfb, guansong, jholewinski. Herald added a project: clang. This patch adds support for the registration of the requires directives with the runtime.