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

[Phabricator via Phabricator via cfe-commits]

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:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60568/new/

https://reviews.llvm.org/D60568

Files:
  cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp
  cfe/trunk/lib/CodeGen/CGOpenMPRuntime.h
  cfe/trunk/lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
  cfe/trunk/lib/CodeGen/CGOpenMPRuntimeNVPTX.h
  cfe/trunk/lib/CodeGen/CodeGenModule.cpp
  cfe/trunk/test/OpenMP/openmp_offload_registration.cpp
  cfe/trunk/test/OpenMP/target_codegen.cpp
  cfe/trunk/test/OpenMP/target_codegen_registration.cpp
  cfe/trunk/test/OpenMP/target_depend_codegen.cpp
  cfe/trunk/test/OpenMP/target_parallel_codegen.cpp
  cfe/trunk/test/OpenMP/target_parallel_codegen_registration.cpp
  cfe/trunk/test/OpenMP/target_parallel_depend_codegen.cpp
  cfe/trunk/test/OpenMP/target_parallel_for_codegen.cpp
  cfe/trunk/test/OpenMP/target_parallel_for_codegen_registration.cpp
  cfe/trunk/test/OpenMP/target_parallel_for_depend_codegen.cpp
  cfe/trunk/test/OpenMP/target_parallel_for_simd_codegen.cpp
  cfe/trunk/test/OpenMP/target_parallel_for_simd_codegen_registration.cpp
  cfe/trunk/test/OpenMP/target_parallel_for_simd_depend_codegen.cpp
  cfe/trunk/test/OpenMP/target_parallel_if_codegen.cpp
  cfe/trunk/test/OpenMP/target_parallel_num_threads_codegen.cpp
  cfe/trunk/test/OpenMP/target_simd_codegen.cpp
  cfe/trunk/test/OpenMP/target_simd_codegen_registration.cpp
  cfe/trunk/test/OpenMP/target_simd_depend_codegen.cpp
  cfe/trunk/test/OpenMP/target_teams_codegen.cpp
  cfe/trunk/test/OpenMP/target_teams_codegen_registration.cpp
  cfe/trunk/test/OpenMP/target_teams_depend_codegen.cpp
  cfe/trunk/test/OpenMP/target_teams_distribute_codegen.cpp
  cfe/trunk/test/OpenMP/target_teams_distribute_codegen_registration.cpp
  cfe/trunk/test/OpenMP/target_teams_distribute_depend_codegen.cpp
  cfe/trunk/test/OpenMP/target_teams_distribute_parallel_for_depend_codegen.cpp
  
cfe/trunk/test/OpenMP/target_teams_distribute_parallel_for_simd_codegen_registration.cpp
  
cfe/trunk/test/OpenMP/target_teams_distribute_parallel_for_simd_depend_codegen.cpp
  cfe/trunk/test/OpenMP/target_teams_distribute_simd_codegen.cpp
  cfe/trunk/test/OpenMP/target_teams_distribute_simd_codegen_registration.cpp
  cfe/trunk/test/OpenMP/target_teams_distribute_simd_depend_codegen.cpp
  cfe/trunk/test/OpenMP/target_teams_num_teams_codegen.cpp
  cfe/trunk/test/OpenMP/target_teams_thread_limit_codegen.cpp

Index: cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp
===
--- cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp
+++ cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp
@@ -457,6 +457,26 @@
   LLVM_MARK_AS_BITMASK_ENUM(/*LargestValue=*/OMP_IDENT_WORK_DISTRIBUTE)
 };
 
+namespace {
+LLVM_ENABLE_BITMASK_ENUMS_IN_NAMESPACE();
+/// Values for bit flags for marking which requires clauses have been used.
+enum OpenMPOffloadingRequiresDirFlags : int64_t {
+  /// flag undefined.
+  OMP_REQ_UNDEFINED   = 0x000,
+  /// no requires clause present.
+  OMP_REQ_NONE= 0x001,
+  /// reverse_offload clause.
+  OMP_REQ_REVERSE_OFFLOAD = 0x002,
+  /// unified_address clause.
+  OMP_REQ_UNIFIED_ADDRESS = 0x004,
+  /// unified_shared_memory clause.
+  OMP_REQ_UNIFIED_SHARED_MEMORY   = 0x008,
+  /// dynamic_allocators clause.
+  OMP_REQ_DYNAMIC_ALLOCATORS  = 0x010,
+  LLVM_MARK_AS_BITMASK_ENUM(/*LargestValue=*/OMP_REQ_DYNAMIC_ALLOCATORS)
+};
+} // anonymous namespace
+
 /// Describes ident structure that describes a source location.
 /// All descriptions are taken from
 /// https://github.com/llvm/llvm-project/blob/master/openmp/runtime/src/kmp.h
@@ -694,6 +714,8 @@
   // *host_ptr, int32_t arg_num, void** args_base, void **args, size_t
   // *arg_sizes, int64_t *arg_types, int32_t num_teams, int32_t thread_limit);
   OMPRTL__tgt_target_teams_nowait,
+  // Call to void __tgt_register_requires(int64_t flags);
+  OMPRTL__tgt_register_requires,
   // Call to void __tgt_register_lib(__tgt_bin_desc *desc);
   OMPRTL__tgt_register_lib,
   // Call to void __tgt_unregister_lib(__tgt_bin_desc *desc);
@@ -2296,6 +2318,14 @@
 RTLFn = CGM.CreateRuntimeFunction(FnTy, "__tgt_target_teams_nowait");
 break;
   }
+  case OMPRTL__tgt_register_requires: {
+// Build void __tgt_register_requires(int64_t flags);
+llvm::Type *TypeParams[] = {CGM.Int64Ty};
+auto *FnTy =
+llvm::FunctionType::get(CGM.VoidTy, TypeParams, /*isVarArg*/ false);
+RTLFn = CGM.CreateRuntimeFunction(FnTy, "__tgt_register_requires");
+break;
+  }
   case OMPRTL__tgt_register_lib: {
 // Build void __tgt_register_lib(__tgt_bin_desc *desc);
 QualType ParamTy =
@@ -6405,6 +6435,7 @@
 llvm::Function *&OutlinedFn, llvm::Constant *&OutlinedFnID

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

[Alexey Bataev via Phabricator via cfe-commits]

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



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

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

[Gheorghe-Teodor Bercea via Phabricator via cfe-commits]

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
  lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
  lib/CodeGen/CGOpenMPRuntimeNVPTX.h
  lib/CodeGen/CodeGenModule.cpp
  test/OpenMP/openmp_offload_registration.cpp
  test/OpenMP/target_codegen.cpp
  test/OpenMP/target_codegen_registration.cpp
  test/OpenMP/target_depend_codegen.cpp
  test/OpenMP/target_parallel_codegen.cpp
  test/OpenMP/target_parallel_codegen_registration.cpp
  test/OpenMP/target_parallel_depend_codegen.cpp
  test/OpenMP/target_parallel_for_codegen.cpp
  test/OpenMP/target_parallel_for_codegen_registration.cpp
  test/OpenMP/target_parallel_for_depend_codegen.cpp
  test/OpenMP/target_parallel_for_simd_codegen.cpp
  test/OpenMP/target_parallel_for_simd_codegen_registration.cpp
  test/OpenMP/target_parallel_for_simd_depend_codegen.cpp
  test/OpenMP/target_parallel_if_codegen.cpp
  test/OpenMP/target_parallel_num_threads_codegen.cpp
  test/OpenMP/target_simd_codegen.cpp
  test/OpenMP/target_simd_codegen_registration.cpp
  test/OpenMP/target_simd_depend_codegen.cpp
  test/OpenMP/target_teams_codegen.cpp
  test/OpenMP/target_teams_codegen_registration.cpp
  test/OpenMP/target_teams_depend_codegen.cpp
  test/OpenMP/target_teams_distribute_codegen.cpp
  test/OpenMP/target_teams_distribute_codegen_registration.cpp
  test/OpenMP/target_teams_distribute_depend_codegen.cpp
  test/OpenMP/target_teams_distribute_parallel_for_depend_codegen.cpp
  test/OpenMP/target_teams_distribute_parallel_for_simd_codegen_registration.cpp
  test/OpenMP/target_teams_distribute_parallel_for_simd_depend_codegen.cpp
  test/OpenMP/target_teams_distribute_simd_codegen.cpp
  test/OpenMP/target_teams_distribute_simd_codegen_registration.cpp
  test/OpenMP/target_teams_distribute_simd_depend_codegen.cpp
  test/OpenMP/target_teams_num_teams_codegen.cpp
  test/OpenMP/target_teams_thread_limit_codegen.cpp

Index: test/OpenMP/target_teams_thread_limit_codegen.cpp
===
--- test/OpenMP/target_teams_thread_limit_codegen.cpp
+++ test/OpenMP/target_teams_thread_limit_codegen.cpp
@@ -76,7 +76,7 @@
 // CHECK: [[DESC:@.+]] = internal constant [[DSCTY]] { i32 1, [[DEVTY]]* getelementptr inbounds ([1 x [[DEVTY]]], [1 x [[DEVTY]]]* [[IMAGES]], i32 0, i32 0), [[ENTTY]]* [[ENTBEGIN]], [[ENTTY]]* [[ENTEND]] }, comdat($[[REGFN]])
 
 // Check target registration is registered as a Ctor.
-// CHECK: appending global [1 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 0, void ()* @[[REGFN]], i8* bitcast (void ()* @[[REGFN]] to i8*) }]
+// CHECK: appending global [2 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 0, void ()* @.omp_offloading.requires_reg, i8* null }, { i32, void ()*, i8* } { i32 0, void ()* @[[REGFN]], i8* bitcast (void ()* @[[REGFN]] to i8*) }]
 
 
 template
Index: test/OpenMP/target_teams_num_teams_codegen.cpp
===
--- test/OpenMP/target_teams_num_teams_codegen.cpp
+++ test/OpenMP/target_teams_num_teams_codegen.cpp
@@ -76,7 +76,7 @@
 // CHECK: [[DESC:@.+]] = internal constant [[DSCTY]] { i32 1, [[DEVTY]]* getelementptr inbounds ([1 x [[DEVTY]]], [1 x [[DEVTY]]]* [[IMAGES]], i32 0, i32 0), [[ENTTY]]* [[ENTBEGIN]], [[ENTTY]]* [[ENTEND]] }, comdat($[[REGFN]])
 
 // Check target registration is registered as a Ctor.
-// CHECK: appending global [1 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 0, void ()* @[[REGFN]], i8* bitcast (void ()* @[[REGFN]] to i8*) }]
+// CHECK: appending global [2 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 0, void ()* @.omp_offloading.requires_reg, i8* null }, { i32, void ()*, i8* } { i32 0, void ()* @[[REGFN]], i8* bitcast (void ()* @[[REGFN]] to i8*) }]
 
 
 template
Index: test/OpenMP/target_teams_distribute_simd_depend_codegen.cpp
===
--- test/OpenMP/target_teams_distribute_simd_depend_codegen.cpp
+++ test/OpenMP/target_teams_distribute_simd_depend_codegen.cpp
@@ -64,7 +64,7 @@
 // CHECK: [[DESC:@.+]] = internal constant [[DSCTY]] { i32 1, [[DEVTY]]* getelementptr inbounds ([1 x [[DEVTY]]], [1 x [[DEVTY]]]* [[IMAGES]], i32 0, i32 0), [[ENTTY]]* [[ENTBEGIN]], [[ENTTY]]* [[ENTEND]] }, comdat($[[REGFN]])
 
 // Check target registration is registered as a Ctor.
-// CHECK: appending global [1 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 0, void ()* @[[REGFN]], i8* bitcast (void ()* @[[REGFN]] to i8*) }]
+// CHECK: appending global [2 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 0, void ()* @.omp_offloading.requires_reg, i8* null }, { i32, void ()*, i8* } { i32 0, void ()* @[[REGFN]], i8* bitcast (void ()* @[[REGFN]] to i8*) }]
 
 
 template
Index: test/OpenMP/target_teams_distribute_simd_co

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

[Gheorghe-Teodor Bercea via Phabricator via cfe-commits]

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 && Args.hasArg(options::OPT_fopenmp_targets_EQ);

ABataev wrote:
> We already have `Opts.OMPTargetTripples`. We use it currently to check if the 
> offloading enabled `!CGM.getLangOpts().OMPTargetTriples.empty()`, so, 
> probably, we don't need this new flag.
Even better!! I'll make the change!


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
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Alexey Bataev via Phabricator via cfe-commits]

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 `Opts.OMPTargetTripples`. We use it currently to check if the 
offloading enabled `!CGM.getLangOpts().OMPTargetTriples.empty()`, so, probably, 
we don't need this new flag.


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
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Gheorghe-Teodor Bercea via Phabricator via cfe-commits]

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 called, I think, because we're not going to use 
> offloading at all in this case and __tgt_register_requires should not be 
> emitted. Otherwise, we may end up with the linker error that 
> __tgt_register_requires is not found. Would be good to have a test with 
> -fopenmp and #pragma omp requires that check that no __tgt_register_requires 
> is emitted in this case.


Done with test included.


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
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Gheorghe-Teodor Bercea via Phabricator via cfe-commits]

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:
  include/clang/Basic/LangOptions.def
  lib/CodeGen/CGOpenMPRuntime.cpp
  lib/CodeGen/CGOpenMPRuntime.h
  lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
  lib/CodeGen/CGOpenMPRuntimeNVPTX.h
  lib/CodeGen/CodeGenModule.cpp
  lib/Frontend/CompilerInvocation.cpp
  test/OpenMP/openmp_offload_registration.cpp
  test/OpenMP/target_codegen.cpp
  test/OpenMP/target_codegen_registration.cpp
  test/OpenMP/target_depend_codegen.cpp
  test/OpenMP/target_parallel_codegen.cpp
  test/OpenMP/target_parallel_codegen_registration.cpp
  test/OpenMP/target_parallel_depend_codegen.cpp
  test/OpenMP/target_parallel_for_codegen.cpp
  test/OpenMP/target_parallel_for_codegen_registration.cpp
  test/OpenMP/target_parallel_for_depend_codegen.cpp
  test/OpenMP/target_parallel_for_simd_codegen.cpp
  test/OpenMP/target_parallel_for_simd_codegen_registration.cpp
  test/OpenMP/target_parallel_for_simd_depend_codegen.cpp
  test/OpenMP/target_parallel_if_codegen.cpp
  test/OpenMP/target_parallel_num_threads_codegen.cpp
  test/OpenMP/target_simd_codegen.cpp
  test/OpenMP/target_simd_codegen_registration.cpp
  test/OpenMP/target_simd_depend_codegen.cpp
  test/OpenMP/target_teams_codegen.cpp
  test/OpenMP/target_teams_codegen_registration.cpp
  test/OpenMP/target_teams_depend_codegen.cpp
  test/OpenMP/target_teams_distribute_codegen.cpp
  test/OpenMP/target_teams_distribute_codegen_registration.cpp
  test/OpenMP/target_teams_distribute_depend_codegen.cpp
  test/OpenMP/target_teams_distribute_parallel_for_depend_codegen.cpp
  test/OpenMP/target_teams_distribute_parallel_for_simd_codegen_registration.cpp
  test/OpenMP/target_teams_distribute_parallel_for_simd_depend_codegen.cpp
  test/OpenMP/target_teams_distribute_simd_codegen.cpp
  test/OpenMP/target_teams_distribute_simd_codegen_registration.cpp
  test/OpenMP/target_teams_distribute_simd_depend_codegen.cpp
  test/OpenMP/target_teams_num_teams_codegen.cpp
  test/OpenMP/target_teams_thread_limit_codegen.cpp

Index: test/OpenMP/target_teams_thread_limit_codegen.cpp
===
--- test/OpenMP/target_teams_thread_limit_codegen.cpp
+++ test/OpenMP/target_teams_thread_limit_codegen.cpp
@@ -76,7 +76,7 @@
 // CHECK: [[DESC:@.+]] = internal constant [[DSCTY]] { i32 1, [[DEVTY]]* getelementptr inbounds ([1 x [[DEVTY]]], [1 x [[DEVTY]]]* [[IMAGES]], i32 0, i32 0), [[ENTTY]]* [[ENTBEGIN]], [[ENTTY]]* [[ENTEND]] }, comdat($[[REGFN]])
 
 // Check target registration is registered as a Ctor.
-// CHECK: appending global [1 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 0, void ()* @[[REGFN]], i8* bitcast (void ()* @[[REGFN]] to i8*) }]
+// CHECK: appending global [2 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 0, void ()* @.omp_offloading.requires_reg, i8* null }, { i32, void ()*, i8* } { i32 0, void ()* @[[REGFN]], i8* bitcast (void ()* @[[REGFN]] to i8*) }]
 
 
 template
Index: test/OpenMP/target_teams_num_teams_codegen.cpp
===
--- test/OpenMP/target_teams_num_teams_codegen.cpp
+++ test/OpenMP/target_teams_num_teams_codegen.cpp
@@ -76,7 +76,7 @@
 // CHECK: [[DESC:@.+]] = internal constant [[DSCTY]] { i32 1, [[DEVTY]]* getelementptr inbounds ([1 x [[DEVTY]]], [1 x [[DEVTY]]]* [[IMAGES]], i32 0, i32 0), [[ENTTY]]* [[ENTBEGIN]], [[ENTTY]]* [[ENTEND]] }, comdat($[[REGFN]])
 
 // Check target registration is registered as a Ctor.
-// CHECK: appending global [1 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 0, void ()* @[[REGFN]], i8* bitcast (void ()* @[[REGFN]] to i8*) }]
+// CHECK: appending global [2 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 0, void ()* @.omp_offloading.requires_reg, i8* null }, { i32, void ()*, i8* } { i32 0, void ()* @[[REGFN]], i8* bitcast (void ()* @[[REGFN]] to i8*) }]
 
 
 template
Index: test/OpenMP/target_teams_distribute_simd_depend_codegen.cpp
===
--- test/OpenMP/target_teams_distribute_simd_depend_codegen.cpp
+++ test/OpenMP/target_teams_distribute_simd_depend_codegen.cpp
@@ -64,7 +64,7 @@
 // CHECK: [[DESC:@.+]] = internal constant [[DSCTY]] { i32 1, [[DEVTY]]* getelementptr inbounds ([1 x [[DEVTY]]], [1 x [[DEVTY]]]* [[IMAGES]], i32 0, i32 0), [[ENTTY]]* [[ENTBEGIN]], [[ENTTY]]* [[ENTEND]] }, comdat($[[REGFN]])
 
 // Check target registration is registered as a Ctor.
-// CHECK: appending global [1 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 0, void ()* @[[REGFN]], i8* bitcast (void ()* @[[REGFN]] to i8*) }]
+// CHECK: appending global [2 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 0, void ()* @.omp_offloading.requires_reg, i8* null }, { i32, void ()*, i8* } { i32 0, void ()* @[[R

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

[Alexey Bataev via Phabricator via cfe-commits]

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 __tgt_register_requires should not be 
emitted. Otherwise, we may end up with the linker error that 
__tgt_register_requires is not found. Would be good to have a test with 
-fopenmp and #pragma omp requires that check that no __tgt_register_requires is 
emitted in this case.


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
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Gheorghe-Teodor Bercea via Phabricator via cfe-commits]

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
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Gheorghe-Teodor Bercea via Phabricator via cfe-commits]

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
  lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
  lib/CodeGen/CGOpenMPRuntimeNVPTX.h
  lib/CodeGen/CodeGenModule.cpp
  test/OpenMP/openmp_offload_registration.cpp
  test/OpenMP/target_codegen.cpp
  test/OpenMP/target_codegen_registration.cpp
  test/OpenMP/target_depend_codegen.cpp
  test/OpenMP/target_parallel_codegen.cpp
  test/OpenMP/target_parallel_codegen_registration.cpp
  test/OpenMP/target_parallel_depend_codegen.cpp
  test/OpenMP/target_parallel_for_codegen.cpp
  test/OpenMP/target_parallel_for_codegen_registration.cpp
  test/OpenMP/target_parallel_for_depend_codegen.cpp
  test/OpenMP/target_parallel_for_simd_codegen.cpp
  test/OpenMP/target_parallel_for_simd_codegen_registration.cpp
  test/OpenMP/target_parallel_for_simd_depend_codegen.cpp
  test/OpenMP/target_parallel_if_codegen.cpp
  test/OpenMP/target_parallel_num_threads_codegen.cpp
  test/OpenMP/target_simd_codegen.cpp
  test/OpenMP/target_simd_codegen_registration.cpp
  test/OpenMP/target_simd_depend_codegen.cpp
  test/OpenMP/target_teams_codegen.cpp
  test/OpenMP/target_teams_codegen_registration.cpp
  test/OpenMP/target_teams_depend_codegen.cpp
  test/OpenMP/target_teams_distribute_codegen.cpp
  test/OpenMP/target_teams_distribute_codegen_registration.cpp
  test/OpenMP/target_teams_distribute_depend_codegen.cpp
  test/OpenMP/target_teams_distribute_parallel_for_depend_codegen.cpp
  test/OpenMP/target_teams_distribute_parallel_for_simd_codegen_registration.cpp
  test/OpenMP/target_teams_distribute_parallel_for_simd_depend_codegen.cpp
  test/OpenMP/target_teams_distribute_simd_codegen.cpp
  test/OpenMP/target_teams_distribute_simd_codegen_registration.cpp
  test/OpenMP/target_teams_distribute_simd_depend_codegen.cpp
  test/OpenMP/target_teams_num_teams_codegen.cpp
  test/OpenMP/target_teams_thread_limit_codegen.cpp

Index: test/OpenMP/target_teams_thread_limit_codegen.cpp
===
--- test/OpenMP/target_teams_thread_limit_codegen.cpp
+++ test/OpenMP/target_teams_thread_limit_codegen.cpp
@@ -76,7 +76,7 @@
 // CHECK: [[DESC:@.+]] = internal constant [[DSCTY]] { i32 1, [[DEVTY]]* getelementptr inbounds ([1 x [[DEVTY]]], [1 x [[DEVTY]]]* [[IMAGES]], i32 0, i32 0), [[ENTTY]]* [[ENTBEGIN]], [[ENTTY]]* [[ENTEND]] }, comdat($[[REGFN]])
 
 // Check target registration is registered as a Ctor.
-// CHECK: appending global [1 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 0, void ()* @[[REGFN]], i8* bitcast (void ()* @[[REGFN]] to i8*) }]
+// CHECK: appending global [2 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 0, void ()* @.omp_offloading.requires_reg, i8* null }, { i32, void ()*, i8* } { i32 0, void ()* @[[REGFN]], i8* bitcast (void ()* @[[REGFN]] to i8*) }]
 
 
 template
Index: test/OpenMP/target_teams_num_teams_codegen.cpp
===
--- test/OpenMP/target_teams_num_teams_codegen.cpp
+++ test/OpenMP/target_teams_num_teams_codegen.cpp
@@ -76,7 +76,7 @@
 // CHECK: [[DESC:@.+]] = internal constant [[DSCTY]] { i32 1, [[DEVTY]]* getelementptr inbounds ([1 x [[DEVTY]]], [1 x [[DEVTY]]]* [[IMAGES]], i32 0, i32 0), [[ENTTY]]* [[ENTBEGIN]], [[ENTTY]]* [[ENTEND]] }, comdat($[[REGFN]])
 
 // Check target registration is registered as a Ctor.
-// CHECK: appending global [1 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 0, void ()* @[[REGFN]], i8* bitcast (void ()* @[[REGFN]] to i8*) }]
+// CHECK: appending global [2 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 0, void ()* @.omp_offloading.requires_reg, i8* null }, { i32, void ()*, i8* } { i32 0, void ()* @[[REGFN]], i8* bitcast (void ()* @[[REGFN]] to i8*) }]
 
 
 template
Index: test/OpenMP/target_teams_distribute_simd_depend_codegen.cpp
===
--- test/OpenMP/target_teams_distribute_simd_depend_codegen.cpp
+++ test/OpenMP/target_teams_distribute_simd_depend_codegen.cpp
@@ -64,7 +64,7 @@
 // CHECK: [[DESC:@.+]] = internal constant [[DSCTY]] { i32 1, [[DEVTY]]* getelementptr inbounds ([1 x [[DEVTY]]], [1 x [[DEVTY]]]* [[IMAGES]], i32 0, i32 0), [[ENTTY]]* [[ENTBEGIN]], [[ENTTY]]* [[ENTEND]] }, comdat($[[REGFN]])
 
 // Check target registration is registered as a Ctor.
-// CHECK: appending global [1 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 0, void ()* @[[REGFN]], i8* bitcast (void ()* @[[REGFN]] to i8*) }]
+// CHECK: appending global [2 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 0, void ()* @.omp_offloading.requires_reg, i8* null }, { i32, void ()*, i8* } { i32 0, void ()* @[[REGFN]], i8* bitcast (void ()* @[[REGFN]] to i8*) }]
 
 
 template
Index: test/OpenMP/target_teams_distribute_simd_codegen_r

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

[Gheorghe-Teodor Bercea via Phabricator via cfe-commits]

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
  lib/CodeGen/CGOpenMPRuntime.h
  lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
  lib/CodeGen/CGOpenMPRuntimeNVPTX.h
  lib/CodeGen/CodeGenModule.cpp
  test/OpenMP/openmp_offload_registration.cpp
  test/OpenMP/target_codegen.cpp
  test/OpenMP/target_codegen_registration.cpp
  test/OpenMP/target_depend_codegen.cpp
  test/OpenMP/target_parallel_codegen.cpp
  test/OpenMP/target_parallel_codegen_registration.cpp
  test/OpenMP/target_parallel_depend_codegen.cpp
  test/OpenMP/target_parallel_for_codegen.cpp
  test/OpenMP/target_parallel_for_codegen_registration.cpp
  test/OpenMP/target_parallel_for_depend_codegen.cpp
  test/OpenMP/target_parallel_for_simd_codegen.cpp
  test/OpenMP/target_parallel_for_simd_codegen_registration.cpp
  test/OpenMP/target_parallel_for_simd_depend_codegen.cpp
  test/OpenMP/target_parallel_if_codegen.cpp
  test/OpenMP/target_parallel_num_threads_codegen.cpp
  test/OpenMP/target_simd_codegen.cpp
  test/OpenMP/target_simd_codegen_registration.cpp
  test/OpenMP/target_simd_depend_codegen.cpp
  test/OpenMP/target_teams_codegen.cpp
  test/OpenMP/target_teams_codegen_registration.cpp
  test/OpenMP/target_teams_depend_codegen.cpp
  test/OpenMP/target_teams_distribute_codegen.cpp
  test/OpenMP/target_teams_distribute_codegen_registration.cpp
  test/OpenMP/target_teams_distribute_depend_codegen.cpp
  test/OpenMP/target_teams_distribute_parallel_for_depend_codegen.cpp
  test/OpenMP/target_teams_distribute_parallel_for_simd_codegen_registration.cpp
  test/OpenMP/target_teams_distribute_parallel_for_simd_depend_codegen.cpp
  test/OpenMP/target_teams_distribute_simd_codegen.cpp
  test/OpenMP/target_teams_distribute_simd_codegen_registration.cpp
  test/OpenMP/target_teams_distribute_simd_depend_codegen.cpp
  test/OpenMP/target_teams_num_teams_codegen.cpp
  test/OpenMP/target_teams_thread_limit_codegen.cpp

Index: test/OpenMP/target_teams_thread_limit_codegen.cpp
===
--- test/OpenMP/target_teams_thread_limit_codegen.cpp
+++ test/OpenMP/target_teams_thread_limit_codegen.cpp
@@ -76,7 +76,7 @@
 // CHECK: [[DESC:@.+]] = internal constant [[DSCTY]] { i32 1, [[DEVTY]]* getelementptr inbounds ([1 x [[DEVTY]]], [1 x [[DEVTY]]]* [[IMAGES]], i32 0, i32 0), [[ENTTY]]* [[ENTBEGIN]], [[ENTTY]]* [[ENTEND]] }, comdat($[[REGFN]])
 
 // Check target registration is registered as a Ctor.
-// CHECK: appending global [1 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 0, void ()* @[[REGFN]], i8* bitcast (void ()* @[[REGFN]] to i8*) }]
+// CHECK: appending global [2 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 0, void ()* @.omp_offloading.requires_reg, i8* null }, { i32, void ()*, i8* } { i32 0, void ()* @[[REGFN]], i8* bitcast (void ()* @[[REGFN]] to i8*) }]
 
 
 template
Index: test/OpenMP/target_teams_num_teams_codegen.cpp
===
--- test/OpenMP/target_teams_num_teams_codegen.cpp
+++ test/OpenMP/target_teams_num_teams_codegen.cpp
@@ -76,7 +76,7 @@
 // CHECK: [[DESC:@.+]] = internal constant [[DSCTY]] { i32 1, [[DEVTY]]* getelementptr inbounds ([1 x [[DEVTY]]], [1 x [[DEVTY]]]* [[IMAGES]], i32 0, i32 0), [[ENTTY]]* [[ENTBEGIN]], [[ENTTY]]* [[ENTEND]] }, comdat($[[REGFN]])
 
 // Check target registration is registered as a Ctor.
-// CHECK: appending global [1 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 0, void ()* @[[REGFN]], i8* bitcast (void ()* @[[REGFN]] to i8*) }]
+// CHECK: appending global [2 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 0, void ()* @.omp_offloading.requires_reg, i8* null }, { i32, void ()*, i8* } { i32 0, void ()* @[[REGFN]], i8* bitcast (void ()* @[[REGFN]] to i8*) }]
 
 
 template
Index: test/OpenMP/target_teams_distribute_simd_depend_codegen.cpp
===
--- test/OpenMP/target_teams_distribute_simd_depend_codegen.cpp
+++ test/OpenMP/target_teams_distribute_simd_depend_codegen.cpp
@@ -64,7 +64,7 @@
 // CHECK: [[DESC:@.+]] = internal constant [[DSCTY]] { i32 1, [[DEVTY]]* getelementptr inbounds ([1 x [[DEVTY]]], [1 x [[DEVTY]]]* [[IMAGES]], i32 0, i32 0), [[ENTTY]]* [[ENTBEGIN]], [[ENTTY]]* [[ENTEND]] }, comdat($[[REGFN]])
 
 // Check target registration is registered as a Ctor.
-// CHECK: appending global [1 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 0, void ()* @[[REGFN]], i8* bitcast (void ()* @[[REGFN]] to i8*) }]
+// CHECK: appending global [2 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 0, void ()* @.omp_offloading.requires_reg, i8* null }, { i32, void ()*, i8* } { i32 0, void ()* @[[REGFN]], i8* bitcast (void ()* @[[REGFN]] to i8*) }]
 
 
 template
Index: test/Op

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

[Gheorghe-Teodor Bercea via Phabricator via cfe-commits]

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
  lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
  lib/CodeGen/CGOpenMPRuntimeNVPTX.h
  lib/CodeGen/CodeGenModule.cpp
  test/OpenMP/openmp_offload_registration.cpp
  test/OpenMP/target_codegen.cpp
  test/OpenMP/target_codegen_registration.cpp
  test/OpenMP/target_depend_codegen.cpp
  test/OpenMP/target_parallel_codegen.cpp
  test/OpenMP/target_parallel_codegen_registration.cpp
  test/OpenMP/target_parallel_depend_codegen.cpp
  test/OpenMP/target_parallel_for_codegen.cpp
  test/OpenMP/target_parallel_for_codegen_registration.cpp
  test/OpenMP/target_parallel_for_depend_codegen.cpp
  test/OpenMP/target_parallel_for_simd_codegen.cpp
  test/OpenMP/target_parallel_for_simd_codegen_registration.cpp
  test/OpenMP/target_parallel_for_simd_depend_codegen.cpp
  test/OpenMP/target_parallel_if_codegen.cpp
  test/OpenMP/target_parallel_num_threads_codegen.cpp
  test/OpenMP/target_simd_codegen.cpp
  test/OpenMP/target_simd_codegen_registration.cpp
  test/OpenMP/target_simd_depend_codegen.cpp
  test/OpenMP/target_teams_codegen.cpp
  test/OpenMP/target_teams_codegen_registration.cpp
  test/OpenMP/target_teams_depend_codegen.cpp
  test/OpenMP/target_teams_distribute_codegen.cpp
  test/OpenMP/target_teams_distribute_codegen_registration.cpp
  test/OpenMP/target_teams_distribute_depend_codegen.cpp
  test/OpenMP/target_teams_distribute_parallel_for_depend_codegen.cpp
  test/OpenMP/target_teams_distribute_parallel_for_simd_codegen_registration.cpp
  test/OpenMP/target_teams_distribute_parallel_for_simd_depend_codegen.cpp
  test/OpenMP/target_teams_distribute_simd_codegen.cpp
  test/OpenMP/target_teams_distribute_simd_codegen_registration.cpp
  test/OpenMP/target_teams_distribute_simd_depend_codegen.cpp
  test/OpenMP/target_teams_num_teams_codegen.cpp
  test/OpenMP/target_teams_thread_limit_codegen.cpp

Index: test/OpenMP/target_teams_thread_limit_codegen.cpp
===
--- test/OpenMP/target_teams_thread_limit_codegen.cpp
+++ test/OpenMP/target_teams_thread_limit_codegen.cpp
@@ -76,7 +76,7 @@
 // CHECK: [[DESC:@.+]] = internal constant [[DSCTY]] { i32 1, [[DEVTY]]* getelementptr inbounds ([1 x [[DEVTY]]], [1 x [[DEVTY]]]* [[IMAGES]], i32 0, i32 0), [[ENTTY]]* [[ENTBEGIN]], [[ENTTY]]* [[ENTEND]] }, comdat($[[REGFN]])
 
 // Check target registration is registered as a Ctor.
-// CHECK: appending global [1 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 0, void ()* @[[REGFN]], i8* bitcast (void ()* @[[REGFN]] to i8*) }]
+// CHECK: appending global [2 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 0, void ()* @.omp_offloading.requires_reg, i8* null }, { i32, void ()*, i8* } { i32 0, void ()* @[[REGFN]], i8* bitcast (void ()* @[[REGFN]] to i8*) }]
 
 
 template
Index: test/OpenMP/target_teams_num_teams_codegen.cpp
===
--- test/OpenMP/target_teams_num_teams_codegen.cpp
+++ test/OpenMP/target_teams_num_teams_codegen.cpp
@@ -76,7 +76,7 @@
 // CHECK: [[DESC:@.+]] = internal constant [[DSCTY]] { i32 1, [[DEVTY]]* getelementptr inbounds ([1 x [[DEVTY]]], [1 x [[DEVTY]]]* [[IMAGES]], i32 0, i32 0), [[ENTTY]]* [[ENTBEGIN]], [[ENTTY]]* [[ENTEND]] }, comdat($[[REGFN]])
 
 // Check target registration is registered as a Ctor.
-// CHECK: appending global [1 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 0, void ()* @[[REGFN]], i8* bitcast (void ()* @[[REGFN]] to i8*) }]
+// CHECK: appending global [2 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 0, void ()* @.omp_offloading.requires_reg, i8* null }, { i32, void ()*, i8* } { i32 0, void ()* @[[REGFN]], i8* bitcast (void ()* @[[REGFN]] to i8*) }]
 
 
 template
Index: test/OpenMP/target_teams_distribute_simd_depend_codegen.cpp
===
--- test/OpenMP/target_teams_distribute_simd_depend_codegen.cpp
+++ test/OpenMP/target_teams_distribute_simd_depend_codegen.cpp
@@ -64,7 +64,7 @@
 // CHECK: [[DESC:@.+]] = internal constant [[DSCTY]] { i32 1, [[DEVTY]]* getelementptr inbounds ([1 x [[DEVTY]]], [1 x [[DEVTY]]]* [[IMAGES]], i32 0, i32 0), [[ENTTY]]* [[ENTBEGIN]], [[ENTTY]]* [[ENTEND]] }, comdat($[[REGFN]])
 
 // Check target registration is registered as a Ctor.
-// CHECK: appending global [1 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 0, void ()* @[[REGFN]], i8* bitcast (void ()* @[[REGFN]] to i8*) }]
+// CHECK: appending global [2 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 0, void ()* @.omp_offloading.requires_reg, i8* null }, { i32, void ()*, i8* } { i32 0, void ()* @[[REGFN]], i8* bitcast (void ()* @[[REGFN]] to i8*) }]
 
 
 template
Index: test/OpenMP/target_teams_distribute_simd_codeg

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

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.



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

Missed check for `HasEmittedTargetRegion` here



Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:9312
+// contain at least 1 target region.
+if ((HasEmittedTargetRegion || HasEmittedDeclareTargetRegion) &&
+HasRequiresUnifiedSharedMemory)

The first part of the condition is excessive. You have early exit from the 
function. Instead of the condition, use 
`assert((!OffloadEntriesInfoManager.empty() || HasEmittedDeclareTargetRegion || 
HasEmittedTargetRegion) && "Expected bla-bla-bla");`



Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:10364
+  if (const auto *FD = dyn_cast(D)) {
+if (OMPDeclareTargetDeclAttr::isDeclareTargetDeclaration(FD)) {
+  HasEmittedDeclareTargetRegion = true;

gtbercea wrote:
> ABataev wrote:
> > gtbercea wrote:
> > > ABataev wrote:
> > > > gtbercea wrote:
> > > > > ABataev wrote:
> > > > > > ABataev wrote:
> > > > > > > No need for the braces
> > > > > > What if `declare target` is used only for variabes but not for the 
> > > > > > functions?
> > > > > Even more reason to error in that case since it may contain clauses 
> > > > > like link or to which need for requires directives to be used 
> > > > > consistently.
> > > > But I don't see that your patch works in this situation. Currently, it 
> > > > will emit the error only if the declare target function is found, no?
> > > Actually it will work even when just variables are used in the declare 
> > > target region. There is another problem which I have a fix for. I will 
> > > update the patch in a bit. 
> > Why will it work in this case? If you have just a declare target region in 
> > the code for the variables and nothing else. You don't have target regions, 
> > declare target functions etc. It won't work in this case.
> It will work because the OffloadEntriesInfoManager.empty() will return false 
> in that case.
But you don't have a check for `OffloadEntriesInfoManager.empty() ` when you 
set `Flags` for `__tgt_register_requires` function call.


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
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Gheorghe-Teodor Bercea via Phabricator via cfe-commits]

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;

ABataev wrote:
> gtbercea wrote:
> > ABataev wrote:
> > > gtbercea wrote:
> > > > ABataev wrote:
> > > > > ABataev wrote:
> > > > > > No need for the braces
> > > > > What if `declare target` is used only for variabes but not for the 
> > > > > functions?
> > > > Even more reason to error in that case since it may contain clauses 
> > > > like link or to which need for requires directives to be used 
> > > > consistently.
> > > But I don't see that your patch works in this situation. Currently, it 
> > > will emit the error only if the declare target function is found, no?
> > Actually it will work even when just variables are used in the declare 
> > target region. There is another problem which I have a fix for. I will 
> > update the patch in a bit. 
> Why will it work in this case? If you have just a declare target region in 
> the code for the variables and nothing else. You don't have target regions, 
> declare target functions etc. It won't work in this case.
It will work because the OffloadEntriesInfoManager.empty() will return false in 
that case.


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
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Alexey Bataev via Phabricator via cfe-commits]

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 wrote:
> > gtbercea wrote:
> > > ABataev wrote:
> > > > ABataev wrote:
> > > > > No need for the braces
> > > > What if `declare target` is used only for variabes but not for the 
> > > > functions?
> > > Even more reason to error in that case since it may contain clauses like 
> > > link or to which need for requires directives to be used consistently.
> > But I don't see that your patch works in this situation. Currently, it will 
> > emit the error only if the declare target function is found, no?
> Actually it will work even when just variables are used in the declare target 
> region. There is another problem which I have a fix for. I will update the 
> patch in a bit. 
Why will it work in this case? If you have just a declare target region in the 
code for the variables and nothing else. You don't have target regions, declare 
target functions etc. It won't work in this case.


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
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Gheorghe-Teodor Bercea via Phabricator via cfe-commits]

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;

ABataev wrote:
> gtbercea wrote:
> > ABataev wrote:
> > > ABataev wrote:
> > > > No need for the braces
> > > What if `declare target` is used only for variabes but not for the 
> > > functions?
> > Even more reason to error in that case since it may contain clauses like 
> > link or to which need for requires directives to be used consistently.
> But I don't see that your patch works in this situation. Currently, it will 
> emit the error only if the declare target function is found, no?
Actually it will work even when just variables are used in the declare target 
region. There is another problem which I have a fix for. I will update the 
patch in a bit. 


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
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Alexey Bataev via Phabricator via cfe-commits]

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 wrote:
> > ABataev wrote:
> > > No need for the braces
> > What if `declare target` is used only for variabes but not for the 
> > functions?
> Even more reason to error in that case since it may contain clauses like link 
> or to which need for requires directives to be used consistently.
But I don't see that your patch works in this situation. Currently, it will 
emit the error only if the declare target function is found, no?


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
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Gheorghe-Teodor Bercea via Phabricator via cfe-commits]

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;

ABataev wrote:
> ABataev wrote:
> > No need for the braces
> What if `declare target` is used only for variabes but not for the functions?
Even more reason to error in that case since it may contain clauses like link 
or to which need for requires directives to be used consistently.


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
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.



Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:10362
 
+void CGOpenMPRuntime::emitFunctionProlog(CodeGenFunction &CGF, const Decl *D){
+  if (const auto *FD = dyn_cast(D)) {

Bad formatting.



Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:10364
+  if (const auto *FD = dyn_cast(D)) {
+if (OMPDeclareTargetDeclAttr::isDeclareTargetDeclaration(FD)) {
+  HasEmittedDeclareTargetRegion = true;

No need for the braces



Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:10364
+  if (const auto *FD = dyn_cast(D)) {
+if (OMPDeclareTargetDeclAttr::isDeclareTargetDeclaration(FD)) {
+  HasEmittedDeclareTargetRegion = true;

ABataev wrote:
> No need for the braces
What if `declare target` is used only for variabes but not for the functions?


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
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Gheorghe-Teodor Bercea via Phabricator via cfe-commits]

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
  lib/CodeGen/CGOpenMPRuntime.h
  lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
  lib/CodeGen/CGOpenMPRuntimeNVPTX.h
  lib/CodeGen/CodeGenModule.cpp
  test/OpenMP/openmp_offload_registration.cpp
  test/OpenMP/target_codegen.cpp
  test/OpenMP/target_codegen_registration.cpp
  test/OpenMP/target_depend_codegen.cpp
  test/OpenMP/target_parallel_codegen.cpp
  test/OpenMP/target_parallel_codegen_registration.cpp
  test/OpenMP/target_parallel_depend_codegen.cpp
  test/OpenMP/target_parallel_for_codegen.cpp
  test/OpenMP/target_parallel_for_codegen_registration.cpp
  test/OpenMP/target_parallel_for_depend_codegen.cpp
  test/OpenMP/target_parallel_for_simd_codegen.cpp
  test/OpenMP/target_parallel_for_simd_codegen_registration.cpp
  test/OpenMP/target_parallel_for_simd_depend_codegen.cpp
  test/OpenMP/target_parallel_if_codegen.cpp
  test/OpenMP/target_parallel_num_threads_codegen.cpp
  test/OpenMP/target_simd_codegen.cpp
  test/OpenMP/target_simd_codegen_registration.cpp
  test/OpenMP/target_simd_depend_codegen.cpp
  test/OpenMP/target_teams_codegen.cpp
  test/OpenMP/target_teams_codegen_registration.cpp
  test/OpenMP/target_teams_depend_codegen.cpp
  test/OpenMP/target_teams_distribute_codegen.cpp
  test/OpenMP/target_teams_distribute_codegen_registration.cpp
  test/OpenMP/target_teams_distribute_depend_codegen.cpp
  test/OpenMP/target_teams_distribute_parallel_for_depend_codegen.cpp
  test/OpenMP/target_teams_distribute_parallel_for_simd_codegen_registration.cpp
  test/OpenMP/target_teams_distribute_parallel_for_simd_depend_codegen.cpp
  test/OpenMP/target_teams_distribute_simd_codegen.cpp
  test/OpenMP/target_teams_distribute_simd_codegen_registration.cpp
  test/OpenMP/target_teams_distribute_simd_depend_codegen.cpp
  test/OpenMP/target_teams_num_teams_codegen.cpp
  test/OpenMP/target_teams_thread_limit_codegen.cpp

Index: test/OpenMP/target_teams_thread_limit_codegen.cpp
===
--- test/OpenMP/target_teams_thread_limit_codegen.cpp
+++ test/OpenMP/target_teams_thread_limit_codegen.cpp
@@ -76,7 +76,7 @@
 // CHECK: [[DESC:@.+]] = internal constant [[DSCTY]] { i32 1, [[DEVTY]]* getelementptr inbounds ([1 x [[DEVTY]]], [1 x [[DEVTY]]]* [[IMAGES]], i32 0, i32 0), [[ENTTY]]* [[ENTBEGIN]], [[ENTTY]]* [[ENTEND]] }, comdat($[[REGFN]])
 
 // Check target registration is registered as a Ctor.
-// CHECK: appending global [1 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 0, void ()* @[[REGFN]], i8* bitcast (void ()* @[[REGFN]] to i8*) }]
+// CHECK: appending global [2 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 0, void ()* @.omp_offloading.requires_reg, i8* null }, { i32, void ()*, i8* } { i32 0, void ()* @[[REGFN]], i8* bitcast (void ()* @[[REGFN]] to i8*) }]
 
 
 template
Index: test/OpenMP/target_teams_num_teams_codegen.cpp
===
--- test/OpenMP/target_teams_num_teams_codegen.cpp
+++ test/OpenMP/target_teams_num_teams_codegen.cpp
@@ -76,7 +76,7 @@
 // CHECK: [[DESC:@.+]] = internal constant [[DSCTY]] { i32 1, [[DEVTY]]* getelementptr inbounds ([1 x [[DEVTY]]], [1 x [[DEVTY]]]* [[IMAGES]], i32 0, i32 0), [[ENTTY]]* [[ENTBEGIN]], [[ENTTY]]* [[ENTEND]] }, comdat($[[REGFN]])
 
 // Check target registration is registered as a Ctor.
-// CHECK: appending global [1 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 0, void ()* @[[REGFN]], i8* bitcast (void ()* @[[REGFN]] to i8*) }]
+// CHECK: appending global [2 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 0, void ()* @.omp_offloading.requires_reg, i8* null }, { i32, void ()*, i8* } { i32 0, void ()* @[[REGFN]], i8* bitcast (void ()* @[[REGFN]] to i8*) }]
 
 
 template
Index: test/OpenMP/target_teams_distribute_simd_depend_codegen.cpp
===
--- test/OpenMP/target_teams_distribute_simd_depend_codegen.cpp
+++ test/OpenMP/target_teams_distribute_simd_depend_codegen.cpp
@@ -64,7 +64,7 @@
 // CHECK: [[DESC:@.+]] = internal constant [[DSCTY]] { i32 1, [[DEVTY]]* getelementptr inbounds ([1 x [[DEVTY]]], [1 x [[DEVTY]]]* [[IMAGES]], i32 0, i32 0), [[ENTTY]]* [[ENTBEGIN]], [[ENTTY]]* [[ENTEND]] }, comdat($[[REGFN]])
 
 // Check target registration is registered as a Ctor.
-// CHECK: appending global [1 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 0, void ()* @[[REGFN]], i8* bitcast (void ()* @[[REGFN]] to i8*) }]
+// CHECK: appending global [2 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 0, void ()* @.omp_offloading.requires_reg, i8* null }, { i32, void ()*, i8* } { i32 0, void ()* @[[REGFN]], i8* bitcast (void ()* @[[REGFN]] to i8*) }]
 
 
 template
Index: test/Ope

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

[Alexey Bataev via Phabricator via cfe-commits]

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



Comment at: test/OpenMP/target_codegen.cpp:99
 // Check target registration is registered as a Ctor.
-// CHECK: appending global [1 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* 
} { i32 0, void ()* @[[REGFN]], i8* bitcast (void ()* @[[REGFN]] to i8*) }]
+// CHECK: appending global [2 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* 
} { i32 0, void ()* @.omp_offloading.requires_reg, i8* null }, { i32, void ()*, 
i8* } { i32 0, void ()* @[[REGFN]], i8* bitcast (void ()* @[[REGFN]] to i8*) }]
 

Add a check for function too


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
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Gheorghe-Teodor Bercea via Phabricator via cfe-commits]

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
  lib/CodeGen/CGOpenMPRuntime.h
  lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
  lib/CodeGen/CGOpenMPRuntimeNVPTX.h
  lib/CodeGen/CodeGenModule.cpp
  test/OpenMP/openmp_offload_registration.cpp
  test/OpenMP/target_codegen.cpp
  test/OpenMP/target_codegen_registration.cpp

Index: test/OpenMP/target_codegen_registration.cpp
===
--- test/OpenMP/target_codegen_registration.cpp
+++ test/OpenMP/target_codegen_registration.cpp
@@ -180,10 +180,11 @@
 // CHECK: [[DESC:@.+]] = internal constant [[DSCTY]] { i32 1, [[DEVTY]]* getelementptr inbounds ([1 x [[DEVTY]]], [1 x [[DEVTY]]]* [[IMAGES]], i32 0, i32 0), [[ENTTY]]* [[ENTBEGIN]], [[ENTTY]]* [[ENTEND]] }, comdat($[[REGFN]])
 
 // We have 4 initializers, one for the 500 priority, another one for 501, or more for the default priority, and the last one for the offloading registration function.
-// CHECK: @llvm.global_ctors = appending global [4 x { i32, void ()*, i8* }] [
+// CHECK: @llvm.global_ctors = appending global [5 x { i32, void ()*, i8* }] [
 // CHECK-SAME: { i32, void ()*, i8* } { i32 500, void ()* [[P500:@[^,]+]], i8* null },
 // CHECK-SAME: { i32, void ()*, i8* } { i32 501, void ()* [[P501:@[^,]+]], i8* null },
 // CHECK-SAME: { i32, void ()*, i8* } { i32 65535, void ()* [[PMAX:@[^,]+]], i8* null },
+// CHECK-SAME: { i32, void ()*, i8* } { i32 0, void ()* @.omp_offloading.requires_reg, i8* null },
 // CHECK-SAME: { i32, void ()*, i8* } { i32 0, void ()* @[[REGFN]], i8* bitcast (void ()* @[[REGFN]] to i8*) }]
 
 // CHECK-NTARGET: @llvm.global_ctors = appending global [3   x { i32, void ()*, i8* }] [
@@ -387,6 +388,10 @@
 
 // Check registration and unregistration
 
+//CHECK: define internal void @.omp_offloading.requires_reg()
+//CHECK: call void @__tgt_register_requires(i64 1)
+//CHECK: ret void
+
 //CHECK: define internal void @[[UNREGFN:.+]](i8*)
 //CHECK-SAME: comdat($[[REGFN]]) {
 //CHECK: call i32 @__tgt_unregister_lib([[DSCTY]]* [[DESC]])
@@ -432,31 +437,31 @@
 
 // Check metadata is properly generated:
 // CHECK: !omp_offload.info = !{!{{[0-9]+}}, !{{[0-9]+}}, !{{[0-9]+}}, !{{[0-9]+}}, !{{[0-9]+}}, !{{[0-9]+}}, !{{[0-9]+}}, !{{[0-9]+}}, !{{[0-9]+}}, !{{[0-9]+}}, !{{[0-9]+}}, !{{[0-9]+}}}
-// CHECK-DAG: = !{i32 0, i32 [[DEVID:-?[0-9]+]], i32 [[FILEID:-?[0-9]+]], !"_ZN2SB3fooEv", i32 216, i32 {{[0-9]+}}}
-// CHECK-DAG: = !{i32 0, i32 [[DEVID]], i32 [[FILEID]], !"_ZN2SDD1Ev", i32 266, i32 {{[0-9]+}}}
-// CHECK-DAG: = !{i32 0, i32 [[DEVID]], i32 [[FILEID]], !"_ZN2SEC1Ev", i32 282, i32 {{[0-9]+}}}
-// CHECK-DAG: = !{i32 0, i32 [[DEVID]], i32 [[FILEID]], !"_ZN2SED1Ev", i32 288, i32 {{[0-9]+}}}
-// CHECK-DAG: = !{i32 0, i32 [[DEVID]], i32 [[FILEID]], !"_ZN2STILi1000EE3fooEv", i32 299, i32 {{[0-9]+}}}
-// CHECK-DAG: = !{i32 0, i32 [[DEVID]], i32 [[FILEID]], !"_ZN2STILi100EEC1Ev", i32 305, i32 {{[0-9]+}}}
-// CHECK-DAG: = !{i32 0, i32 [[DEVID]], i32 [[FILEID]], !"_Z3bari", i32 427, i32 {{[0-9]+}}}
-// CHECK-DAG: = !{i32 0, i32 [[DEVID]], i32 [[FILEID]], !"_ZN2STILi100EED1Ev", i32 311, i32 {{[0-9]+}}}
-// CHECK-DAG: = !{i32 0, i32 [[DEVID]], i32 [[FILEID]], !"_ZN2STILi1000EEC1Ev", i32 305, i32 {{[0-9]+}}}
-// CHECK-DAG: = !{i32 0, i32 [[DEVID]], i32 [[FILEID]], !"_ZN2STILi1000EED1Ev", i32 311, i32 {{[0-9]+}}}
-// CHECK-DAG: = !{i32 0, i32 [[DEVID]], i32 [[FILEID]], !"_ZN2STILi100EE3fooEv", i32 299, i32 {{[0-9]+}}}
-// CHECK-DAG: = !{i32 0, i32 [[DEVID]], i32 [[FILEID]], !"_ZN2SCC1Ev", i32 241, i32 {{[0-9]+}}}
+// CHECK-DAG: = !{i32 0, i32 [[DEVID:-?[0-9]+]], i32 [[FILEID:-?[0-9]+]], !"_ZN2SB3fooEv", i32 217, i32 {{[0-9]+}}}
+// CHECK-DAG: = !{i32 0, i32 [[DEVID]], i32 [[FILEID]], !"_ZN2SDD1Ev", i32 267, i32 {{[0-9]+}}}
+// CHECK-DAG: = !{i32 0, i32 [[DEVID]], i32 [[FILEID]], !"_ZN2SEC1Ev", i32 283, i32 {{[0-9]+}}}
+// CHECK-DAG: = !{i32 0, i32 [[DEVID]], i32 [[FILEID]], !"_ZN2SED1Ev", i32 289, i32 {{[0-9]+}}}
+// CHECK-DAG: = !{i32 0, i32 [[DEVID]], i32 [[FILEID]], !"_ZN2STILi1000EE3fooEv", i32 300, i32 {{[0-9]+}}}
+// CHECK-DAG: = !{i32 0, i32 [[DEVID]], i32 [[FILEID]], !"_ZN2STILi100EEC1Ev", i32 306, i32 {{[0-9]+}}}
+// CHECK-DAG: = !{i32 0, i32 [[DEVID]], i32 [[FILEID]], !"_Z3bari", i32 432, i32 {{[0-9]+}}}
+// CHECK-DAG: = !{i32 0, i32 [[DEVID]], i32 [[FILEID]], !"_ZN2STILi100EED1Ev", i32 312, i32 {{[0-9]+}}}
+// CHECK-DAG: = !{i32 0, i32 [[DEVID]], i32 [[FILEID]], !"_ZN2STILi1000EEC1Ev", i32 306, i32 {{[0-9]+}}}
+// CHECK-DAG: = !{i32 0, i32 [[DEVID]], i32 [[FILEID]], !"_ZN2STILi1000EED1Ev", i32 312, i32 {{[0-9]+}}}
+// CHECK-DAG: = !{i32 0, i32 [[DEVID]], i32 [[FILEID]], !"_ZN2STILi100EE3fooEv", i32 300, i32 {{[0-9]+}}}
+// CHECK-DAG: = !{i32 0, i32 [[DEVID]], i32 [[FILEID]], !"_ZN2SCC1Ev", i32 242, i32 {{[0-9]

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

[Alexey Bataev via Phabricator via cfe-commits]

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, sema part must be separate.


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
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Gheorghe-Teodor Bercea via Phabricator via cfe-commits]

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
  lib/CodeGen/CGOpenMPRuntime.cpp
  lib/CodeGen/CGOpenMPRuntime.h
  lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
  lib/CodeGen/CGOpenMPRuntimeNVPTX.h
  lib/CodeGen/CodeGenModule.cpp
  lib/Sema/SemaOpenMP.cpp
  test/OpenMP/openmp_offload_registration.cpp
  test/OpenMP/target_codegen.cpp
  test/OpenMP/target_codegen_registration.cpp

Index: test/OpenMP/target_codegen_registration.cpp
===
--- test/OpenMP/target_codegen_registration.cpp
+++ test/OpenMP/target_codegen_registration.cpp
@@ -180,10 +180,11 @@
 // CHECK: [[DESC:@.+]] = internal constant [[DSCTY]] { i32 1, [[DEVTY]]* getelementptr inbounds ([1 x [[DEVTY]]], [1 x [[DEVTY]]]* [[IMAGES]], i32 0, i32 0), [[ENTTY]]* [[ENTBEGIN]], [[ENTTY]]* [[ENTEND]] }, comdat($[[REGFN]])
 
 // We have 4 initializers, one for the 500 priority, another one for 501, or more for the default priority, and the last one for the offloading registration function.
-// CHECK: @llvm.global_ctors = appending global [4 x { i32, void ()*, i8* }] [
+// CHECK: @llvm.global_ctors = appending global [5 x { i32, void ()*, i8* }] [
 // CHECK-SAME: { i32, void ()*, i8* } { i32 500, void ()* [[P500:@[^,]+]], i8* null },
 // CHECK-SAME: { i32, void ()*, i8* } { i32 501, void ()* [[P501:@[^,]+]], i8* null },
 // CHECK-SAME: { i32, void ()*, i8* } { i32 65535, void ()* [[PMAX:@[^,]+]], i8* null },
+// CHECK-SAME: { i32, void ()*, i8* } { i32 0, void ()* @.omp_offloading.requires_reg, i8* null },
 // CHECK-SAME: { i32, void ()*, i8* } { i32 0, void ()* @[[REGFN]], i8* bitcast (void ()* @[[REGFN]] to i8*) }]
 
 // CHECK-NTARGET: @llvm.global_ctors = appending global [3   x { i32, void ()*, i8* }] [
@@ -387,6 +388,10 @@
 
 // Check registration and unregistration
 
+//CHECK: define internal void @.omp_offloading.requires_reg()
+//CHECK: call void @__tgt_register_requires(i64 1)
+//CHECK: ret void
+
 //CHECK: define internal void @[[UNREGFN:.+]](i8*)
 //CHECK-SAME: comdat($[[REGFN]]) {
 //CHECK: call i32 @__tgt_unregister_lib([[DSCTY]]* [[DESC]])
@@ -432,31 +437,31 @@
 
 // Check metadata is properly generated:
 // CHECK: !omp_offload.info = !{!{{[0-9]+}}, !{{[0-9]+}}, !{{[0-9]+}}, !{{[0-9]+}}, !{{[0-9]+}}, !{{[0-9]+}}, !{{[0-9]+}}, !{{[0-9]+}}, !{{[0-9]+}}, !{{[0-9]+}}, !{{[0-9]+}}, !{{[0-9]+}}}
-// CHECK-DAG: = !{i32 0, i32 [[DEVID:-?[0-9]+]], i32 [[FILEID:-?[0-9]+]], !"_ZN2SB3fooEv", i32 216, i32 {{[0-9]+}}}
-// CHECK-DAG: = !{i32 0, i32 [[DEVID]], i32 [[FILEID]], !"_ZN2SDD1Ev", i32 266, i32 {{[0-9]+}}}
-// CHECK-DAG: = !{i32 0, i32 [[DEVID]], i32 [[FILEID]], !"_ZN2SEC1Ev", i32 282, i32 {{[0-9]+}}}
-// CHECK-DAG: = !{i32 0, i32 [[DEVID]], i32 [[FILEID]], !"_ZN2SED1Ev", i32 288, i32 {{[0-9]+}}}
-// CHECK-DAG: = !{i32 0, i32 [[DEVID]], i32 [[FILEID]], !"_ZN2STILi1000EE3fooEv", i32 299, i32 {{[0-9]+}}}
-// CHECK-DAG: = !{i32 0, i32 [[DEVID]], i32 [[FILEID]], !"_ZN2STILi100EEC1Ev", i32 305, i32 {{[0-9]+}}}
-// CHECK-DAG: = !{i32 0, i32 [[DEVID]], i32 [[FILEID]], !"_Z3bari", i32 427, i32 {{[0-9]+}}}
-// CHECK-DAG: = !{i32 0, i32 [[DEVID]], i32 [[FILEID]], !"_ZN2STILi100EED1Ev", i32 311, i32 {{[0-9]+}}}
-// CHECK-DAG: = !{i32 0, i32 [[DEVID]], i32 [[FILEID]], !"_ZN2STILi1000EEC1Ev", i32 305, i32 {{[0-9]+}}}
-// CHECK-DAG: = !{i32 0, i32 [[DEVID]], i32 [[FILEID]], !"_ZN2STILi1000EED1Ev", i32 311, i32 {{[0-9]+}}}
-// CHECK-DAG: = !{i32 0, i32 [[DEVID]], i32 [[FILEID]], !"_ZN2STILi100EE3fooEv", i32 299, i32 {{[0-9]+}}}
-// CHECK-DAG: = !{i32 0, i32 [[DEVID]], i32 [[FILEID]], !"_ZN2SCC1Ev", i32 241, i32 {{[0-9]+}}}
+// CHECK-DAG: = !{i32 0, i32 [[DEVID:-?[0-9]+]], i32 [[FILEID:-?[0-9]+]], !"_ZN2SB3fooEv", i32 217, i32 {{[0-9]+}}}
+// CHECK-DAG: = !{i32 0, i32 [[DEVID]], i32 [[FILEID]], !"_ZN2SDD1Ev", i32 267, i32 {{[0-9]+}}}
+// CHECK-DAG: = !{i32 0, i32 [[DEVID]], i32 [[FILEID]], !"_ZN2SEC1Ev", i32 283, i32 {{[0-9]+}}}
+// CHECK-DAG: = !{i32 0, i32 [[DEVID]], i32 [[FILEID]], !"_ZN2SED1Ev", i32 289, i32 {{[0-9]+}}}
+// CHECK-DAG: = !{i32 0, i32 [[DEVID]], i32 [[FILEID]], !"_ZN2STILi1000EE3fooEv", i32 300, i32 {{[0-9]+}}}
+// CHECK-DAG: = !{i32 0, i32 [[DEVID]], i32 [[FILEID]], !"_ZN2STILi100EEC1Ev", i32 306, i32 {{[0-9]+}}}
+// CHECK-DAG: = !{i32 0, i32 [[DEVID]], i32 [[FILEID]], !"_Z3bari", i32 432, i32 {{[0-9]+}}}
+// CHECK-DAG: = !{i32 0, i32 [[DEVID]], i32 [[FILEID]], !"_ZN2STILi100EED1Ev", i32 312, i32 {{[0-9]+}}}
+// CHECK-DAG: = !{i32 0, i32 [[DEVID]], i32 [[FILEID]], !"_ZN2STILi1000EEC1Ev", i32 306, i32 {{[0-9]+}}}
+// CHECK-DAG: = !{i32 0, i32 [[DEVID]], i32 [[FILEID]], !"_ZN2STILi1000EED1Ev", i32 312, i32 {{[0-9]+}}}
+// CHECK-DAG: = !{i32 0, i32 [[DEVID]], i32 [[FILEID]], !"_ZN2STILi100EE3fooEv", i32 300, i32 {{[0-9]+}}}
+// CHECK-DAG: =

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

[Alexey Bataev via Phabricator via cfe-commits]

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 diagnostic emission, it serves as the 
> > container. Better to move the diagnostics to Act... functions.
> The "bool hasDuplicateRequiresClause(ArrayRef ClauseList) const 
> {" function works in a similar function as this one and does emit 
> diagnostics. See a few lines above.
> 
> I need to iterate through a private member of DSAStack so I don't see how 
> that can be in any other place than in DSAStack.
1. `hasDuplicateRequiresClause` must be fixed.
2. You can return ArrayRef and iterate over it in the Act.. 
function


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
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Gheorghe-Teodor Bercea via Phabricator via cfe-commits]

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 is not used for diagnostic emission, it serves as the 
> container. Better to move the diagnostics to Act... functions.
The "bool hasDuplicateRequiresClause(ArrayRef ClauseList) const {" 
function works in a similar function as this one and does emit diagnostics. See 
a few lines above.

I need to iterate through a private member of DSAStack so I don't see how that 
can be in any other place than in DSAStack.


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
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Alexey Bataev via Phabricator via cfe-commits]

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 (const auto *C : this->CurDir.getClausesOfKind()) {

ABataev wrote:
> Restore the original, this must be changed in a separate patch.
Restore



Comment at: lib/Sema/SemaOpenMP.cpp:196
   nullptr};
+  /// Vector of previously encountered target directives
+  SmallVector TargetLocations;

I would suggest to split this patch into 2 parts: the sema checking part and 
the codegen part.



Comment at: lib/Sema/SemaOpenMP.cpp:468
+  void checkEncounteredTargets(SourceLocation Loc,
+   ArrayRef ClauseList) {
+for (OMPClause *CNew : ClauseList) {

Make function `const`



Comment at: lib/Sema/SemaOpenMP.cpp:469
+   ArrayRef ClauseList) {
+for (OMPClause *CNew : ClauseList) {
+  // Check if any of the requires clauses affect target regions.

`const OMPClause *`



Comment at: lib/Sema/SemaOpenMP.cpp:476
+   isa(CNew))) {
+SemaRef.Diag(Loc,
+ diag::err_omp_target_before_requires)

DSAStack usually is not used for diagnostic emission, it serves as the 
container. Better to move the diagnostics to Act... functions.


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
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Gheorghe-Teodor Bercea via Phabricator via cfe-commits]

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
  lib/CodeGen/CGOpenMPRuntime.cpp
  lib/CodeGen/CGOpenMPRuntime.h
  lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
  lib/CodeGen/CGOpenMPRuntimeNVPTX.h
  lib/CodeGen/CodeGenModule.cpp
  lib/Sema/SemaOpenMP.cpp
  test/OpenMP/openmp_offload_registration.cpp
  test/OpenMP/target_codegen.cpp
  test/OpenMP/target_codegen_registration.cpp

Index: test/OpenMP/target_codegen_registration.cpp
===
--- test/OpenMP/target_codegen_registration.cpp
+++ test/OpenMP/target_codegen_registration.cpp
@@ -180,10 +180,11 @@
 // CHECK: [[DESC:@.+]] = internal constant [[DSCTY]] { i32 1, [[DEVTY]]* getelementptr inbounds ([1 x [[DEVTY]]], [1 x [[DEVTY]]]* [[IMAGES]], i32 0, i32 0), [[ENTTY]]* [[ENTBEGIN]], [[ENTTY]]* [[ENTEND]] }, comdat($[[REGFN]])
 
 // We have 4 initializers, one for the 500 priority, another one for 501, or more for the default priority, and the last one for the offloading registration function.
-// CHECK: @llvm.global_ctors = appending global [4 x { i32, void ()*, i8* }] [
+// CHECK: @llvm.global_ctors = appending global [5 x { i32, void ()*, i8* }] [
 // CHECK-SAME: { i32, void ()*, i8* } { i32 500, void ()* [[P500:@[^,]+]], i8* null },
 // CHECK-SAME: { i32, void ()*, i8* } { i32 501, void ()* [[P501:@[^,]+]], i8* null },
 // CHECK-SAME: { i32, void ()*, i8* } { i32 65535, void ()* [[PMAX:@[^,]+]], i8* null },
+// CHECK-SAME: { i32, void ()*, i8* } { i32 0, void ()* @.omp_offloading.requires_reg, i8* null },
 // CHECK-SAME: { i32, void ()*, i8* } { i32 0, void ()* @[[REGFN]], i8* bitcast (void ()* @[[REGFN]] to i8*) }]
 
 // CHECK-NTARGET: @llvm.global_ctors = appending global [3   x { i32, void ()*, i8* }] [
@@ -387,6 +388,10 @@
 
 // Check registration and unregistration
 
+//CHECK: define internal void @.omp_offloading.requires_reg()
+//CHECK: call void @__tgt_register_requires(i64 1)
+//CHECK: ret void
+
 //CHECK: define internal void @[[UNREGFN:.+]](i8*)
 //CHECK-SAME: comdat($[[REGFN]]) {
 //CHECK: call i32 @__tgt_unregister_lib([[DSCTY]]* [[DESC]])
@@ -432,31 +437,31 @@
 
 // Check metadata is properly generated:
 // CHECK: !omp_offload.info = !{!{{[0-9]+}}, !{{[0-9]+}}, !{{[0-9]+}}, !{{[0-9]+}}, !{{[0-9]+}}, !{{[0-9]+}}, !{{[0-9]+}}, !{{[0-9]+}}, !{{[0-9]+}}, !{{[0-9]+}}, !{{[0-9]+}}, !{{[0-9]+}}}
-// CHECK-DAG: = !{i32 0, i32 [[DEVID:-?[0-9]+]], i32 [[FILEID:-?[0-9]+]], !"_ZN2SB3fooEv", i32 216, i32 {{[0-9]+}}}
-// CHECK-DAG: = !{i32 0, i32 [[DEVID]], i32 [[FILEID]], !"_ZN2SDD1Ev", i32 266, i32 {{[0-9]+}}}
-// CHECK-DAG: = !{i32 0, i32 [[DEVID]], i32 [[FILEID]], !"_ZN2SEC1Ev", i32 282, i32 {{[0-9]+}}}
-// CHECK-DAG: = !{i32 0, i32 [[DEVID]], i32 [[FILEID]], !"_ZN2SED1Ev", i32 288, i32 {{[0-9]+}}}
-// CHECK-DAG: = !{i32 0, i32 [[DEVID]], i32 [[FILEID]], !"_ZN2STILi1000EE3fooEv", i32 299, i32 {{[0-9]+}}}
-// CHECK-DAG: = !{i32 0, i32 [[DEVID]], i32 [[FILEID]], !"_ZN2STILi100EEC1Ev", i32 305, i32 {{[0-9]+}}}
-// CHECK-DAG: = !{i32 0, i32 [[DEVID]], i32 [[FILEID]], !"_Z3bari", i32 427, i32 {{[0-9]+}}}
-// CHECK-DAG: = !{i32 0, i32 [[DEVID]], i32 [[FILEID]], !"_ZN2STILi100EED1Ev", i32 311, i32 {{[0-9]+}}}
-// CHECK-DAG: = !{i32 0, i32 [[DEVID]], i32 [[FILEID]], !"_ZN2STILi1000EEC1Ev", i32 305, i32 {{[0-9]+}}}
-// CHECK-DAG: = !{i32 0, i32 [[DEVID]], i32 [[FILEID]], !"_ZN2STILi1000EED1Ev", i32 311, i32 {{[0-9]+}}}
-// CHECK-DAG: = !{i32 0, i32 [[DEVID]], i32 [[FILEID]], !"_ZN2STILi100EE3fooEv", i32 299, i32 {{[0-9]+}}}
-// CHECK-DAG: = !{i32 0, i32 [[DEVID]], i32 [[FILEID]], !"_ZN2SCC1Ev", i32 241, i32 {{[0-9]+}}}
+// CHECK-DAG: = !{i32 0, i32 [[DEVID:-?[0-9]+]], i32 [[FILEID:-?[0-9]+]], !"_ZN2SB3fooEv", i32 217, i32 {{[0-9]+}}}
+// CHECK-DAG: = !{i32 0, i32 [[DEVID]], i32 [[FILEID]], !"_ZN2SDD1Ev", i32 267, i32 {{[0-9]+}}}
+// CHECK-DAG: = !{i32 0, i32 [[DEVID]], i32 [[FILEID]], !"_ZN2SEC1Ev", i32 283, i32 {{[0-9]+}}}
+// CHECK-DAG: = !{i32 0, i32 [[DEVID]], i32 [[FILEID]], !"_ZN2SED1Ev", i32 289, i32 {{[0-9]+}}}
+// CHECK-DAG: = !{i32 0, i32 [[DEVID]], i32 [[FILEID]], !"_ZN2STILi1000EE3fooEv", i32 300, i32 {{[0-9]+}}}
+// CHECK-DAG: = !{i32 0, i32 [[DEVID]], i32 [[FILEID]], !"_ZN2STILi100EEC1Ev", i32 306, i32 {{[0-9]+}}}
+// CHECK-DAG: = !{i32 0, i32 [[DEVID]], i32 [[FILEID]], !"_Z3bari", i32 432, i32 {{[0-9]+}}}
+// CHECK-DAG: = !{i32 0, i32 [[DEVID]], i32 [[FILEID]], !"_ZN2STILi100EED1Ev", i32 312, i32 {{[0-9]+}}}
+// CHECK-DAG: = !{i32 0, i32 [[DEVID]], i32 [[FILEID]], !"_ZN2STILi1000EEC1Ev", i32 306, i32 {{[0-9]+}}}
+// CHECK-DAG: = !{i32 0, i32 [[DEVID]], i32 [[FILEID]], !"_ZN2STILi1000EED1Ev", i32 312, i32 {{[0-9]+}}}
+// CHECK-DAG: = !{i32 0, i32 [[DEVID]], i32 [[FILEID]], !"_ZN2STILi100EE3fooEv", i32 300, i32 {{[0-9]+}}}
+// C

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

[Alexey Bataev via Phabricator via cfe-commits]

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.");
+  HasRequiresUnifiedSharedMemory = true;

gtbercea wrote:
> ABataev wrote:
> > The message speaks about target region but points to the clause. You need 
> > to correct the message.
> "A target region was emitted before this requires directive." ?
Still does not look good, better to add previously emitted target regions in 
the diagnostic somehow. Also, it would be good to mention the clause in the 
message.



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 (const auto *C : this->CurDir.getClausesOfKind()) {

Restore the original, this must be changed in a separate patch.



Comment at: test/OpenMP/openmp_offload_registration.cpp:40
+// CHECK: ret void
+// CHECK: declare void @__tgt_register_requires(i64)
+

Why do you need this check?


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
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Gheorghe-Teodor Bercea via Phabricator via cfe-commits]

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 = true;

ABataev wrote:
> The message speaks about target region but points to the clause. You need to 
> correct the message.
"A target region was emitted before this requires directive." ?


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
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Gheorghe-Teodor Bercea via Phabricator via cfe-commits]

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:
  lib/CodeGen/CGOpenMPRuntime.cpp
  lib/CodeGen/CGOpenMPRuntime.h
  lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
  lib/CodeGen/CGOpenMPRuntimeNVPTX.h
  lib/CodeGen/CodeGenModule.cpp
  test/OpenMP/openmp_offload_registration.cpp

Index: test/OpenMP/openmp_offload_registration.cpp
===
--- test/OpenMP/openmp_offload_registration.cpp
+++ test/OpenMP/openmp_offload_registration.cpp
@@ -26,7 +26,7 @@
 // CHECK: [[DESC:@.+]] = internal constant [[DSCTY]] { i32 2, [[DEVTY]]* getelementptr inbounds ([2 x [[DEVTY]]], [2 x [[DEVTY]]]* [[IMAGES]], i32 0, i32 0), [[ENTTY]]* [[ENTBEGIN]], [[ENTTY]]* [[ENTEND]] }, comdat($[[REGFN]])
 
 // Check target registration is registered as a Ctor.
-// CHECK: appending global [1 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 0, void ()* @[[REGFN]], i8* bitcast (void ()* @[[REGFN]] to i8*) }]
+// CHECK: appending global [2 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 0, void ()* @.omp_offloading.requires_reg, i8* null }, { i32, void ()*, i8* } { i32 0, void ()* @[[REGFN]], i8* bitcast (void ()* @[[REGFN]] to i8*) }]
 
 // Check presence of foo() and the outlined target region
 // CHECK: define void [[FOO:@.+]]()
@@ -34,6 +34,11 @@
 
 // Check registration and unregistration code.
 
+// CHECK: define internal void @.omp_offloading.requires_reg()
+// CHECK: call void @__tgt_register_requires(i64 0)
+// CHECK: ret void
+// CHECK: declare void @__tgt_register_requires(i64)
+
 // CHECK: define internal void @[[UNREGFN:.+]](i8*)
 // CHECK-SAME: comdat($[[REGFN]]) {
 // CHECK: call i32 @__tgt_unregister_lib([[DSCTY]]* [[DESC]])
Index: lib/CodeGen/CodeGenModule.cpp
===
--- lib/CodeGen/CodeGenModule.cpp
+++ lib/CodeGen/CodeGenModule.cpp
@@ -410,6 +410,10 @@
   AddGlobalCtor(CudaCtorFunction);
   }
   if (OpenMPRuntime) {
+if (llvm::Function *OpenMPRequiresDirectiveRegFun =
+OpenMPRuntime->emitRequiresDirectiveRegFun()) {
+  AddGlobalCtor(OpenMPRequiresDirectiveRegFun, 0);
+}
 if (llvm::Function *OpenMPRegistrationFunction =
 OpenMPRuntime->emitRegistrationFunction()) {
   auto ComdatKey = OpenMPRegistrationFunction->hasComdat() ?
Index: lib/CodeGen/CGOpenMPRuntimeNVPTX.h
===
--- lib/CodeGen/CGOpenMPRuntimeNVPTX.h
+++ lib/CodeGen/CGOpenMPRuntimeNVPTX.h
@@ -383,7 +383,7 @@
 
   /// Perform check on requires decl to ensure that target architecture
   /// supports unified addressing
-  void checkArchForUnifiedAddressing(const OMPRequiresDecl *D) const override;
+  void checkArchForUnifiedAddressing(const OMPRequiresDecl *D) override;
 
   /// Returns default address space for the constant firstprivates, __constant__
   /// address space by default.
Index: lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
===
--- lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
+++ lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
@@ -4942,7 +4942,7 @@
 /// Check to see if target architecture supports unified addressing which is
 /// a restriction for OpenMP requires clause "unified_shared_memory".
 void CGOpenMPRuntimeNVPTX::checkArchForUnifiedAddressing(
-const OMPRequiresDecl *D) const {
+const OMPRequiresDecl *D) {
   for (const OMPClause *Clause : D->clauselists()) {
 if (Clause->getClauseKind() == OMPC_unified_shared_memory) {
   switch (getCudaArch(CGM)) {
@@ -4987,6 +4987,7 @@
   }
 }
   }
+  CGOpenMPRuntime::checkArchForUnifiedAddressing(D);
 }
 
 /// Get number of SMs and number of blocks per SM.
Index: lib/CodeGen/CGOpenMPRuntime.h
===
--- lib/CodeGen/CGOpenMPRuntime.h
+++ lib/CodeGen/CGOpenMPRuntime.h
@@ -636,6 +636,17 @@
   /// must be emitted.
   llvm::SmallDenseSet DeferredGlobalVariables;
 
+  /// Flag for keeping track of weather a requires unified_shared_memory
+  /// directive is present.
+  bool HasRequiresUnifiedSharedMemory = false;
+
+  /// Flag for keeping track of weather a target region has been emitted.
+  bool HasEmittedTargetRegion = false;
+
+  /// Flag for keeping track of weather a device routine has been emitted.
+  /// Device routines are specific to the
+  bool HasEmittedDeclareTargetRegion = false;
+
   /// Creates and registers offloading binary descriptor for the current
   /// compilation unit. The function that does the registration is returned.
   llvm::Function *createOffloadingBinaryDescriptorRegistration();
@@ -1429,6 +1440,10 @@
   /// \param GD Global to scan.
   virtual bool emitTargetGlobal(GlobalDecl GD);
 
+  ///

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

[Gheorghe-Teodor Bercea via Phabricator via cfe-commits]

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:
  lib/CodeGen/CGOpenMPRuntime.cpp
  lib/CodeGen/CGOpenMPRuntime.h
  lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
  lib/CodeGen/CGOpenMPRuntimeNVPTX.h
  lib/CodeGen/CodeGenModule.cpp
  test/OpenMP/openmp_offload_registration.cpp

Index: test/OpenMP/openmp_offload_registration.cpp
===
--- test/OpenMP/openmp_offload_registration.cpp
+++ test/OpenMP/openmp_offload_registration.cpp
@@ -26,7 +26,7 @@
 // CHECK: [[DESC:@.+]] = internal constant [[DSCTY]] { i32 2, [[DEVTY]]* getelementptr inbounds ([2 x [[DEVTY]]], [2 x [[DEVTY]]]* [[IMAGES]], i32 0, i32 0), [[ENTTY]]* [[ENTBEGIN]], [[ENTTY]]* [[ENTEND]] }, comdat($[[REGFN]])
 
 // Check target registration is registered as a Ctor.
-// CHECK: appending global [1 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 0, void ()* @[[REGFN]], i8* bitcast (void ()* @[[REGFN]] to i8*) }]
+// CHECK: appending global [2 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 0, void ()* @.omp_offloading.requires_reg, i8* null }, { i32, void ()*, i8* } { i32 0, void ()* @[[REGFN]], i8* bitcast (void ()* @[[REGFN]] to i8*) }]
 
 // Check presence of foo() and the outlined target region
 // CHECK: define void [[FOO:@.+]]()
@@ -34,6 +34,11 @@
 
 // Check registration and unregistration code.
 
+// CHECK: define internal void @.omp_offloading.requires_reg()
+// CHECK: call void @__tgt_register_requires(i64 0)
+// CHECK: ret void
+// CHECK: declare void @__tgt_register_requires(i64)
+
 // CHECK: define internal void @[[UNREGFN:.+]](i8*)
 // CHECK-SAME: comdat($[[REGFN]]) {
 // CHECK: call i32 @__tgt_unregister_lib([[DSCTY]]* [[DESC]])
Index: lib/CodeGen/CodeGenModule.cpp
===
--- lib/CodeGen/CodeGenModule.cpp
+++ lib/CodeGen/CodeGenModule.cpp
@@ -410,6 +410,10 @@
   AddGlobalCtor(CudaCtorFunction);
   }
   if (OpenMPRuntime) {
+if (llvm::Function *OpenMPRequiresDirectiveRegFun =
+OpenMPRuntime->emitRequiresDirectiveRegFun()) {
+  AddGlobalCtor(OpenMPRequiresDirectiveRegFun, 0);
+}
 if (llvm::Function *OpenMPRegistrationFunction =
 OpenMPRuntime->emitRegistrationFunction()) {
   auto ComdatKey = OpenMPRegistrationFunction->hasComdat() ?
Index: lib/CodeGen/CGOpenMPRuntimeNVPTX.h
===
--- lib/CodeGen/CGOpenMPRuntimeNVPTX.h
+++ lib/CodeGen/CGOpenMPRuntimeNVPTX.h
@@ -383,7 +383,7 @@
 
   /// Perform check on requires decl to ensure that target architecture
   /// supports unified addressing
-  void checkArchForUnifiedAddressing(const OMPRequiresDecl *D) const override;
+  void checkArchForUnifiedAddressing(const OMPRequiresDecl *D) override;
 
   /// Returns default address space for the constant firstprivates, __constant__
   /// address space by default.
Index: lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
===
--- lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
+++ lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
@@ -4942,7 +4942,7 @@
 /// Check to see if target architecture supports unified addressing which is
 /// a restriction for OpenMP requires clause "unified_shared_memory".
 void CGOpenMPRuntimeNVPTX::checkArchForUnifiedAddressing(
-const OMPRequiresDecl *D) const {
+const OMPRequiresDecl *D) {
   for (const OMPClause *Clause : D->clauselists()) {
 if (Clause->getClauseKind() == OMPC_unified_shared_memory) {
   switch (getCudaArch(CGM)) {
@@ -4987,6 +4987,7 @@
   }
 }
   }
+  CGOpenMPRuntime::checkArchForUnifiedAddressing(D);
 }
 
 /// Get number of SMs and number of blocks per SM.
Index: lib/CodeGen/CGOpenMPRuntime.h
===
--- lib/CodeGen/CGOpenMPRuntime.h
+++ lib/CodeGen/CGOpenMPRuntime.h
@@ -636,6 +636,17 @@
   /// must be emitted.
   llvm::SmallDenseSet DeferredGlobalVariables;
 
+  /// Flag for keeping track of weather a requires unified_shared_memory
+  /// directive is present.
+  bool HasRequiresUnifiedSharedMemory = false;
+
+  /// Flag for keeping track of weather a target region has been emitted.
+  bool HasEmittedTargetRegion = false;
+
+  /// Flag for keeping track of weather a device routine has been emitted.
+  /// Device routines are specific to the
+  bool HasEmittedDeclareTargetRegion = false;
+
   /// Creates and registers offloading binary descriptor for the current
   /// compilation unit. The function that does the registration is returned.
   llvm::Function *createOffloadingBinaryDescriptorRegistration();
@@ -1429,6 +1440,10 @@
   /// \param GD Global to scan.
   virtual bool emitTargetGlobal(Globa

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

[Alexey Bataev via Phabricator via cfe-commits]

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 about target region but points to the clause. You need to 
correct the message.



Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:9065
+// contain at least 1 target region.
+if (HasRequiresUnifiedSharedMemory && hasEmittedTargetRegion)
+  Flags = OMP_REQ_UNIFIED_SHARED_MEMORY;

Hmm, according to the standard `A requires directive with any of the following 
clauses must appear in all compilation units of a program that contain device 
constructs or device routines or in none of them`. You can not detect use of 
the device routines in the code, can you?



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

ABataev wrote:
> gtbercea wrote:
> > ABataev wrote:
> > > gtbercea wrote:
> > > > gtbercea wrote:
> > > > > ABataev wrote:
> > > > > > gtbercea wrote:
> > > > > > > ABataev wrote:
> > > > > > > > Why do you need this? I think your function should be called 
> > > > > > > > without any conditions. It does not depend on the presence of 
> > > > > > > > the target regions or not. Plus, your check is not consistent 
> > > > > > > > if you're calling the function from another module, which has 
> > > > > > > > target region inside.
> > > > > > > This does not determine if the function is called or not. This 
> > > > > > > helps determine the flags with which the function is called.
> > > > > > It does not matter, it still does not work correctly if the target 
> > > > > > region is called in the function from another module
> > > > > If the target is in another compilation unit, that unit will need to 
> > > > > have a requires directive.
> > > > If you can come up with an example which you think doesn't work I'm 
> > > > happy to try it.
> > > If the module without target directives was compiled with unified memory 
> > > and the module with target directives compiled without unfied memory 
> > > support? Is this a problem? Shall we diagnose it?
> > The requires directives in a module without targets are just not taken into 
> > consideration. In general, a target region is needed before the requires 
> > flags are checked for compatibility with flags in other modules.
> You're saying that we're going to ignore the directive completely if the 
> module does not have target regions. I'd suggest to discuss it with Alex if 
> this is ok per standard.
Must start from capital letter


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
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Gheorghe-Teodor Bercea via Phabricator via cfe-commits]

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:
> gtbercea wrote:
> > ABataev wrote:
> > > gtbercea wrote:
> > > > gtbercea wrote:
> > > > > ABataev wrote:
> > > > > > gtbercea wrote:
> > > > > > > ABataev wrote:
> > > > > > > > Why do you need this? I think your function should be called 
> > > > > > > > without any conditions. It does not depend on the presence of 
> > > > > > > > the target regions or not. Plus, your check is not consistent 
> > > > > > > > if you're calling the function from another module, which has 
> > > > > > > > target region inside.
> > > > > > > This does not determine if the function is called or not. This 
> > > > > > > helps determine the flags with which the function is called.
> > > > > > It does not matter, it still does not work correctly if the target 
> > > > > > region is called in the function from another module
> > > > > If the target is in another compilation unit, that unit will need to 
> > > > > have a requires directive.
> > > > If you can come up with an example which you think doesn't work I'm 
> > > > happy to try it.
> > > If the module without target directives was compiled with unified memory 
> > > and the module with target directives compiled without unfied memory 
> > > support? Is this a problem? Shall we diagnose it?
> > The requires directives in a module without targets are just not taken into 
> > consideration. In general, a target region is needed before the requires 
> > flags are checked for compatibility with flags in other modules.
> You're saying that we're going to ignore the directive completely if the 
> module does not have target regions. I'd suggest to discuss it with Alex if 
> this is ok per standard.
I discussed with Alex and we agreed on this being the most practical solution. 
This will enable users to ensure consistency of requires flags across relevant 
compilation units only.


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
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Alexandre Eichenberger via Phabricator via cfe-commits]

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



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

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

[Alexey Bataev via Phabricator via cfe-commits]

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 wrote:
> > > > ABataev wrote:
> > > > > gtbercea wrote:
> > > > > > ABataev wrote:
> > > > > > > Why do you need this? I think your function should be called 
> > > > > > > without any conditions. It does not depend on the presence of the 
> > > > > > > target regions or not. Plus, your check is not consistent if 
> > > > > > > you're calling the function from another module, which has target 
> > > > > > > region inside.
> > > > > > This does not determine if the function is called or not. This 
> > > > > > helps determine the flags with which the function is called.
> > > > > It does not matter, it still does not work correctly if the target 
> > > > > region is called in the function from another module
> > > > If the target is in another compilation unit, that unit will need to 
> > > > have a requires directive.
> > > If you can come up with an example which you think doesn't work I'm happy 
> > > to try it.
> > If the module without target directives was compiled with unified memory 
> > and the module with target directives compiled without unfied memory 
> > support? Is this a problem? Shall we diagnose it?
> The requires directives in a module without targets are just not taken into 
> consideration. In general, a target region is needed before the requires 
> flags are checked for compatibility with flags in other modules.
You're saying that we're going to ignore the directive completely if the module 
does not have target regions. I'd suggest to discuss it with Alex if this is ok 
per standard.


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
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Gheorghe-Teodor Bercea via Phabricator via cfe-commits]

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:
> gtbercea wrote:
> > gtbercea wrote:
> > > ABataev wrote:
> > > > gtbercea wrote:
> > > > > ABataev wrote:
> > > > > > Why do you need this? I think your function should be called 
> > > > > > without any conditions. It does not depend on the presence of the 
> > > > > > target regions or not. Plus, your check is not consistent if you're 
> > > > > > calling the function from another module, which has target region 
> > > > > > inside.
> > > > > This does not determine if the function is called or not. This helps 
> > > > > determine the flags with which the function is called.
> > > > It does not matter, it still does not work correctly if the target 
> > > > region is called in the function from another module
> > > If the target is in another compilation unit, that unit will need to have 
> > > a requires directive.
> > If you can come up with an example which you think doesn't work I'm happy 
> > to try it.
> If the module without target directives was compiled with unified memory and 
> the module with target directives compiled without unfied memory support? Is 
> this a problem? Shall we diagnose it?
The requires directives in a module without targets are just not taken into 
consideration. In general, a target region is needed before the requires flags 
are checked for compatibility with flags in other modules.


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
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Alexey Bataev via Phabricator via cfe-commits]

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 wrote:
> > > > ABataev wrote:
> > > > > Why do you need this? I think your function should be called without 
> > > > > any conditions. It does not depend on the presence of the target 
> > > > > regions or not. Plus, your check is not consistent if you're calling 
> > > > > the function from another module, which has target region inside.
> > > > This does not determine if the function is called or not. This helps 
> > > > determine the flags with which the function is called.
> > > It does not matter, it still does not work correctly if the target region 
> > > is called in the function from another module
> > If the target is in another compilation unit, that unit will need to have a 
> > requires directive.
> If you can come up with an example which you think doesn't work I'm happy to 
> try it.
If the module without target directives was compiled with unified memory and 
the module with target directives compiled without unfied memory support? Is 
this a problem? Shall we diagnose it?


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
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Gheorghe-Teodor Bercea via Phabricator via cfe-commits]

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:
> ABataev wrote:
> > gtbercea wrote:
> > > ABataev wrote:
> > > > Why do you need this? I think your function should be called without 
> > > > any conditions. It does not depend on the presence of the target 
> > > > regions or not. Plus, your check is not consistent if you're calling 
> > > > the function from another module, which has target region inside.
> > > This does not determine if the function is called or not. This helps 
> > > determine the flags with which the function is called.
> > It does not matter, it still does not work correctly if the target region 
> > is called in the function from another module
> If the target is in another compilation unit, that unit will need to have a 
> requires directive.
If you can come up with an example which you think doesn't work I'm happy to 
try it.


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
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Gheorghe-Teodor Bercea via Phabricator via cfe-commits]

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:
> gtbercea wrote:
> > ABataev wrote:
> > > Why do you need this? I think your function should be called without any 
> > > conditions. It does not depend on the presence of the target regions or 
> > > not. Plus, your check is not consistent if you're calling the function 
> > > from another module, which has target region inside.
> > This does not determine if the function is called or not. This helps 
> > determine the flags with which the function is called.
> It does not matter, it still does not work correctly if the target region is 
> called in the function from another module
If the target is in another compilation unit, that unit will need to have a 
requires directive.


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
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Alexey Bataev via Phabricator via cfe-commits]

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 your function should be called without any 
> > conditions. It does not depend on the presence of the target regions or 
> > not. Plus, your check is not consistent if you're calling the function from 
> > another module, which has target region inside.
> This does not determine if the function is called or not. This helps 
> determine the flags with which the function is called.
It does not matter, it still does not work correctly if the target region is 
called in the function from another module


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
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Gheorghe-Teodor Bercea via Phabricator via cfe-commits]

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 you need this? I think your function should be called without any 
> conditions. It does not depend on the presence of the target regions or not. 
> Plus, your check is not consistent if you're calling the function from 
> another module, which has target region inside.
This does not determine if the function is called or not. This helps determine 
the flags with which the function is called.


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
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Alexey Bataev via Phabricator via cfe-commits]

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 any 
conditions. It does not depend on the presence of the target regions or not. 
Plus, your check is not consistent if you're calling the function from another 
module, which has target region inside.


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
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Gheorghe-Teodor Bercea via Phabricator via cfe-commits]

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
  lib/CodeGen/CGOpenMPRuntime.h
  lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
  lib/CodeGen/CGOpenMPRuntimeNVPTX.h
  lib/CodeGen/CodeGenModule.cpp
  test/OpenMP/openmp_offload_registration.cpp

Index: test/OpenMP/openmp_offload_registration.cpp
===
--- test/OpenMP/openmp_offload_registration.cpp
+++ test/OpenMP/openmp_offload_registration.cpp
@@ -26,7 +26,7 @@
 // CHECK: [[DESC:@.+]] = internal constant [[DSCTY]] { i32 2, [[DEVTY]]* getelementptr inbounds ([2 x [[DEVTY]]], [2 x [[DEVTY]]]* [[IMAGES]], i32 0, i32 0), [[ENTTY]]* [[ENTBEGIN]], [[ENTTY]]* [[ENTEND]] }, comdat($[[REGFN]])
 
 // Check target registration is registered as a Ctor.
-// CHECK: appending global [1 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 0, void ()* @[[REGFN]], i8* bitcast (void ()* @[[REGFN]] to i8*) }]
+// CHECK: appending global [2 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 0, void ()* @.omp_offloading.requires_reg, i8* null }, { i32, void ()*, i8* } { i32 0, void ()* @[[REGFN]], i8* bitcast (void ()* @[[REGFN]] to i8*) }]
 
 // Check presence of foo() and the outlined target region
 // CHECK: define void [[FOO:@.+]]()
@@ -34,6 +34,11 @@
 
 // Check registration and unregistration code.
 
+// CHECK: define internal void @.omp_offloading.requires_reg()
+// CHECK: call void @__tgt_register_requires(i64 0)
+// CHECK: ret void
+// CHECK: declare void @__tgt_register_requires(i64)
+
 // CHECK: define internal void @[[UNREGFN:.+]](i8*)
 // CHECK-SAME: comdat($[[REGFN]]) {
 // CHECK: call i32 @__tgt_unregister_lib([[DSCTY]]* [[DESC]])
Index: lib/CodeGen/CodeGenModule.cpp
===
--- lib/CodeGen/CodeGenModule.cpp
+++ lib/CodeGen/CodeGenModule.cpp
@@ -410,6 +410,10 @@
   AddGlobalCtor(CudaCtorFunction);
   }
   if (OpenMPRuntime) {
+if (llvm::Function *OpenMPRequiresDirectiveRegFun =
+OpenMPRuntime->emitRequiresDirectiveRegFun()) {
+  AddGlobalCtor(OpenMPRequiresDirectiveRegFun, 0);
+}
 if (llvm::Function *OpenMPRegistrationFunction =
 OpenMPRuntime->emitRegistrationFunction()) {
   auto ComdatKey = OpenMPRegistrationFunction->hasComdat() ?
Index: lib/CodeGen/CGOpenMPRuntimeNVPTX.h
===
--- lib/CodeGen/CGOpenMPRuntimeNVPTX.h
+++ lib/CodeGen/CGOpenMPRuntimeNVPTX.h
@@ -383,7 +383,7 @@
 
   /// Perform check on requires decl to ensure that target architecture
   /// supports unified addressing
-  void checkArchForUnifiedAddressing(const OMPRequiresDecl *D) const override;
+  void checkArchForUnifiedAddressing(const OMPRequiresDecl *D) override;
 
   /// Returns default address space for the constant firstprivates, __constant__
   /// address space by default.
Index: lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
===
--- lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
+++ lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
@@ -4942,7 +4942,7 @@
 /// Check to see if target architecture supports unified addressing which is
 /// a restriction for OpenMP requires clause "unified_shared_memory".
 void CGOpenMPRuntimeNVPTX::checkArchForUnifiedAddressing(
-const OMPRequiresDecl *D) const {
+const OMPRequiresDecl *D) {
   for (const OMPClause *Clause : D->clauselists()) {
 if (Clause->getClauseKind() == OMPC_unified_shared_memory) {
   switch (getCudaArch(CGM)) {
@@ -4987,6 +4987,7 @@
   }
 }
   }
+  CGOpenMPRuntime::checkArchForUnifiedAddressing(D);
 }
 
 /// Get number of SMs and number of blocks per SM.
Index: lib/CodeGen/CGOpenMPRuntime.h
===
--- lib/CodeGen/CGOpenMPRuntime.h
+++ lib/CodeGen/CGOpenMPRuntime.h
@@ -636,6 +636,13 @@
   /// must be emitted.
   llvm::SmallDenseSet DeferredGlobalVariables;
 
+  /// Flag for keeping track of weather a requires unified_shared_memory
+  /// directive is present.
+  bool HasRequiresUnifiedSharedMemory = false;
+
+  /// Flag for keeping track of weather a target region has been emitted.
+  bool hasEmittedTargetRegion = false;
+
   /// Creates and registers offloading binary descriptor for the current
   /// compilation unit. The function that does the registration is returned.
   llvm::Function *createOffloadingBinaryDescriptorRegistration();
@@ -1429,6 +1436,10 @@
   /// \param GD Global to scan.
   virtual bool emitTargetGlobal(GlobalDecl GD);
 
+  /// Creates and returns a registration function for when at least one
+  /// requires directives was used in the current module.
+  llvm::Function *emitRequiresDirectiveRegFun();
+
   /// Create

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

[Gheorghe-Teodor Bercea via Phabricator via cfe-commits]

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 void

ABataev wrote:
> Add a test where the unified memory flag is used
Agreed. More test will be coming anyway. There are a lot of regression tests 
which need to be updated.


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
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Alexey Bataev via Phabricator via cfe-commits]

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:
> > ABataev wrote:
> > > ABataev wrote:
> > > > Just `HasRequiresUnifiedSharedMemory = true;`
> > > Not done
> > Ah I misunderstood your initial comment. I thought you were complaining 
> > about the break being in there.
> Because the member is non-static I have to provide the CGM.getOpenMPRuntime() 
> part.
Because the member function is non-static and the member data is non-static, 
you can just use `HasRequiresUnifiedSharedMemory = true;`



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 void

Add a test where the unified memory flag is used


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
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Gheorghe-Teodor Bercea via Phabricator via cfe-commits]

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
  lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
  lib/CodeGen/CGOpenMPRuntimeNVPTX.h
  lib/CodeGen/CodeGenModule.cpp
  test/OpenMP/openmp_offload_registration.cpp

Index: test/OpenMP/openmp_offload_registration.cpp
===
--- test/OpenMP/openmp_offload_registration.cpp
+++ test/OpenMP/openmp_offload_registration.cpp
@@ -26,7 +26,7 @@
 // CHECK: [[DESC:@.+]] = internal constant [[DSCTY]] { i32 2, [[DEVTY]]* getelementptr inbounds ([2 x [[DEVTY]]], [2 x [[DEVTY]]]* [[IMAGES]], i32 0, i32 0), [[ENTTY]]* [[ENTBEGIN]], [[ENTTY]]* [[ENTEND]] }, comdat($[[REGFN]])
 
 // Check target registration is registered as a Ctor.
-// CHECK: appending global [1 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 0, void ()* @[[REGFN]], i8* bitcast (void ()* @[[REGFN]] to i8*) }]
+// CHECK: appending global [2 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 0, void ()* @.omp_offloading.requires_reg, i8* null }, { i32, void ()*, i8* } { i32 0, void ()* @[[REGFN]], i8* bitcast (void ()* @[[REGFN]] to i8*) }]
 
 // Check presence of foo() and the outlined target region
 // CHECK: define void [[FOO:@.+]]()
@@ -34,6 +34,11 @@
 
 // Check registration and unregistration code.
 
+// CHECK: define internal void @.omp_offloading.requires_reg()
+// CHECK: call void @__tgt_register_requires(i64 0)
+// CHECK: ret void
+// CHECK: declare void @__tgt_register_requires(i64)
+
 // CHECK: define internal void @[[UNREGFN:.+]](i8*)
 // CHECK-SAME: comdat($[[REGFN]]) {
 // CHECK: call i32 @__tgt_unregister_lib([[DSCTY]]* [[DESC]])
Index: lib/CodeGen/CodeGenModule.cpp
===
--- lib/CodeGen/CodeGenModule.cpp
+++ lib/CodeGen/CodeGenModule.cpp
@@ -410,6 +410,10 @@
   AddGlobalCtor(CudaCtorFunction);
   }
   if (OpenMPRuntime) {
+if (llvm::Function *OpenMPRequiresDirectiveRegFun =
+OpenMPRuntime->emitRequiresDirectiveRegFun()) {
+  AddGlobalCtor(OpenMPRequiresDirectiveRegFun, 0);
+}
 if (llvm::Function *OpenMPRegistrationFunction =
 OpenMPRuntime->emitRegistrationFunction()) {
   auto ComdatKey = OpenMPRegistrationFunction->hasComdat() ?
Index: lib/CodeGen/CGOpenMPRuntimeNVPTX.h
===
--- lib/CodeGen/CGOpenMPRuntimeNVPTX.h
+++ lib/CodeGen/CGOpenMPRuntimeNVPTX.h
@@ -383,7 +383,7 @@
 
   /// Perform check on requires decl to ensure that target architecture
   /// supports unified addressing
-  void checkArchForUnifiedAddressing(const OMPRequiresDecl *D) const override;
+  void checkArchForUnifiedAddressing(const OMPRequiresDecl *D) override;
 
   /// Returns default address space for the constant firstprivates, __constant__
   /// address space by default.
Index: lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
===
--- lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
+++ lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
@@ -4942,7 +4942,7 @@
 /// Check to see if target architecture supports unified addressing which is
 /// a restriction for OpenMP requires clause "unified_shared_memory".
 void CGOpenMPRuntimeNVPTX::checkArchForUnifiedAddressing(
-const OMPRequiresDecl *D) const {
+const OMPRequiresDecl *D) {
   for (const OMPClause *Clause : D->clauselists()) {
 if (Clause->getClauseKind() == OMPC_unified_shared_memory) {
   switch (getCudaArch(CGM)) {
@@ -4987,6 +4987,7 @@
   }
 }
   }
+  CGOpenMPRuntime::checkArchForUnifiedAddressing(D);
 }
 
 /// Get number of SMs and number of blocks per SM.
Index: lib/CodeGen/CGOpenMPRuntime.h
===
--- lib/CodeGen/CGOpenMPRuntime.h
+++ lib/CodeGen/CGOpenMPRuntime.h
@@ -636,6 +636,10 @@
   /// must be emitted.
   llvm::SmallDenseSet DeferredGlobalVariables;
 
+  /// Flag for keeping track of weather a requires unified_shared_memory
+  /// directive is present.
+  bool HasRequiresUnifiedSharedMemory = false;
+
   /// Creates and registers offloading binary descriptor for the current
   /// compilation unit. The function that does the registration is returned.
   llvm::Function *createOffloadingBinaryDescriptorRegistration();
@@ -1429,6 +1433,10 @@
   /// \param GD Global to scan.
   virtual bool emitTargetGlobal(GlobalDecl GD);
 
+  /// Creates and returns a registration function for when at least one
+  /// requires directives was used in the current module.
+  llvm::Function *emitRequiresDirectiveRegFun();
+
   /// Creates the offloading descriptor in the event any target region
   /// was emitted in the current module and return the function that registers
   ///

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

[Gheorghe-Teodor Bercea via Phabricator via cfe-commits]

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;

ABataev wrote:
> ABataev wrote:
> > Just `HasRequiresUnifiedSharedMemory = true;`
> Not done
Ah I misunderstood your initial comment. I thought you were complaining about 
the break being in there.



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

gtbercea wrote:
> ABataev wrote:
> > ABataev wrote:
> > > Just `HasRequiresUnifiedSharedMemory = true;`
> > Not done
> Ah I misunderstood your initial comment. I thought you were complaining about 
> the break being in there.
Because the member is non-static I have to provide the CGM.getOpenMPRuntime() 
part.


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
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Gheorghe-Teodor Bercea via Phabricator via cfe-commits]

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:
> ABataev wrote:
> > Why do you need this vertual funtion? I think static local is going to be 
> > enough
> Can you make it `const` member function?
Cannot add that due to createRuntimeFunction not being const.


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
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Gheorghe-Teodor Bercea via Phabricator via cfe-commits]

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
  lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
  lib/CodeGen/CodeGenModule.cpp
  test/OpenMP/openmp_offload_registration.cpp

Index: test/OpenMP/openmp_offload_registration.cpp
===
--- test/OpenMP/openmp_offload_registration.cpp
+++ test/OpenMP/openmp_offload_registration.cpp
@@ -26,7 +26,7 @@
 // CHECK: [[DESC:@.+]] = internal constant [[DSCTY]] { i32 2, [[DEVTY]]* getelementptr inbounds ([2 x [[DEVTY]]], [2 x [[DEVTY]]]* [[IMAGES]], i32 0, i32 0), [[ENTTY]]* [[ENTBEGIN]], [[ENTTY]]* [[ENTEND]] }, comdat($[[REGFN]])
 
 // Check target registration is registered as a Ctor.
-// CHECK: appending global [1 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 0, void ()* @[[REGFN]], i8* bitcast (void ()* @[[REGFN]] to i8*) }]
+// CHECK: appending global [2 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 0, void ()* @.omp_offloading.requires_reg, i8* null }, { i32, void ()*, i8* } { i32 0, void ()* @[[REGFN]], i8* bitcast (void ()* @[[REGFN]] to i8*) }]
 
 // Check presence of foo() and the outlined target region
 // CHECK: define void [[FOO:@.+]]()
@@ -34,6 +34,11 @@
 
 // Check registration and unregistration code.
 
+// CHECK: define internal void @.omp_offloading.requires_reg()
+// CHECK: call void @__tgt_register_requires(i64 0)
+// CHECK: ret void
+// CHECK: declare void @__tgt_register_requires(i64)
+
 // CHECK: define internal void @[[UNREGFN:.+]](i8*)
 // CHECK-SAME: comdat($[[REGFN]]) {
 // CHECK: call i32 @__tgt_unregister_lib([[DSCTY]]* [[DESC]])
Index: lib/CodeGen/CodeGenModule.cpp
===
--- lib/CodeGen/CodeGenModule.cpp
+++ lib/CodeGen/CodeGenModule.cpp
@@ -410,6 +410,10 @@
   AddGlobalCtor(CudaCtorFunction);
   }
   if (OpenMPRuntime) {
+if (llvm::Function *OpenMPRequiresDirectiveRegFun =
+OpenMPRuntime->emitRequiresDirectiveRegFun()) {
+  AddGlobalCtor(OpenMPRequiresDirectiveRegFun, 0);
+}
 if (llvm::Function *OpenMPRegistrationFunction =
 OpenMPRuntime->emitRegistrationFunction()) {
   auto ComdatKey = OpenMPRegistrationFunction->hasComdat() ?
Index: lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
===
--- lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
+++ lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
@@ -4987,6 +4987,7 @@
   }
 }
   }
+  CGOpenMPRuntime::checkArchForUnifiedAddressing(D);
 }
 
 /// Get number of SMs and number of blocks per SM.
Index: lib/CodeGen/CGOpenMPRuntime.h
===
--- lib/CodeGen/CGOpenMPRuntime.h
+++ lib/CodeGen/CGOpenMPRuntime.h
@@ -636,6 +636,10 @@
   /// must be emitted.
   llvm::SmallDenseSet DeferredGlobalVariables;
 
+  /// Flag for keeping track of weather a requires unified_shared_memory
+  /// directive is present.
+  bool HasRequiresUnifiedSharedMemory = false;
+
   /// Creates and registers offloading binary descriptor for the current
   /// compilation unit. The function that does the registration is returned.
   llvm::Function *createOffloadingBinaryDescriptorRegistration();
@@ -1429,6 +1433,10 @@
   /// \param GD Global to scan.
   virtual bool emitTargetGlobal(GlobalDecl GD);
 
+  /// Creates and returns a registration function for when at least one
+  /// requires directives was used in the current module.
+  llvm::Function *emitRequiresDirectiveRegFun();
+
   /// Creates the offloading descriptor in the event any target region
   /// was emitted in the current module and return the function that registers
   /// it.
@@ -1597,7 +1605,7 @@
 
   /// Perform check on requires decl to ensure that target architecture
   /// supports unified addressing
-  virtual void checkArchForUnifiedAddressing(const OMPRequiresDecl *D) const {}
+  virtual void checkArchForUnifiedAddressing(const OMPRequiresDecl *D) const;
 
   /// Checks if the variable has associated OMPAllocateDeclAttr attribute with
   /// the predefined allocator and translates it into the corresponding address
Index: lib/CodeGen/CGOpenMPRuntime.cpp
===
--- lib/CodeGen/CGOpenMPRuntime.cpp
+++ lib/CodeGen/CGOpenMPRuntime.cpp
@@ -457,6 +457,24 @@
   LLVM_MARK_AS_BITMASK_ENUM(/*LargestValue=*/OMP_IDENT_WORK_DISTRIBUTE)
 };
 
+namespace {
+LLVM_ENABLE_BITMASK_ENUMS_IN_NAMESPACE();
+/// Values for bit flags for marking which requires clauses have been used.
+enum OpenMPOffloadingRequiresDirFlags : int64_t {
+  /// no requires directive present.
+  OMP_REQ_NONE= 0x000,
+  /// reverse_offload clause.
+  OMP_REQ_REVERSE_OFFLOAD = 0x001,
+  ///

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

[Gheorghe-Teodor Bercea via Phabricator via cfe-commits]

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
  lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
  lib/CodeGen/CodeGenModule.cpp
  test/OpenMP/openmp_offload_registration.cpp

Index: test/OpenMP/openmp_offload_registration.cpp
===
--- test/OpenMP/openmp_offload_registration.cpp
+++ test/OpenMP/openmp_offload_registration.cpp
@@ -26,7 +26,7 @@
 // CHECK: [[DESC:@.+]] = internal constant [[DSCTY]] { i32 2, [[DEVTY]]* getelementptr inbounds ([2 x [[DEVTY]]], [2 x [[DEVTY]]]* [[IMAGES]], i32 0, i32 0), [[ENTTY]]* [[ENTBEGIN]], [[ENTTY]]* [[ENTEND]] }, comdat($[[REGFN]])
 
 // Check target registration is registered as a Ctor.
-// CHECK: appending global [1 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 0, void ()* @[[REGFN]], i8* bitcast (void ()* @[[REGFN]] to i8*) }]
+// CHECK: appending global [2 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 0, void ()* @.omp_offloading.requires_reg, i8* null }, { i32, void ()*, i8* } { i32 0, void ()* @[[REGFN]], i8* bitcast (void ()* @[[REGFN]] to i8*) }]
 
 // Check presence of foo() and the outlined target region
 // CHECK: define void [[FOO:@.+]]()
@@ -34,6 +34,11 @@
 
 // Check registration and unregistration code.
 
+// CHECK: define internal void @.omp_offloading.requires_reg()
+// CHECK: call void @__tgt_register_requires(i64 0)
+// CHECK: ret void
+// CHECK: declare void @__tgt_register_requires(i64)
+
 // CHECK: define internal void @[[UNREGFN:.+]](i8*)
 // CHECK-SAME: comdat($[[REGFN]]) {
 // CHECK: call i32 @__tgt_unregister_lib([[DSCTY]]* [[DESC]])
Index: lib/CodeGen/CodeGenModule.cpp
===
--- lib/CodeGen/CodeGenModule.cpp
+++ lib/CodeGen/CodeGenModule.cpp
@@ -410,6 +410,10 @@
   AddGlobalCtor(CudaCtorFunction);
   }
   if (OpenMPRuntime) {
+if (llvm::Function *OpenMPRequiresDirectiveRegFun =
+OpenMPRuntime->emitRequiresDirectiveRegFun()) {
+  AddGlobalCtor(OpenMPRequiresDirectiveRegFun, 0);
+}
 if (llvm::Function *OpenMPRegistrationFunction =
 OpenMPRuntime->emitRegistrationFunction()) {
   auto ComdatKey = OpenMPRegistrationFunction->hasComdat() ?
Index: lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
===
--- lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
+++ lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
@@ -4987,6 +4987,7 @@
   }
 }
   }
+  CGOpenMPRuntime::checkArchForUnifiedAddressing(D);
 }
 
 /// Get number of SMs and number of blocks per SM.
Index: lib/CodeGen/CGOpenMPRuntime.h
===
--- lib/CodeGen/CGOpenMPRuntime.h
+++ lib/CodeGen/CGOpenMPRuntime.h
@@ -636,6 +636,10 @@
   /// must be emitted.
   llvm::SmallDenseSet DeferredGlobalVariables;
 
+  /// Flag for keeping track of weather a requires unified_shared_memory
+  /// directive is present.
+  bool HasRequiresUnifiedSharedMemory = false;
+
   /// Creates and registers offloading binary descriptor for the current
   /// compilation unit. The function that does the registration is returned.
   llvm::Function *createOffloadingBinaryDescriptorRegistration();
@@ -1429,6 +1433,10 @@
   /// \param GD Global to scan.
   virtual bool emitTargetGlobal(GlobalDecl GD);
 
+  /// Creates and returns a registration function for when at least one
+  /// requires directives was used in the current module.
+  llvm::Function *emitRequiresDirectiveRegFun();
+
   /// Creates the offloading descriptor in the event any target region
   /// was emitted in the current module and return the function that registers
   /// it.
@@ -1597,7 +1605,7 @@
 
   /// Perform check on requires decl to ensure that target architecture
   /// supports unified addressing
-  virtual void checkArchForUnifiedAddressing(const OMPRequiresDecl *D) const {}
+  virtual void checkArchForUnifiedAddressing(const OMPRequiresDecl *D) const;
 
   /// Checks if the variable has associated OMPAllocateDeclAttr attribute with
   /// the predefined allocator and translates it into the corresponding address
Index: lib/CodeGen/CGOpenMPRuntime.cpp
===
--- lib/CodeGen/CGOpenMPRuntime.cpp
+++ lib/CodeGen/CGOpenMPRuntime.cpp
@@ -457,6 +457,24 @@
   LLVM_MARK_AS_BITMASK_ENUM(/*LargestValue=*/OMP_IDENT_WORK_DISTRIBUTE)
 };
 
+namespace {
+LLVM_ENABLE_BITMASK_ENUMS_IN_NAMESPACE();
+/// Values for bit flags for marking which requires clauses have been used.
+enum OpenMPOffloadingRequiresDirFlags : int64_t {
+  /// no requires directive present.
+  OMP_REQ_NONE= 0x000,
+  /// reverse_offload clause.
+  OMP_REQ_REVERSE_OFFLOAD = 0x001,
+  /// 

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

[Alexey Bataev via Phabricator via cfe-commits]

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 further analysis



Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:9050
+CGF.StartFunction(GlobalDecl(), C.VoidTy, RequiresRegFn, FI, {});
+int64_t Flags = OMP_REQ_NONE;
+//TODO: check for other requires clauses.

Change the type from `int64_t` to `OpenMPOffloadingRequiresDirFlags`



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

ABataev wrote:
> Just `HasRequiresUnifiedSharedMemory = true;`
Not done



Comment at: lib/CodeGen/CGOpenMPRuntime.h:641
+  /// directive is present.
+  bool HasRequiresUnifiedSharedMemory = false;
+

AlexEichenberger wrote:
> Don't you need a bool for each characteristics? Your intention is to have one 
> bit vector for each characteristics that matter to the compiler?
> 
> Also, it is my belief that you need to record 2 states: 
> unmaterialized (meaning I have not processed any target yet, so I should 
> record any requires as they arrive)
> finalized (I am processing a target, so the state is now fixed)
> 
> you need this in case you have an input like this:
> 
> declare target
> int x
> end declare target
> 
> requires unified memory
> 
> which is illegal
What about this comment?



Comment at: lib/CodeGen/CGOpenMPRuntime.h:1438
+  /// requires directives was used in the current module.
+  virtual llvm::Function *emitRequiresDirectiveRegFun();
+

ABataev wrote:
> Why do you need this vertual funtion? I think static local is going to be 
> enough
Can you make it `const` member function?


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
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Gheorghe-Teodor Bercea via Phabricator via cfe-commits]

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
  lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
  lib/CodeGen/CodeGenModule.cpp
  test/OpenMP/openmp_offload_registration.cpp

Index: test/OpenMP/openmp_offload_registration.cpp
===
--- test/OpenMP/openmp_offload_registration.cpp
+++ test/OpenMP/openmp_offload_registration.cpp
@@ -26,7 +26,7 @@
 // CHECK: [[DESC:@.+]] = internal constant [[DSCTY]] { i32 2, [[DEVTY]]* getelementptr inbounds ([2 x [[DEVTY]]], [2 x [[DEVTY]]]* [[IMAGES]], i32 0, i32 0), [[ENTTY]]* [[ENTBEGIN]], [[ENTTY]]* [[ENTEND]] }, comdat($[[REGFN]])
 
 // Check target registration is registered as a Ctor.
-// CHECK: appending global [1 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 0, void ()* @[[REGFN]], i8* bitcast (void ()* @[[REGFN]] to i8*) }]
+// CHECK: appending global [2 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 0, void ()* @.omp_offloading.requires_reg, i8* null }, { i32, void ()*, i8* } { i32 0, void ()* @[[REGFN]], i8* bitcast (void ()* @[[REGFN]] to i8*) }]
 
 // Check presence of foo() and the outlined target region
 // CHECK: define void [[FOO:@.+]]()
@@ -34,6 +34,11 @@
 
 // Check registration and unregistration code.
 
+// CHECK: define internal void @.omp_offloading.requires_reg()
+// CHECK: call void @__tgt_register_requires(i64 0)
+// CHECK: ret void
+// CHECK: declare void @__tgt_register_requires(i64)
+
 // CHECK: define internal void @[[UNREGFN:.+]](i8*)
 // CHECK-SAME: comdat($[[REGFN]]) {
 // CHECK: call i32 @__tgt_unregister_lib([[DSCTY]]* [[DESC]])
Index: lib/CodeGen/CodeGenModule.cpp
===
--- lib/CodeGen/CodeGenModule.cpp
+++ lib/CodeGen/CodeGenModule.cpp
@@ -410,6 +410,10 @@
   AddGlobalCtor(CudaCtorFunction);
   }
   if (OpenMPRuntime) {
+if (llvm::Function *OpenMPRequiresDirectiveRegFun =
+OpenMPRuntime->emitRequiresDirectiveRegFun()) {
+  AddGlobalCtor(OpenMPRequiresDirectiveRegFun, 0);
+}
 if (llvm::Function *OpenMPRegistrationFunction =
 OpenMPRuntime->emitRegistrationFunction()) {
   auto ComdatKey = OpenMPRegistrationFunction->hasComdat() ?
Index: lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
===
--- lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
+++ lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
@@ -4987,6 +4987,7 @@
   }
 }
   }
+  CGOpenMPRuntime::checkArchForUnifiedAddressing(D);
 }
 
 /// Get number of SMs and number of blocks per SM.
Index: lib/CodeGen/CGOpenMPRuntime.h
===
--- lib/CodeGen/CGOpenMPRuntime.h
+++ lib/CodeGen/CGOpenMPRuntime.h
@@ -636,6 +636,10 @@
   /// must be emitted.
   llvm::SmallDenseSet DeferredGlobalVariables;
 
+  /// Flag for keeping track of weather a requires unified_shared_memory
+  /// directive is present.
+  bool HasRequiresUnifiedSharedMemory = false;
+
   /// Creates and registers offloading binary descriptor for the current
   /// compilation unit. The function that does the registration is returned.
   llvm::Function *createOffloadingBinaryDescriptorRegistration();
@@ -1429,6 +1433,10 @@
   /// \param GD Global to scan.
   virtual bool emitTargetGlobal(GlobalDecl GD);
 
+  /// Creates and returns a registration function for when at least one
+  /// requires directives was used in the current module.
+  llvm::Function *emitRequiresDirectiveRegFun();
+
   /// Creates the offloading descriptor in the event any target region
   /// was emitted in the current module and return the function that registers
   /// it.
@@ -1597,7 +1605,7 @@
 
   /// Perform check on requires decl to ensure that target architecture
   /// supports unified addressing
-  virtual void checkArchForUnifiedAddressing(const OMPRequiresDecl *D) const {}
+  virtual void checkArchForUnifiedAddressing(const OMPRequiresDecl *D) const;
 
   /// Checks if the variable has associated OMPAllocateDeclAttr attribute with
   /// the predefined allocator and translates it into the corresponding address
Index: lib/CodeGen/CGOpenMPRuntime.cpp
===
--- lib/CodeGen/CGOpenMPRuntime.cpp
+++ lib/CodeGen/CGOpenMPRuntime.cpp
@@ -457,6 +457,24 @@
   LLVM_MARK_AS_BITMASK_ENUM(/*LargestValue=*/OMP_IDENT_WORK_DISTRIBUTE)
 };
 
+namespace {
+LLVM_ENABLE_BITMASK_ENUMS_IN_NAMESPACE();
+/// Values for bit flags for marking which requires clauses have been used.
+enum OpenMPOffloadingRequiresDirFlags : int64_t {
+  /// no requires directive present.
+  OMP_REQ_NONE= 0x000,
+  /// reverse_offload clause.
+  OMP_REQ_REVERSE_OFFLOAD = 0x0

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

[Gheorghe-Teodor Bercea via Phabricator via cfe-commits]

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
  lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
  lib/CodeGen/CodeGenModule.cpp
  test/OpenMP/openmp_offload_registration.cpp

Index: test/OpenMP/openmp_offload_registration.cpp
===
--- test/OpenMP/openmp_offload_registration.cpp
+++ test/OpenMP/openmp_offload_registration.cpp
@@ -26,7 +26,7 @@
 // CHECK: [[DESC:@.+]] = internal constant [[DSCTY]] { i32 2, [[DEVTY]]* getelementptr inbounds ([2 x [[DEVTY]]], [2 x [[DEVTY]]]* [[IMAGES]], i32 0, i32 0), [[ENTTY]]* [[ENTBEGIN]], [[ENTTY]]* [[ENTEND]] }, comdat($[[REGFN]])
 
 // Check target registration is registered as a Ctor.
-// CHECK: appending global [1 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 0, void ()* @[[REGFN]], i8* bitcast (void ()* @[[REGFN]] to i8*) }]
+// CHECK: appending global [2 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 0, void ()* @.omp_offloading.requires_reg, i8* null }, { i32, void ()*, i8* } { i32 0, void ()* @[[REGFN]], i8* bitcast (void ()* @[[REGFN]] to i8*) }]
 
 // Check presence of foo() and the outlined target region
 // CHECK: define void [[FOO:@.+]]()
@@ -34,6 +34,11 @@
 
 // Check registration and unregistration code.
 
+// CHECK: define internal void @.omp_offloading.requires_reg()
+// CHECK: call void @__tgt_register_requires(i64 0)
+// CHECK: ret void
+// CHECK: declare void @__tgt_register_requires(i64)
+
 // CHECK: define internal void @[[UNREGFN:.+]](i8*)
 // CHECK-SAME: comdat($[[REGFN]]) {
 // CHECK: call i32 @__tgt_unregister_lib([[DSCTY]]* [[DESC]])
Index: lib/CodeGen/CodeGenModule.cpp
===
--- lib/CodeGen/CodeGenModule.cpp
+++ lib/CodeGen/CodeGenModule.cpp
@@ -410,6 +410,10 @@
   AddGlobalCtor(CudaCtorFunction);
   }
   if (OpenMPRuntime) {
+if (llvm::Function *OpenMPRequiresDirectiveRegFun =
+OpenMPRuntime->emitRequiresDirectiveRegFun()) {
+  AddGlobalCtor(OpenMPRequiresDirectiveRegFun, 0);
+}
 if (llvm::Function *OpenMPRegistrationFunction =
 OpenMPRuntime->emitRegistrationFunction()) {
   auto ComdatKey = OpenMPRegistrationFunction->hasComdat() ?
Index: lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
===
--- lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
+++ lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
@@ -4987,6 +4987,7 @@
   }
 }
   }
+  CGOpenMPRuntime::checkArchForUnifiedAddressing(D);
 }
 
 /// Get number of SMs and number of blocks per SM.
Index: lib/CodeGen/CGOpenMPRuntime.h
===
--- lib/CodeGen/CGOpenMPRuntime.h
+++ lib/CodeGen/CGOpenMPRuntime.h
@@ -636,6 +636,10 @@
   /// must be emitted.
   llvm::SmallDenseSet DeferredGlobalVariables;
 
+  /// Flag for keeping track of weather a requires unified_shared_memory
+  /// directive is present.
+  bool HasRequiresUnifiedSharedMemory = false;
+
   /// Creates and registers offloading binary descriptor for the current
   /// compilation unit. The function that does the registration is returned.
   llvm::Function *createOffloadingBinaryDescriptorRegistration();
@@ -1429,6 +1433,10 @@
   /// \param GD Global to scan.
   virtual bool emitTargetGlobal(GlobalDecl GD);
 
+  /// Creates and returns a registration function for when at least one
+  /// requires directives was used in the current module.
+  llvm::Function *emitRequiresDirectiveRegFun();
+
   /// Creates the offloading descriptor in the event any target region
   /// was emitted in the current module and return the function that registers
   /// it.
@@ -1597,7 +1605,7 @@
 
   /// Perform check on requires decl to ensure that target architecture
   /// supports unified addressing
-  virtual void checkArchForUnifiedAddressing(const OMPRequiresDecl *D) const {}
+  virtual void checkArchForUnifiedAddressing(const OMPRequiresDecl *D) const;
 
   /// Checks if the variable has associated OMPAllocateDeclAttr attribute with
   /// the predefined allocator and translates it into the corresponding address
Index: lib/CodeGen/CGOpenMPRuntime.cpp
===
--- lib/CodeGen/CGOpenMPRuntime.cpp
+++ lib/CodeGen/CGOpenMPRuntime.cpp
@@ -457,6 +457,30 @@
   LLVM_MARK_AS_BITMASK_ENUM(/*LargestValue=*/OMP_IDENT_WORK_DISTRIBUTE)
 };
 
+namespace {
+LLVM_ENABLE_BITMASK_ENUMS_IN_NAMESPACE();
+/// Values for bit flags for marking which requires clauses have been used.
+enum OpenMPOffloadingRequiresDirFlags : int64_t {
+  /// no requires directive present.
+  OMP_REQ_NONE= 0x000,
+  /// reverse_offload clause.
+  OMP_REQ_REVERSE_OFFLOAD = 0x001,

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

[Alexey Bataev via Phabricator via cfe-commits]

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 target-specific.



Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:1254
+
+  HasRequiresUnifiedSharedMemory = false;
 }

No need to do this initialization here



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

Just `HasRequiresUnifiedSharedMemory = true;`



Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:9045
+  // don't need to do anything.
+  if (CGM.getLangOpts().OpenMPIsDevice || OffloadEntriesInfoManager.empty())
+return nullptr;

Just add a check for OpenMPSimd mode here and do not emit anything in this case.



Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:9061
+//TODO: check for other requires clauses.
+if (HasRequiresUnifiedSharedMemory) {
+  Flags |= OMP_REQ_UNIFIED_SHARED_MEMORY;

No need for braces



Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:9065
+CGF.EmitRuntimeCall(createRuntimeFunction(OMPRTL__tgt_register_requires),
+llvm::ConstantInt::get(CGF.CGM.Int64Ty, Flags));
+CGF.FinishFunction();

`CGF.CGM.Int64Ty`->`CGM.Int64Ty`



Comment at: lib/CodeGen/CGOpenMPRuntime.h:1438
+  /// requires directives was used in the current module.
+  virtual llvm::Function *emitRequiresDirectiveRegFun();
+

Why do you need this vertual funtion? I think static local is going to be enough



Comment at: lib/CodeGen/CodeGenModule.cpp:415
+OpenMPRuntime->emitRequiresDirectiveRegFun()) {
+  AddGlobalCtor(OpenMPRequiresDirectiveRegFun, 0, nullptr);
+}

You can remove the third `nullptr` argument


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
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Alexandre Eichenberger via Phabricator via cfe-commits]

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
+  /// directive is present.
+  bool HasRequiresUnifiedSharedMemory = false;
+

Don't you need a bool for each characteristics? Your intention is to have one 
bit vector for each characteristics that matter to the compiler?

Also, it is my belief that you need to record 2 states: 
unmaterialized (meaning I have not processed any target yet, so I should record 
any requires as they arrive)
finalized (I am processing a target, so the state is now fixed)

you need this in case you have an input like this:

declare target
int x
end declare target

requires unified memory

which is illegal


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
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Gheorghe-Teodor Bercea via Phabricator via cfe-commits]

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
  lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
  lib/CodeGen/CodeGenModule.cpp
  test/OpenMP/openmp_offload_registration.cpp

Index: test/OpenMP/openmp_offload_registration.cpp
===
--- test/OpenMP/openmp_offload_registration.cpp
+++ test/OpenMP/openmp_offload_registration.cpp
@@ -26,7 +26,7 @@
 // CHECK: [[DESC:@.+]] = internal constant [[DSCTY]] { i32 2, [[DEVTY]]* getelementptr inbounds ([2 x [[DEVTY]]], [2 x [[DEVTY]]]* [[IMAGES]], i32 0, i32 0), [[ENTTY]]* [[ENTBEGIN]], [[ENTTY]]* [[ENTEND]] }, comdat($[[REGFN]])
 
 // Check target registration is registered as a Ctor.
-// CHECK: appending global [1 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 0, void ()* @[[REGFN]], i8* bitcast (void ()* @[[REGFN]] to i8*) }]
+// CHECK: appending global [2 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 0, void ()* @.omp_offloading.requires_reg, i8* null }, { i32, void ()*, i8* } { i32 0, void ()* @[[REGFN]], i8* bitcast (void ()* @[[REGFN]] to i8*) }]
 
 // Check presence of foo() and the outlined target region
 // CHECK: define void [[FOO:@.+]]()
@@ -34,6 +34,11 @@
 
 // Check registration and unregistration code.
 
+// CHECK: define internal void @.omp_offloading.requires_reg()
+// CHECK: call i32 @__tgt_register_requires(i64 0)
+// CHECK: ret void
+// CHECK: declare i32 @__tgt_register_requires(i64)
+
 // CHECK: define internal void @[[UNREGFN:.+]](i8*)
 // CHECK-SAME: comdat($[[REGFN]]) {
 // CHECK: call i32 @__tgt_unregister_lib([[DSCTY]]* [[DESC]])
Index: lib/CodeGen/CodeGenModule.cpp
===
--- lib/CodeGen/CodeGenModule.cpp
+++ lib/CodeGen/CodeGenModule.cpp
@@ -410,6 +410,10 @@
   AddGlobalCtor(CudaCtorFunction);
   }
   if (OpenMPRuntime) {
+if (llvm::Function *OpenMPRequiresDirectiveRegFun =
+OpenMPRuntime->emitRequiresDirectiveRegFun()) {
+  AddGlobalCtor(OpenMPRequiresDirectiveRegFun, 0, nullptr);
+}
 if (llvm::Function *OpenMPRegistrationFunction =
 OpenMPRuntime->emitRegistrationFunction()) {
   auto ComdatKey = OpenMPRegistrationFunction->hasComdat() ?
Index: lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
===
--- lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
+++ lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
@@ -4987,6 +4987,7 @@
   }
 }
   }
+  CGOpenMPRuntime::checkArchForUnifiedAddressing(D);
 }
 
 /// Get number of SMs and number of blocks per SM.
Index: lib/CodeGen/CGOpenMPRuntime.h
===
--- lib/CodeGen/CGOpenMPRuntime.h
+++ lib/CodeGen/CGOpenMPRuntime.h
@@ -636,6 +636,10 @@
   /// must be emitted.
   llvm::SmallDenseSet DeferredGlobalVariables;
 
+  /// Flag for keeping track of weather a requires unified_shared_memory
+  /// directive is present.
+  bool HasRequiresUnifiedSharedMemory = false;
+
   /// Creates and registers offloading binary descriptor for the current
   /// compilation unit. The function that does the registration is returned.
   llvm::Function *createOffloadingBinaryDescriptorRegistration();
@@ -1429,6 +1433,10 @@
   /// \param GD Global to scan.
   virtual bool emitTargetGlobal(GlobalDecl GD);
 
+  /// Creates and returns a registration function for when at least one
+  /// requires directives was used in the current module.
+  virtual llvm::Function *emitRequiresDirectiveRegFun();
+
   /// Creates the offloading descriptor in the event any target region
   /// was emitted in the current module and return the function that registers
   /// it.
@@ -1597,7 +1605,7 @@
 
   /// Perform check on requires decl to ensure that target architecture
   /// supports unified addressing
-  virtual void checkArchForUnifiedAddressing(const OMPRequiresDecl *D) const {}
+  virtual void checkArchForUnifiedAddressing(const OMPRequiresDecl *D) const;
 
   /// Checks if the variable has associated OMPAllocateDeclAttr attribute with
   /// the predefined allocator and translates it into the corresponding address
@@ -2094,6 +2102,10 @@
   /// \param GD Global to scan.
   bool emitTargetGlobal(GlobalDecl GD) override;
 
+  /// Creates and returns a registration function for when at least one
+  /// requires directives was used in the current module.
+  llvm::Function *emitRequiresDirectiveRegFun() override;
+
   /// Creates the offloading descriptor in the event any target region
   /// was emitted in the current module and return the function that registers
   /// it.
Index: lib/CodeGen/CGOpenMPRuntime.cpp
===
--- lib/CodeGen/CGOpenMPRu

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

[Gheorghe-Teodor Bercea via Phabricator via cfe-commits]

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
  lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
  lib/CodeGen/CodeGenModule.cpp
  test/OpenMP/openmp_offload_registration.cpp

Index: test/OpenMP/openmp_offload_registration.cpp
===
--- test/OpenMP/openmp_offload_registration.cpp
+++ test/OpenMP/openmp_offload_registration.cpp
@@ -26,7 +26,7 @@
 // CHECK: [[DESC:@.+]] = internal constant [[DSCTY]] { i32 2, [[DEVTY]]* getelementptr inbounds ([2 x [[DEVTY]]], [2 x [[DEVTY]]]* [[IMAGES]], i32 0, i32 0), [[ENTTY]]* [[ENTBEGIN]], [[ENTTY]]* [[ENTEND]] }, comdat($[[REGFN]])
 
 // Check target registration is registered as a Ctor.
-// CHECK: appending global [1 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 0, void ()* @[[REGFN]], i8* bitcast (void ()* @[[REGFN]] to i8*) }]
+// CHECK: appending global [2 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 0, void ()* @.omp_offloading.requires_reg, i8* null }, { i32, void ()*, i8* } { i32 0, void ()* @[[REGFN]], i8* bitcast (void ()* @[[REGFN]] to i8*) }]
 
 // Check presence of foo() and the outlined target region
 // CHECK: define void [[FOO:@.+]]()
@@ -34,6 +34,11 @@
 
 // Check registration and unregistration code.
 
+// CHECK: define internal void @.omp_offloading.requires_reg()
+// CHECK: call i32 @__tgt_register_requires(i64 0)
+// CHECK: ret void
+// CHECK: declare i32 @__tgt_register_requires(i64)
+
 // CHECK: define internal void @[[UNREGFN:.+]](i8*)
 // CHECK-SAME: comdat($[[REGFN]]) {
 // CHECK: call i32 @__tgt_unregister_lib([[DSCTY]]* [[DESC]])
Index: lib/CodeGen/CodeGenModule.cpp
===
--- lib/CodeGen/CodeGenModule.cpp
+++ lib/CodeGen/CodeGenModule.cpp
@@ -410,6 +410,10 @@
   AddGlobalCtor(CudaCtorFunction);
   }
   if (OpenMPRuntime) {
+if (llvm::Function *OpenMPRequiresDirectiveRegFun =
+OpenMPRuntime->emitRequiresDirectiveRegFun()) {
+  AddGlobalCtor(OpenMPRequiresDirectiveRegFun, 0, nullptr);
+}
 if (llvm::Function *OpenMPRegistrationFunction =
 OpenMPRuntime->emitRegistrationFunction()) {
   auto ComdatKey = OpenMPRegistrationFunction->hasComdat() ?
Index: lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
===
--- lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
+++ lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
@@ -4987,6 +4987,7 @@
   }
 }
   }
+  CGOpenMPRuntime::checkArchForUnifiedAddressing(D);
 }
 
 /// Get number of SMs and number of blocks per SM.
Index: lib/CodeGen/CGOpenMPRuntime.h
===
--- lib/CodeGen/CGOpenMPRuntime.h
+++ lib/CodeGen/CGOpenMPRuntime.h
@@ -636,6 +636,10 @@
   /// must be emitted.
   llvm::SmallDenseSet DeferredGlobalVariables;
 
+  /// Flag for keeping track of weather a requires unified_shared_memory
+  /// directive is present.
+  bool HasRequiresUnifiedSharedMemory = false;
+
   /// Creates and registers offloading binary descriptor for the current
   /// compilation unit. The function that does the registration is returned.
   llvm::Function *createOffloadingBinaryDescriptorRegistration();
@@ -1429,6 +1433,10 @@
   /// \param GD Global to scan.
   virtual bool emitTargetGlobal(GlobalDecl GD);
 
+  /// Creates and returns a registration function for when at least one
+  /// requires directives was used in the current module.
+  virtual llvm::Function *emitRequiresDirectiveRegFun();
+
   /// Creates the offloading descriptor in the event any target region
   /// was emitted in the current module and return the function that registers
   /// it.
@@ -1597,7 +1605,7 @@
 
   /// Perform check on requires decl to ensure that target architecture
   /// supports unified addressing
-  virtual void checkArchForUnifiedAddressing(const OMPRequiresDecl *D) const {}
+  virtual void checkArchForUnifiedAddressing(const OMPRequiresDecl *D) const;
 
   /// Checks if the variable has associated OMPAllocateDeclAttr attribute with
   /// the predefined allocator and translates it into the corresponding address
Index: lib/CodeGen/CGOpenMPRuntime.cpp
===
--- lib/CodeGen/CGOpenMPRuntime.cpp
+++ lib/CodeGen/CGOpenMPRuntime.cpp
@@ -457,6 +457,30 @@
   LLVM_MARK_AS_BITMASK_ENUM(/*LargestValue=*/OMP_IDENT_WORK_DISTRIBUTE)
 };
 
+namespace {
+LLVM_ENABLE_BITMASK_ENUMS_IN_NAMESPACE();
+/// Values for bit flags for marking which requires clauses have been used.
+enum OpenMPOffloadingRequiresDirFlags : int64_t {
+  /// no requires directive present.
+  OMP_REQ_NONE= 0x000,
+  /// reverse_offload clause.
+  OMP_REQ_REVERSE_OFFLOAD   

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

[Gheorghe-Teodor Bercea via Phabricator via cfe-commits]

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
  lib/CodeGen/CGOpenMPRuntime.h
  lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
  lib/CodeGen/CodeGenModule.cpp
  test/OpenMP/openmp_offload_registration.cpp

Index: test/OpenMP/openmp_offload_registration.cpp
===
--- test/OpenMP/openmp_offload_registration.cpp
+++ test/OpenMP/openmp_offload_registration.cpp
@@ -26,7 +26,7 @@
 // CHECK: [[DESC:@.+]] = internal constant [[DSCTY]] { i32 2, [[DEVTY]]* getelementptr inbounds ([2 x [[DEVTY]]], [2 x [[DEVTY]]]* [[IMAGES]], i32 0, i32 0), [[ENTTY]]* [[ENTBEGIN]], [[ENTTY]]* [[ENTEND]] }, comdat($[[REGFN]])
 
 // Check target registration is registered as a Ctor.
-// CHECK: appending global [1 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 0, void ()* @[[REGFN]], i8* bitcast (void ()* @[[REGFN]] to i8*) }]
+// CHECK: appending global [2 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 0, void ()* @.omp_offloading.requires_reg, i8* null }, { i32, void ()*, i8* } { i32 0, void ()* @[[REGFN]], i8* bitcast (void ()* @[[REGFN]] to i8*) }]
 
 // Check presence of foo() and the outlined target region
 // CHECK: define void [[FOO:@.+]]()
@@ -34,6 +34,11 @@
 
 // Check registration and unregistration code.
 
+// CHECK: define internal void @.omp_offloading.requires_reg()
+// CHECK: call i32 @__tgt_register_requires(i64 0)
+// CHECK: ret void
+// CHECK: declare i32 @__tgt_register_requires(i64)
+
 // CHECK: define internal void @[[UNREGFN:.+]](i8*)
 // CHECK-SAME: comdat($[[REGFN]]) {
 // CHECK: call i32 @__tgt_unregister_lib([[DSCTY]]* [[DESC]])
Index: lib/CodeGen/CodeGenModule.cpp
===
--- lib/CodeGen/CodeGenModule.cpp
+++ lib/CodeGen/CodeGenModule.cpp
@@ -410,6 +410,10 @@
   AddGlobalCtor(CudaCtorFunction);
   }
   if (OpenMPRuntime) {
+if (llvm::Function *OpenMPRequiresDirectiveRegFun =
+OpenMPRuntime->emitRequiresDirectiveRegFun()) {
+  AddGlobalCtor(OpenMPRequiresDirectiveRegFun, 0, nullptr);
+}
 if (llvm::Function *OpenMPRegistrationFunction =
 OpenMPRuntime->emitRegistrationFunction()) {
   auto ComdatKey = OpenMPRegistrationFunction->hasComdat() ?
Index: lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
===
--- lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
+++ lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
@@ -4987,6 +4987,7 @@
   }
 }
   }
+  CGOpenMPRuntime::checkArchForUnifiedAddressing(D);
 }
 
 /// Get number of SMs and number of blocks per SM.
Index: lib/CodeGen/CGOpenMPRuntime.h
===
--- lib/CodeGen/CGOpenMPRuntime.h
+++ lib/CodeGen/CGOpenMPRuntime.h
@@ -636,6 +636,10 @@
   /// must be emitted.
   llvm::SmallDenseSet DeferredGlobalVariables;
 
+  /// Flag for keeping track of weather a requires unified_shared_memory
+  /// directive is present.
+  bool HasRequiresUnifiedSharedMemory = false;
+
   /// Creates and registers offloading binary descriptor for the current
   /// compilation unit. The function that does the registration is returned.
   llvm::Function *createOffloadingBinaryDescriptorRegistration();
@@ -1429,6 +1433,10 @@
   /// \param GD Global to scan.
   virtual bool emitTargetGlobal(GlobalDecl GD);
 
+  /// Creates and returns a registration function for when at least one
+  /// requires directives was used in the current module.
+  virtual llvm::Function *emitRequiresDirectiveRegFun();
+
   /// Creates the offloading descriptor in the event any target region
   /// was emitted in the current module and return the function that registers
   /// it.
@@ -1597,7 +1605,10 @@
 
   /// Perform check on requires decl to ensure that target architecture
   /// supports unified addressing
-  virtual void checkArchForUnifiedAddressing(const OMPRequiresDecl *D) const {}
+  virtual void checkArchForUnifiedAddressing(const OMPRequiresDecl *D) const;
+
+  /// Return true if unified addressing is supported by the architecture.
+  virtual bool hasUnifiedAddressingSupport() const;
 
   /// Checks if the variable has associated OMPAllocateDeclAttr attribute with
   /// the predefined allocator and translates it into the corresponding address
Index: lib/CodeGen/CGOpenMPRuntime.cpp
===
--- lib/CodeGen/CGOpenMPRuntime.cpp
+++ lib/CodeGen/CGOpenMPRuntime.cpp
@@ -457,6 +457,30 @@
   LLVM_MARK_AS_BITMASK_ENUM(/*LargestValue=*/OMP_IDENT_WORK_DISTRIBUTE)
 };
 
+namespace {
+LLVM_ENABLE_BITMASK_ENUMS_IN_NAMESPACE();
+/// Values for bit flags for marking which requires clauses have been used.
+enum OpenMPOffloadingRequ

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

[Gheorghe-Teodor Bercea via Phabricator via cfe-commits]

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

ABataev wrote:
> Why do you need a new member function? Can you make a static local function?
Same as for register lib.



Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:3887
+CGF.StartFunction(GlobalDecl(), C.VoidTy, RequiresRegFn, FI, {});
+int64_t Flags = OMP_REQ_NONE;
+//TODO: check for other requires clauses.

ABataev wrote:
> Use `OpenMPOffloadingRequiresDirFlags` instead of `int64_t`
Leads to error if I do that. This enum behaves like OpenMPLocationFlags.



Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:7983
 MapFlagsArrayTy &Types) const {
+// If using unified memory, no need to do the mappings.
+if (CGF.CGM.HasRequiresUnifiedSharedMemory)

ABataev wrote:
> Seems to me, this must be in another patch, has nothing to do with this patch
I will split it.



Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:9069
 
+llvm::Function *CGOpenMPRuntime::emitRequiresDirectiveRegFun() {
+  // Create and register the function that handles the requires directives.

ABataev wrote:
> Why do you need the second function?
Second function eliminated.


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
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Gheorghe-Teodor Bercea via Phabricator via cfe-commits]

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:
  lib/CodeGen/CGOpenMPRuntime.cpp
  lib/CodeGen/CGOpenMPRuntime.h
  lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
  lib/CodeGen/CodeGenModule.cpp
  test/OpenMP/openmp_offload_registration.cpp

Index: test/OpenMP/openmp_offload_registration.cpp
===
--- test/OpenMP/openmp_offload_registration.cpp
+++ test/OpenMP/openmp_offload_registration.cpp
@@ -26,7 +26,7 @@
 // CHECK: [[DESC:@.+]] = internal constant [[DSCTY]] { i32 2, [[DEVTY]]* getelementptr inbounds ([2 x [[DEVTY]]], [2 x [[DEVTY]]]* [[IMAGES]], i32 0, i32 0), [[ENTTY]]* [[ENTBEGIN]], [[ENTTY]]* [[ENTEND]] }, comdat($[[REGFN]])
 
 // Check target registration is registered as a Ctor.
-// CHECK: appending global [1 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 0, void ()* @[[REGFN]], i8* bitcast (void ()* @[[REGFN]] to i8*) }]
+// CHECK: appending global [2 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 0, void ()* @.omp_offloading.requires_reg, i8* null }, { i32, void ()*, i8* } { i32 0, void ()* @[[REGFN]], i8* bitcast (void ()* @[[REGFN]] to i8*) }]
 
 // Check presence of foo() and the outlined target region
 // CHECK: define void [[FOO:@.+]]()
@@ -34,6 +34,11 @@
 
 // Check registration and unregistration code.
 
+// CHECK: define internal void @.omp_offloading.requires_reg()
+// CHECK: call i32 @__tgt_register_requires(i64 0)
+// CHECK: ret void
+// CHECK: declare i32 @__tgt_register_requires(i64)
+
 // CHECK: define internal void @[[UNREGFN:.+]](i8*)
 // CHECK-SAME: comdat($[[REGFN]]) {
 // CHECK: call i32 @__tgt_unregister_lib([[DSCTY]]* [[DESC]])
Index: lib/CodeGen/CodeGenModule.cpp
===
--- lib/CodeGen/CodeGenModule.cpp
+++ lib/CodeGen/CodeGenModule.cpp
@@ -410,6 +410,10 @@
   AddGlobalCtor(CudaCtorFunction);
   }
   if (OpenMPRuntime) {
+if (llvm::Function *OpenMPRequiresDirectiveRegFun =
+OpenMPRuntime->emitRequiresDirectiveRegFun()) {
+  AddGlobalCtor(OpenMPRequiresDirectiveRegFun, 0, nullptr);
+}
 if (llvm::Function *OpenMPRegistrationFunction =
 OpenMPRuntime->emitRegistrationFunction()) {
   auto ComdatKey = OpenMPRegistrationFunction->hasComdat() ?
Index: lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
===
--- lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
+++ lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
@@ -4987,6 +4987,7 @@
   }
 }
   }
+  CGOpenMPRuntime::checkArchForUnifiedAddressing(D);
 }
 
 /// Get number of SMs and number of blocks per SM.
Index: lib/CodeGen/CGOpenMPRuntime.h
===
--- lib/CodeGen/CGOpenMPRuntime.h
+++ lib/CodeGen/CGOpenMPRuntime.h
@@ -636,6 +636,10 @@
   /// must be emitted.
   llvm::SmallDenseSet DeferredGlobalVariables;
 
+  /// Flag for keeping track of weather a requires unified_shared_memory
+  /// directive is present.
+  bool HasRequiresUnifiedSharedMemory = false;
+
   /// Creates and registers offloading binary descriptor for the current
   /// compilation unit. The function that does the registration is returned.
   llvm::Function *createOffloadingBinaryDescriptorRegistration();
@@ -1429,6 +1433,10 @@
   /// \param GD Global to scan.
   virtual bool emitTargetGlobal(GlobalDecl GD);
 
+  /// Creates and returns a registration function for when at least one
+  /// requires directives was used in the current module.
+  virtual llvm::Function *emitRequiresDirectiveRegFun();
+
   /// Creates the offloading descriptor in the event any target region
   /// was emitted in the current module and return the function that registers
   /// it.
@@ -1597,7 +1605,10 @@
 
   /// Perform check on requires decl to ensure that target architecture
   /// supports unified addressing
-  virtual void checkArchForUnifiedAddressing(const OMPRequiresDecl *D) const {}
+  virtual void checkArchForUnifiedAddressing(const OMPRequiresDecl *D) const;
+
+  /// Return true if unified addressing is supported by the architecture.
+  virtual bool hasUnifiedAddressingSupport() const;
 
   /// Checks if the variable has associated OMPAllocateDeclAttr attribute with
   /// the predefined allocator and translates it into the corresponding address
Index: lib/CodeGen/CGOpenMPRuntime.cpp
===
--- lib/CodeGen/CGOpenMPRuntime.cpp
+++ lib/CodeGen/CGOpenMPRuntime.cpp
@@ -457,6 +457,26 @@
   LLVM_MARK_AS_BITMASK_ENUM(/*LargestValue=*/OMP_IDENT_WORK_DISTRIBUTE)
 };
 
+/// Values for bit flags for marking which requires clauses have been used.
+enum OpenMPOffloadingRequiresDirFlags : int64_t {
+  /// no requires dire

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

[Gheorghe-Teodor Bercea via Phabricator via cfe-commits]

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
  lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
  lib/CodeGen/CodeGenModule.cpp
  test/OpenMP/openmp_offload_registration.cpp

Index: test/OpenMP/openmp_offload_registration.cpp
===
--- test/OpenMP/openmp_offload_registration.cpp
+++ test/OpenMP/openmp_offload_registration.cpp
@@ -26,7 +26,7 @@
 // CHECK: [[DESC:@.+]] = internal constant [[DSCTY]] { i32 2, [[DEVTY]]* getelementptr inbounds ([2 x [[DEVTY]]], [2 x [[DEVTY]]]* [[IMAGES]], i32 0, i32 0), [[ENTTY]]* [[ENTBEGIN]], [[ENTTY]]* [[ENTEND]] }, comdat($[[REGFN]])
 
 // Check target registration is registered as a Ctor.
-// CHECK: appending global [1 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 0, void ()* @[[REGFN]], i8* bitcast (void ()* @[[REGFN]] to i8*) }]
+// CHECK: appending global [2 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 0, void ()* @.omp_offloading.requires_reg, i8* null }, { i32, void ()*, i8* } { i32 0, void ()* @[[REGFN]], i8* bitcast (void ()* @[[REGFN]] to i8*) }]
 
 // Check presence of foo() and the outlined target region
 // CHECK: define void [[FOO:@.+]]()
@@ -34,6 +34,11 @@
 
 // Check registration and unregistration code.
 
+// CHECK: define internal void @.omp_offloading.requires_reg()
+// CHECK: call i32 @__tgt_register_requires(i64 0)
+// CHECK: ret void
+// CHECK: declare i32 @__tgt_register_requires(i64)
+
 // CHECK: define internal void @[[UNREGFN:.+]](i8*)
 // CHECK-SAME: comdat($[[REGFN]]) {
 // CHECK: call i32 @__tgt_unregister_lib([[DSCTY]]* [[DESC]])
Index: lib/CodeGen/CodeGenModule.cpp
===
--- lib/CodeGen/CodeGenModule.cpp
+++ lib/CodeGen/CodeGenModule.cpp
@@ -410,6 +410,10 @@
   AddGlobalCtor(CudaCtorFunction);
   }
   if (OpenMPRuntime) {
+if (llvm::Function *OpenMPRequiresDirectiveRegFun =
+OpenMPRuntime->emitRequiresDirectiveRegFun()) {
+  AddGlobalCtor(OpenMPRequiresDirectiveRegFun, 0, nullptr);
+}
 if (llvm::Function *OpenMPRegistrationFunction =
 OpenMPRuntime->emitRegistrationFunction()) {
   auto ComdatKey = OpenMPRegistrationFunction->hasComdat() ?
Index: lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
===
--- lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
+++ lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
@@ -4987,6 +4987,7 @@
   }
 }
   }
+  CGOpenMPRuntime::checkArchForUnifiedAddressing(D);
 }
 
 /// Get number of SMs and number of blocks per SM.
Index: lib/CodeGen/CGOpenMPRuntime.h
===
--- lib/CodeGen/CGOpenMPRuntime.h
+++ lib/CodeGen/CGOpenMPRuntime.h
@@ -636,6 +636,13 @@
   /// must be emitted.
   llvm::SmallDenseSet DeferredGlobalVariables;
 
+  /// Flag for keeping track of weather a requires unified_shared_memory
+  /// directive is present.
+  bool HasRequiresUnifiedSharedMemory = false;
+
+  /// Creates and registers requires directives.
+  llvm::Function *createRequiresDirectiveRegistration();
+
   /// Creates and registers offloading binary descriptor for the current
   /// compilation unit. The function that does the registration is returned.
   llvm::Function *createOffloadingBinaryDescriptorRegistration();
@@ -1429,6 +1436,10 @@
   /// \param GD Global to scan.
   virtual bool emitTargetGlobal(GlobalDecl GD);
 
+  /// Creates and returns a registration function for when at least one
+  /// requires directives was used in the current module.
+  virtual llvm::Function *emitRequiresDirectiveRegFun();
+
   /// Creates the offloading descriptor in the event any target region
   /// was emitted in the current module and return the function that registers
   /// it.
@@ -1597,7 +1608,10 @@
 
   /// Perform check on requires decl to ensure that target architecture
   /// supports unified addressing
-  virtual void checkArchForUnifiedAddressing(const OMPRequiresDecl *D) const {}
+  virtual void checkArchForUnifiedAddressing(const OMPRequiresDecl *D) const;
+
+  /// Return true if unified addressing is supported by the architecture.
+  virtual bool hasUnifiedAddressingSupport() const;
 
   /// Checks if the variable has associated OMPAllocateDeclAttr attribute with
   /// the predefined allocator and translates it into the corresponding address
Index: lib/CodeGen/CGOpenMPRuntime.cpp
===
--- lib/CodeGen/CGOpenMPRuntime.cpp
+++ lib/CodeGen/CGOpenMPRuntime.cpp
@@ -457,6 +457,26 @@
   LLVM_MARK_AS_BITMASK_ENUM(/*LargestValue=*/OMP_IDENT_WORK_DISTRIBUTE)
 };
 
+/// Values for bit flags for marking which requires clauses have been used.
+enum O

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

[Alexey Bataev via Phabricator via cfe-commits]

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 CGOpenMPRuntime class, 
it handles a reference to the CodeGenMode already.



Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:726
 
+enum OpenMPOffloadingRequiresDirFlags : int64_t {
+  /// no requires directive present.

You must use `LLVM_ENABLE_BITMASK_ENUMS_IN_NAMESPACE();` and 
`LLVM_MARK_AS_BITMASK_ENUM(/* LargestFlag = */)` from 
`clang/Basic/BitmaskEnum.h` to enable bit operations on your new bit-arithmetic 
based enumeric.



Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:726
 
+enum OpenMPOffloadingRequiresDirFlags : int64_t {
+  /// no requires directive present.

ABataev wrote:
> You must use `LLVM_ENABLE_BITMASK_ENUMS_IN_NAMESPACE();` and 
> `LLVM_MARK_AS_BITMASK_ENUM(/* LargestFlag = */)` from 
> `clang/Basic/BitmaskEnum.h` to enable bit operations on your new 
> bit-arithmetic based enumeric.
Add a comment for the whole new type



Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:728
+  /// no requires directive present.
+  OMP_REQ_NONE= 0x000,
+  /// reverse_offload clause.

You should not handle all the possible flags for the requires directives, since 
you try to support only target-specific flags.



Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:2322
+auto *FnTy =
+llvm::FunctionType::get(CGM.Int32Ty, TypeParams, /*isVarArg*/ false);
+RTLFn = CGM.CreateRuntimeFunction(FnTy, "__tgt_register_requires");

Function return type must be `void`



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

Why do you need a new member function? Can you make a static local function?



Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:3882
+const auto &FI =
+CGM.getTypes().arrangeBuiltinFunctionDeclaration(C.VoidTy, {});
+llvm::FunctionType *FTy = CGM.getTypes().GetFunctionType(FI);

Use `getTypes().arrangeNullaryFunction()`



Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:3887
+CGF.StartFunction(GlobalDecl(), C.VoidTy, RequiresRegFn, FI, {});
+int64_t Flags = OMP_REQ_NONE;
+//TODO: check for other requires clauses.

Use `OpenMPOffloadingRequiresDirFlags` instead of `int64_t`



Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:7983
 MapFlagsArrayTy &Types) const {
+// If using unified memory, no need to do the mappings.
+if (CGF.CGM.HasRequiresUnifiedSharedMemory)

Seems to me, this must be in another patch, has nothing to do with this patch



Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:9069
 
+llvm::Function *CGOpenMPRuntime::emitRequiresDirectiveRegFun() {
+  // Create and register the function that handles the requires directives.

Why do you need the second function?



Comment at: lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp:4946
+CodeGenModule &CGM, const OMPRequiresDecl *D) const {
+  CGOpenMPRuntime::checkArchForUnifiedAddressing(CGM, D);
   for (const OMPClause *Clause : D->clauselists()) {

Call this function after the target-specific processing.



Comment at: lib/CodeGen/CodeGenModule.h:294
 
+  bool HasRequiresUnifiedSharedMemory = false;
+

Why public? Must be private. Also, add comments for this new data member.
Also, I don't think this must be in CGM. It must be a member of CGOpenMPRuntime 
class.


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
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Gheorghe-Teodor Bercea via Phabricator via cfe-commits]

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.

Each requires directive clause will enable a particular flag to be set.

The set of flags is passed to the runtime to be checked for compatibility with 
other such flags coming from other object files.

The registration function is called whenever OpenMP is present even if a 
requires directive is not present. This helps detect cases in which requires 
directives are used inconsistently.


Repository:
  rC Clang

https://reviews.llvm.org/D60568

Files:
  lib/CodeGen/CGDecl.cpp
  lib/CodeGen/CGOpenMPRuntime.cpp
  lib/CodeGen/CGOpenMPRuntime.h
  lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
  lib/CodeGen/CGOpenMPRuntimeNVPTX.h
  lib/CodeGen/CodeGenModule.cpp
  lib/CodeGen/CodeGenModule.h
  test/OpenMP/openmp_offload_registration.cpp

Index: test/OpenMP/openmp_offload_registration.cpp
===
--- test/OpenMP/openmp_offload_registration.cpp
+++ test/OpenMP/openmp_offload_registration.cpp
@@ -26,7 +26,7 @@
 // CHECK: [[DESC:@.+]] = internal constant [[DSCTY]] { i32 2, [[DEVTY]]* getelementptr inbounds ([2 x [[DEVTY]]], [2 x [[DEVTY]]]* [[IMAGES]], i32 0, i32 0), [[ENTTY]]* [[ENTBEGIN]], [[ENTTY]]* [[ENTEND]] }, comdat($[[REGFN]])
 
 // Check target registration is registered as a Ctor.
-// CHECK: appending global [1 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 0, void ()* @[[REGFN]], i8* bitcast (void ()* @[[REGFN]] to i8*) }]
+// CHECK: appending global [2 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 0, void ()* @.omp_offloading.requires_reg, i8* null }, { i32, void ()*, i8* } { i32 0, void ()* @[[REGFN]], i8* bitcast (void ()* @[[REGFN]] to i8*) }]
 
 // Check presence of foo() and the outlined target region
 // CHECK: define void [[FOO:@.+]]()
@@ -34,6 +34,11 @@
 
 // Check registration and unregistration code.
 
+// CHECK: define internal void @.omp_offloading.requires_reg()
+// CHECK: call i32 @__tgt_register_requires(i64 0)
+// CHECK: ret void
+// CHECK: declare i32 @__tgt_register_requires(i64)
+
 // CHECK: define internal void @[[UNREGFN:.+]](i8*)
 // CHECK-SAME: comdat($[[REGFN]]) {
 // CHECK: call i32 @__tgt_unregister_lib([[DSCTY]]* [[DESC]])
Index: lib/CodeGen/CodeGenModule.h
===
--- lib/CodeGen/CodeGenModule.h
+++ lib/CodeGen/CodeGenModule.h
@@ -291,6 +291,8 @@
 
   typedef std::vector CtorList;
 
+  bool HasRequiresUnifiedSharedMemory = false;
+
 private:
   ASTContext &Context;
   const LangOptions &LangOpts;
Index: lib/CodeGen/CodeGenModule.cpp
===
--- lib/CodeGen/CodeGenModule.cpp
+++ lib/CodeGen/CodeGenModule.cpp
@@ -410,6 +410,10 @@
   AddGlobalCtor(CudaCtorFunction);
   }
   if (OpenMPRuntime) {
+if (llvm::Function *OpenMPRequiresDirectiveRegFun =
+OpenMPRuntime->emitRequiresDirectiveRegFun()) {
+  AddGlobalCtor(OpenMPRequiresDirectiveRegFun, 0, nullptr);
+}
 if (llvm::Function *OpenMPRegistrationFunction =
 OpenMPRuntime->emitRegistrationFunction()) {
   auto ComdatKey = OpenMPRegistrationFunction->hasComdat() ?
Index: lib/CodeGen/CGOpenMPRuntimeNVPTX.h
===
--- lib/CodeGen/CGOpenMPRuntimeNVPTX.h
+++ lib/CodeGen/CGOpenMPRuntimeNVPTX.h
@@ -383,7 +383,8 @@
 
   /// Perform check on requires decl to ensure that target architecture
   /// supports unified addressing
-  void checkArchForUnifiedAddressing(const OMPRequiresDecl *D) const override;
+  void checkArchForUnifiedAddressing(CodeGenModule &CGM,
+ const OMPRequiresDecl *D) const override;
 
   /// Returns default address space for the constant firstprivates, __constant__
   /// address space by default.
Index: lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
===
--- lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
+++ lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
@@ -4942,7 +4942,8 @@
 /// Check to see if target architecture supports unified addressing which is
 /// a restriction for OpenMP requires clause "unified_shared_memory".
 void CGOpenMPRuntimeNVPTX::checkArchForUnifiedAddressing(
-const OMPRequiresDecl *D) const {
+CodeGenModule &CGM, const OMPRequiresDecl *D) const {
+  CGOpenMPRuntime::checkArchForUnifiedAddressing(CGM, D);
   for (const OMPClause *Clause : D->clauselists()) {
 if (Clause->getClauseKind() == OMPC_unified_shared_memory) {
   switch (getCudaArch(CGM)) {
Index: lib/CodeGen/CGOpenMPRuntime.h
===
--- lib/CodeGen/CGOpenMPRuntime.h
+++ lib/CodeGen/CGOpen