[PATCH] D155692: [clang][OpenMP] Add interop support for multiple depend clauses

[Michael Halkenhäuser via Phabricator via cfe-commits]

mhalk created this revision.
Herald added subscribers: sunshaoce, guansong, yaxunl.
Herald added a project: All.
mhalk requested review of this revision.
Herald added a reviewer: jdoerfert.
Herald added subscribers: cfe-commits, jplehr, sstefan1.
Herald added a project: clang.

This patch removes the constraint of the `interop` directive where only a single
`depend` clause was allowed.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D155692

Files:
  clang/lib/CodeGen/CGStmtOpenMP.cpp
  clang/test/OpenMP/interop_irbuilder.cpp

Index: clang/test/OpenMP/interop_irbuilder.cpp
===
--- clang/test/OpenMP/interop_irbuilder.cpp
+++ clang/test/OpenMP/interop_irbuilder.cpp
@@ -7,7 +7,7 @@
 void test1() {
 
   int device_id = 4;
-  int D0, D1;
+  int D0, D1, D2;
   omp_interop_t interop;
 
 #pragma omp interop init(target : interop)
@@ -20,6 +20,9 @@
 
 #pragma omp interop use(interop) depend(in : D0, D1) nowait
 
+#pragma omp interop use(interop) depend(in : D0) depend(inout : D1) \
+depend(out : D2) nowait
+
 #pragma omp interop destroy(interop) depend(in : D0, D1)
 }
 
@@ -31,7 +34,7 @@
 void S::member_test() {
 
   int device_id = 4;
-  int D0, D1;
+  int D0, D1, D2;
 
 #pragma omp interop init(target : interop)
 
@@ -43,6 +46,9 @@
 
 #pragma omp interop use(interop) depend(in : D0, D1) nowait
 
+#pragma omp interop use(interop) depend(in : D0) depend(inout : D1) \
+depend(out : D2) nowait
+
 #pragma omp interop destroy(interop) depend(in : D0, D1)
 }
 // CHECK-LABEL: @_Z5test1v(
@@ -50,11 +56,14 @@
 // CHECK-NEXT:[[DEVICE_ID:%.*]] = alloca i32, align 4
 // CHECK-NEXT:[[D0:%.*]] = alloca i32, align 4
 // CHECK-NEXT:[[D1:%.*]] = alloca i32, align 4
+// CHECK-NEXT:[[D2:%.*]] = alloca i32, align 4
 // CHECK-NEXT:[[INTEROP:%.*]] = alloca ptr, align 8
 // CHECK-NEXT:[[DOTDEP_ARR_ADDR:%.*]] = alloca [2 x %struct.kmp_depend_info], align 8
 // CHECK-NEXT:[[DEP_COUNTER_ADDR:%.*]] = alloca i64, align 8
-// CHECK-NEXT:[[DOTDEP_ARR_ADDR5:%.*]] = alloca [2 x %struct.kmp_depend_info], align 8
+// CHECK-NEXT:[[DOTDEP_ARR_ADDR5:%.*]] = alloca [3 x %struct.kmp_depend_info], align 8
 // CHECK-NEXT:[[DEP_COUNTER_ADDR6:%.*]] = alloca i64, align 8
+// CHECK-NEXT:[[DOTDEP_ARR_ADDR8:%.*]] = alloca [2 x %struct.kmp_depend_info], align 8
+// CHECK-NEXT:[[DEP_COUNTER_ADDR9:%.*]] = alloca i64, align 8
 // CHECK-NEXT:store i32 4, ptr [[DEVICE_ID]], align 4
 // CHECK-NEXT:[[OMP_GLOBAL_THREAD_NUM:%.*]] = call i32 @__kmpc_global_thread_num(ptr @[[GLOB1:[0-9]+]])
 // CHECK-NEXT:call void @__tgt_interop_init(ptr @[[GLOB1]], i32 [[OMP_GLOBAL_THREAD_NUM]], ptr [[INTEROP]], i32 1, i32 -1, i32 0, ptr null, i32 0)
@@ -85,8 +94,8 @@
 // CHECK-NEXT:store i8 1, ptr [[TMP12]], align 8
 // CHECK-NEXT:store i64 2, ptr [[DEP_COUNTER_ADDR]], align 8
 // CHECK-NEXT:[[OMP_GLOBAL_THREAD_NUM4:%.*]] = call i32 @__kmpc_global_thread_num(ptr @[[GLOB1]])
-// CHECK-NEXT:call void @__tgt_interop_use(ptr @[[GLOB1]], i32 [[OMP_GLOBAL_THREAD_NUM4]], ptr [[INTEROP]], i32 -1, i32 2, ptr [[TMP2]], i32 1)
-// CHECK-NEXT:[[TMP13:%.*]] = getelementptr inbounds [2 x %struct.kmp_depend_info], ptr [[DOTDEP_ARR_ADDR5]], i64 0, i64 0
+// CHECK-NEXT:call void @__tgt_interop_use(ptr @[[GLOB1]], i32 [[OMP_GLOBAL_THREAD_NUM4]], ptr [[INTEROP]], i32 -1, i32 2, ptr [[TMP2]], i32 0)
+// CHECK-NEXT:[[TMP13:%.*]] = getelementptr inbounds [3 x %struct.kmp_depend_info], ptr [[DOTDEP_ARR_ADDR5]], i64 0, i64 0
 // CHECK-NEXT:[[TMP14:%.*]] = ptrtoint ptr [[D0]] to i64
 // CHECK-NEXT:[[TMP15:%.*]] = getelementptr [[STRUCT_KMP_DEPEND_INFO]], ptr [[TMP13]], i64 0
 // CHECK-NEXT:[[TMP16:%.*]] = getelementptr inbounds [[STRUCT_KMP_DEPEND_INFO]], ptr [[TMP15]], i32 0, i32 0
@@ -102,10 +111,38 @@
 // CHECK-NEXT:[[TMP22:%.*]] = getelementptr inbounds [[STRUCT_KMP_DEPEND_INFO]], ptr [[TMP20]], i32 0, i32 1
 // CHECK-NEXT:store i64 4, ptr [[TMP22]], align 8
 // CHECK-NEXT:[[TMP23:%.*]] = getelementptr inbounds [[STRUCT_KMP_DEPEND_INFO]], ptr [[TMP20]], i32 0, i32 2
-// CHECK-NEXT:store i8 1, ptr [[TMP23]], align 8
-// CHECK-NEXT:store i64 2, ptr [[DEP_COUNTER_ADDR6]], align 8
+// CHECK-NEXT:store i8 3, ptr [[TMP23]], align 8
+// CHECK-NEXT:[[TMP24:%.*]] = ptrtoint ptr [[D2]] to i64
+// CHECK-NEXT:[[TMP25:%.*]] = getelementptr [[STRUCT_KMP_DEPEND_INFO]], ptr [[TMP13]], i64 2
+// CHECK-NEXT:[[TMP26:%.*]] = getelementptr inbounds [[STRUCT_KMP_DEPEND_INFO]], ptr [[TMP25]], i32 0, i32 0
+// CHECK-NEXT:store i64 [[TMP24]], ptr [[TMP26]], align 8
+// CHECK-NEXT:[[TMP27:%.*]] = getelementptr inbounds [[STRUCT_KMP_DEPEND_INFO]], ptr [[TMP25]], i32 0, i32 1
+// CHECK-NEXT:store i64 4, ptr [[TMP27]], align 8
+// CHECK-NEXT:[[TMP28:%.*]] = getelementptr inbounds [[STRUCT_KMP_DEPEND_INFO]], ptr [[TMP25]], i32 0, i32 2
+// CHECK-NEXT:store i8 3, ptr [[TMP28]], align 8
+// CHECK

[PATCH] D155692: [clang][OpenMP] Add interop support for multiple depend clauses

[Michael Halkenhäuser via Phabricator via cfe-commits]

mhalk updated this revision to Diff 541942.
mhalk added a comment.

Fix accidental remove of `assert(...)`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155692

Files:
  clang/lib/CodeGen/CGStmtOpenMP.cpp
  clang/test/OpenMP/interop_irbuilder.cpp

Index: clang/test/OpenMP/interop_irbuilder.cpp
===
--- clang/test/OpenMP/interop_irbuilder.cpp
+++ clang/test/OpenMP/interop_irbuilder.cpp
@@ -7,7 +7,7 @@
 void test1() {
 
   int device_id = 4;
-  int D0, D1;
+  int D0, D1, D2;
   omp_interop_t interop;
 
 #pragma omp interop init(target : interop)
@@ -20,6 +20,9 @@
 
 #pragma omp interop use(interop) depend(in : D0, D1) nowait
 
+#pragma omp interop use(interop) depend(in : D0) depend(inout : D1) \
+depend(out : D2) nowait
+
 #pragma omp interop destroy(interop) depend(in : D0, D1)
 }
 
@@ -31,7 +34,7 @@
 void S::member_test() {
 
   int device_id = 4;
-  int D0, D1;
+  int D0, D1, D2;
 
 #pragma omp interop init(target : interop)
 
@@ -43,6 +46,9 @@
 
 #pragma omp interop use(interop) depend(in : D0, D1) nowait
 
+#pragma omp interop use(interop) depend(in : D0) depend(inout : D1) \
+depend(out : D2) nowait
+
 #pragma omp interop destroy(interop) depend(in : D0, D1)
 }
 // CHECK-LABEL: @_Z5test1v(
@@ -50,11 +56,14 @@
 // CHECK-NEXT:[[DEVICE_ID:%.*]] = alloca i32, align 4
 // CHECK-NEXT:[[D0:%.*]] = alloca i32, align 4
 // CHECK-NEXT:[[D1:%.*]] = alloca i32, align 4
+// CHECK-NEXT:[[D2:%.*]] = alloca i32, align 4
 // CHECK-NEXT:[[INTEROP:%.*]] = alloca ptr, align 8
 // CHECK-NEXT:[[DOTDEP_ARR_ADDR:%.*]] = alloca [2 x %struct.kmp_depend_info], align 8
 // CHECK-NEXT:[[DEP_COUNTER_ADDR:%.*]] = alloca i64, align 8
-// CHECK-NEXT:[[DOTDEP_ARR_ADDR5:%.*]] = alloca [2 x %struct.kmp_depend_info], align 8
+// CHECK-NEXT:[[DOTDEP_ARR_ADDR5:%.*]] = alloca [3 x %struct.kmp_depend_info], align 8
 // CHECK-NEXT:[[DEP_COUNTER_ADDR6:%.*]] = alloca i64, align 8
+// CHECK-NEXT:[[DOTDEP_ARR_ADDR8:%.*]] = alloca [2 x %struct.kmp_depend_info], align 8
+// CHECK-NEXT:[[DEP_COUNTER_ADDR9:%.*]] = alloca i64, align 8
 // CHECK-NEXT:store i32 4, ptr [[DEVICE_ID]], align 4
 // CHECK-NEXT:[[OMP_GLOBAL_THREAD_NUM:%.*]] = call i32 @__kmpc_global_thread_num(ptr @[[GLOB1:[0-9]+]])
 // CHECK-NEXT:call void @__tgt_interop_init(ptr @[[GLOB1]], i32 [[OMP_GLOBAL_THREAD_NUM]], ptr [[INTEROP]], i32 1, i32 -1, i32 0, ptr null, i32 0)
@@ -85,8 +94,8 @@
 // CHECK-NEXT:store i8 1, ptr [[TMP12]], align 8
 // CHECK-NEXT:store i64 2, ptr [[DEP_COUNTER_ADDR]], align 8
 // CHECK-NEXT:[[OMP_GLOBAL_THREAD_NUM4:%.*]] = call i32 @__kmpc_global_thread_num(ptr @[[GLOB1]])
-// CHECK-NEXT:call void @__tgt_interop_use(ptr @[[GLOB1]], i32 [[OMP_GLOBAL_THREAD_NUM4]], ptr [[INTEROP]], i32 -1, i32 2, ptr [[TMP2]], i32 1)
-// CHECK-NEXT:[[TMP13:%.*]] = getelementptr inbounds [2 x %struct.kmp_depend_info], ptr [[DOTDEP_ARR_ADDR5]], i64 0, i64 0
+// CHECK-NEXT:call void @__tgt_interop_use(ptr @[[GLOB1]], i32 [[OMP_GLOBAL_THREAD_NUM4]], ptr [[INTEROP]], i32 -1, i32 2, ptr [[TMP2]], i32 0)
+// CHECK-NEXT:[[TMP13:%.*]] = getelementptr inbounds [3 x %struct.kmp_depend_info], ptr [[DOTDEP_ARR_ADDR5]], i64 0, i64 0
 // CHECK-NEXT:[[TMP14:%.*]] = ptrtoint ptr [[D0]] to i64
 // CHECK-NEXT:[[TMP15:%.*]] = getelementptr [[STRUCT_KMP_DEPEND_INFO]], ptr [[TMP13]], i64 0
 // CHECK-NEXT:[[TMP16:%.*]] = getelementptr inbounds [[STRUCT_KMP_DEPEND_INFO]], ptr [[TMP15]], i32 0, i32 0
@@ -102,10 +111,38 @@
 // CHECK-NEXT:[[TMP22:%.*]] = getelementptr inbounds [[STRUCT_KMP_DEPEND_INFO]], ptr [[TMP20]], i32 0, i32 1
 // CHECK-NEXT:store i64 4, ptr [[TMP22]], align 8
 // CHECK-NEXT:[[TMP23:%.*]] = getelementptr inbounds [[STRUCT_KMP_DEPEND_INFO]], ptr [[TMP20]], i32 0, i32 2
-// CHECK-NEXT:store i8 1, ptr [[TMP23]], align 8
-// CHECK-NEXT:store i64 2, ptr [[DEP_COUNTER_ADDR6]], align 8
+// CHECK-NEXT:store i8 3, ptr [[TMP23]], align 8
+// CHECK-NEXT:[[TMP24:%.*]] = ptrtoint ptr [[D2]] to i64
+// CHECK-NEXT:[[TMP25:%.*]] = getelementptr [[STRUCT_KMP_DEPEND_INFO]], ptr [[TMP13]], i64 2
+// CHECK-NEXT:[[TMP26:%.*]] = getelementptr inbounds [[STRUCT_KMP_DEPEND_INFO]], ptr [[TMP25]], i32 0, i32 0
+// CHECK-NEXT:store i64 [[TMP24]], ptr [[TMP26]], align 8
+// CHECK-NEXT:[[TMP27:%.*]] = getelementptr inbounds [[STRUCT_KMP_DEPEND_INFO]], ptr [[TMP25]], i32 0, i32 1
+// CHECK-NEXT:store i64 4, ptr [[TMP27]], align 8
+// CHECK-NEXT:[[TMP28:%.*]] = getelementptr inbounds [[STRUCT_KMP_DEPEND_INFO]], ptr [[TMP25]], i32 0, i32 2
+// CHECK-NEXT:store i8 3, ptr [[TMP28]], align 8
+// CHECK-NEXT:store i64 3, ptr [[DEP_COUNTER_ADDR6]], align 8
 // CHECK-NEXT:[[OMP_GLOBAL_THREAD_NUM7:%.*]] = call i32 @__kmpc_global_thread_num(ptr @[[GLOB1]])
-// CHECK-NEXT:call void @__tgt_interop_destroy(p

[PATCH] D155692: [clang][OpenMP] Add interop support for multiple depend clauses

[Michael Halkenhäuser via Phabricator via cfe-commits]

mhalk added inline comments.



Comment at: clang/lib/CodeGen/CGStmtOpenMP.cpp:6986
+DependenceAddress = DependenciesArray.getPointer();
+CodeGenFunction::RunCleanupsScope LocalScope(*this);
   }

Not sure if this is useful here.
Saw this `RunCleanupsScope` in conjunction with the emission of dependences + 
calls.

But, at least in the generated IR it won't remove duplicate `depend` clauses.

If anybody can comment on this, that would be great.
Otherwise, I'd tend to remove it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155692

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

[PATCH] D155692: [clang][OpenMP] Add interop support for multiple depend clauses

[Michael Halkenhäuser via Phabricator via cfe-commits]

mhalk added inline comments.



Comment at: clang/lib/CodeGen/CGStmtOpenMP.cpp:6986
+DependenceAddress = DependenciesArray.getPointer();
+CodeGenFunction::RunCleanupsScope LocalScope(*this);
   }

ABataev wrote:
> mhalk wrote:
> > Not sure if this is useful here.
> > Saw this `RunCleanupsScope` in conjunction with the emission of dependences 
> > + calls.
> > 
> > But, at least in the generated IR it won't remove duplicate `depend` 
> > clauses.
> > 
> > If anybody can comment on this, that would be great.
> > Otherwise, I'd tend to remove it.
> Not required here
Thank you! Will be removed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155692

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

[PATCH] D155692: [clang][OpenMP] Add interop support for multiple depend clauses

[Michael Halkenhäuser via Phabricator via cfe-commits]

mhalk updated this revision to Diff 542032.
mhalk added a comment.

Rebase + removed `RunCleanupsScope` for preparation w.r.t. D137607 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155692

Files:
  clang/lib/CodeGen/CGStmtOpenMP.cpp
  clang/test/OpenMP/interop_irbuilder.cpp

Index: clang/test/OpenMP/interop_irbuilder.cpp
===
--- clang/test/OpenMP/interop_irbuilder.cpp
+++ clang/test/OpenMP/interop_irbuilder.cpp
@@ -7,7 +7,7 @@
 void test1() {
 
   int device_id = 4;
-  int D0, D1;
+  int D0, D1, D2;
   omp_interop_t interop;
 
 #pragma omp interop init(target : interop)
@@ -20,6 +20,9 @@
 
 #pragma omp interop use(interop) depend(in : D0, D1) nowait
 
+#pragma omp interop use(interop) depend(in : D0) depend(inout : D1) \
+depend(out : D2) nowait
+
 #pragma omp interop destroy(interop) depend(in : D0, D1)
 }
 
@@ -31,7 +34,7 @@
 void S::member_test() {
 
   int device_id = 4;
-  int D0, D1;
+  int D0, D1, D2;
 
 #pragma omp interop init(target : interop)
 
@@ -43,6 +46,9 @@
 
 #pragma omp interop use(interop) depend(in : D0, D1) nowait
 
+#pragma omp interop use(interop) depend(in : D0) depend(inout : D1) \
+depend(out : D2) nowait
+
 #pragma omp interop destroy(interop) depend(in : D0, D1)
 }
 // CHECK-LABEL: @_Z5test1v(
@@ -50,11 +56,14 @@
 // CHECK-NEXT:[[DEVICE_ID:%.*]] = alloca i32, align 4
 // CHECK-NEXT:[[D0:%.*]] = alloca i32, align 4
 // CHECK-NEXT:[[D1:%.*]] = alloca i32, align 4
+// CHECK-NEXT:[[D2:%.*]] = alloca i32, align 4
 // CHECK-NEXT:[[INTEROP:%.*]] = alloca ptr, align 8
 // CHECK-NEXT:[[DOTDEP_ARR_ADDR:%.*]] = alloca [2 x %struct.kmp_depend_info], align 8
 // CHECK-NEXT:[[DEP_COUNTER_ADDR:%.*]] = alloca i64, align 8
-// CHECK-NEXT:[[DOTDEP_ARR_ADDR5:%.*]] = alloca [2 x %struct.kmp_depend_info], align 8
+// CHECK-NEXT:[[DOTDEP_ARR_ADDR5:%.*]] = alloca [3 x %struct.kmp_depend_info], align 8
 // CHECK-NEXT:[[DEP_COUNTER_ADDR6:%.*]] = alloca i64, align 8
+// CHECK-NEXT:[[DOTDEP_ARR_ADDR8:%.*]] = alloca [2 x %struct.kmp_depend_info], align 8
+// CHECK-NEXT:[[DEP_COUNTER_ADDR9:%.*]] = alloca i64, align 8
 // CHECK-NEXT:store i32 4, ptr [[DEVICE_ID]], align 4
 // CHECK-NEXT:[[OMP_GLOBAL_THREAD_NUM:%.*]] = call i32 @__kmpc_global_thread_num(ptr @[[GLOB1:[0-9]+]])
 // CHECK-NEXT:call void @__tgt_interop_init(ptr @[[GLOB1]], i32 [[OMP_GLOBAL_THREAD_NUM]], ptr [[INTEROP]], i32 1, i32 -1, i32 0, ptr null, i32 0)
@@ -85,8 +94,8 @@
 // CHECK-NEXT:store i8 1, ptr [[TMP12]], align 8
 // CHECK-NEXT:store i64 2, ptr [[DEP_COUNTER_ADDR]], align 8
 // CHECK-NEXT:[[OMP_GLOBAL_THREAD_NUM4:%.*]] = call i32 @__kmpc_global_thread_num(ptr @[[GLOB1]])
-// CHECK-NEXT:call void @__tgt_interop_use(ptr @[[GLOB1]], i32 [[OMP_GLOBAL_THREAD_NUM4]], ptr [[INTEROP]], i32 -1, i32 2, ptr [[TMP2]], i32 1)
-// CHECK-NEXT:[[TMP13:%.*]] = getelementptr inbounds [2 x %struct.kmp_depend_info], ptr [[DOTDEP_ARR_ADDR5]], i64 0, i64 0
+// CHECK-NEXT:call void @__tgt_interop_use(ptr @[[GLOB1]], i32 [[OMP_GLOBAL_THREAD_NUM4]], ptr [[INTEROP]], i32 -1, i32 2, ptr [[TMP2]], i32 0)
+// CHECK-NEXT:[[TMP13:%.*]] = getelementptr inbounds [3 x %struct.kmp_depend_info], ptr [[DOTDEP_ARR_ADDR5]], i64 0, i64 0
 // CHECK-NEXT:[[TMP14:%.*]] = ptrtoint ptr [[D0]] to i64
 // CHECK-NEXT:[[TMP15:%.*]] = getelementptr [[STRUCT_KMP_DEPEND_INFO]], ptr [[TMP13]], i64 0
 // CHECK-NEXT:[[TMP16:%.*]] = getelementptr inbounds [[STRUCT_KMP_DEPEND_INFO]], ptr [[TMP15]], i32 0, i32 0
@@ -102,10 +111,38 @@
 // CHECK-NEXT:[[TMP22:%.*]] = getelementptr inbounds [[STRUCT_KMP_DEPEND_INFO]], ptr [[TMP20]], i32 0, i32 1
 // CHECK-NEXT:store i64 4, ptr [[TMP22]], align 8
 // CHECK-NEXT:[[TMP23:%.*]] = getelementptr inbounds [[STRUCT_KMP_DEPEND_INFO]], ptr [[TMP20]], i32 0, i32 2
-// CHECK-NEXT:store i8 1, ptr [[TMP23]], align 8
-// CHECK-NEXT:store i64 2, ptr [[DEP_COUNTER_ADDR6]], align 8
+// CHECK-NEXT:store i8 3, ptr [[TMP23]], align 8
+// CHECK-NEXT:[[TMP24:%.*]] = ptrtoint ptr [[D2]] to i64
+// CHECK-NEXT:[[TMP25:%.*]] = getelementptr [[STRUCT_KMP_DEPEND_INFO]], ptr [[TMP13]], i64 2
+// CHECK-NEXT:[[TMP26:%.*]] = getelementptr inbounds [[STRUCT_KMP_DEPEND_INFO]], ptr [[TMP25]], i32 0, i32 0
+// CHECK-NEXT:store i64 [[TMP24]], ptr [[TMP26]], align 8
+// CHECK-NEXT:[[TMP27:%.*]] = getelementptr inbounds [[STRUCT_KMP_DEPEND_INFO]], ptr [[TMP25]], i32 0, i32 1
+// CHECK-NEXT:store i64 4, ptr [[TMP27]], align 8
+// CHECK-NEXT:[[TMP28:%.*]] = getelementptr inbounds [[STRUCT_KMP_DEPEND_INFO]], ptr [[TMP25]], i32 0, i32 2
+// CHECK-NEXT:store i8 3, ptr [[TMP28]], align 8
+// CHECK-NEXT:store i64 3, ptr [[DEP_COUNTER_ADDR6]], align 8
 // CHECK-NEXT:[[OMP_GLOBAL_THREAD_NUM7:%.*]] = call i32 @__kmpc_global_thread_num(ptr @[

[PATCH] D155692: [clang][OpenMP] Add interop support for multiple depend clauses

[Michael Halkenhäuser via Phabricator via cfe-commits]

mhalk added a comment.

In D155692#4515265 , @tianshilei1992 
wrote:

> We don't need to update the interface function to indicate how many deps we 
> have?

Which //interface function// exactly? `__tgt_interop_[init|use|destroy]`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155692

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

[PATCH] D155692: [clang][OpenMP] Add interop support for multiple depend clauses

[Michael Halkenhäuser via Phabricator via cfe-commits]

mhalk added a comment.

As far as I can tell, no.
This *should* already be covered by Ndeps + DepList from `__tgt_interop_init`'s 
signature:
__tgt_interop_init(ident_t *LocRef, kmp_int32 Gtid,

  [...],
  kmp_int32 Ndeps, kmp_depend_info_t *DepList,
  kmp_int32 HaveNowait)

Which is very similar to the signature of `__kmpc_omp_taskwait_deps_51` -- 
which also uses the result of `buildDependences`:
__kmpc_omp_taskwait_deps_51(ident_t *loc_ref, kmp_int32 gtid,

  kmp_int32 ndeps, kmp_depend_info_t *dep_list,
  [...], kmp_int32 has_no_wait);

I guess the naming should be improved, changed that to `DependenceList`.
And I did not set the `HasNowaitClause` correctly, will update immediately.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155692

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

[PATCH] D155692: [clang][OpenMP] Add interop support for multiple depend clauses

[Michael Halkenhäuser via Phabricator via cfe-commits]

mhalk updated this revision to Diff 542130.
mhalk added a comment.

Fixed `HasNowaitClause` always set to zero
Renamed corresponding variable to `DependenceList`, to better reflect its 
meaning.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155692

Files:
  clang/lib/CodeGen/CGStmtOpenMP.cpp
  clang/test/OpenMP/interop_irbuilder.cpp

Index: clang/test/OpenMP/interop_irbuilder.cpp
===
--- clang/test/OpenMP/interop_irbuilder.cpp
+++ clang/test/OpenMP/interop_irbuilder.cpp
@@ -7,7 +7,7 @@
 void test1() {
 
   int device_id = 4;
-  int D0, D1;
+  int D0, D1, D2;
   omp_interop_t interop;
 
 #pragma omp interop init(target : interop)
@@ -20,6 +20,9 @@
 
 #pragma omp interop use(interop) depend(in : D0, D1) nowait
 
+#pragma omp interop use(interop) depend(in : D0) depend(inout : D1) \
+depend(out : D2) nowait
+
 #pragma omp interop destroy(interop) depend(in : D0, D1)
 }
 
@@ -31,7 +34,7 @@
 void S::member_test() {
 
   int device_id = 4;
-  int D0, D1;
+  int D0, D1, D2;
 
 #pragma omp interop init(target : interop)
 
@@ -43,6 +46,9 @@
 
 #pragma omp interop use(interop) depend(in : D0, D1) nowait
 
+#pragma omp interop use(interop) depend(in : D0) depend(inout : D1) \
+depend(out : D2) nowait
+
 #pragma omp interop destroy(interop) depend(in : D0, D1)
 }
 // CHECK-LABEL: @_Z5test1v(
@@ -50,11 +56,14 @@
 // CHECK-NEXT:[[DEVICE_ID:%.*]] = alloca i32, align 4
 // CHECK-NEXT:[[D0:%.*]] = alloca i32, align 4
 // CHECK-NEXT:[[D1:%.*]] = alloca i32, align 4
+// CHECK-NEXT:[[D2:%.*]] = alloca i32, align 4
 // CHECK-NEXT:[[INTEROP:%.*]] = alloca ptr, align 8
 // CHECK-NEXT:[[DOTDEP_ARR_ADDR:%.*]] = alloca [2 x %struct.kmp_depend_info], align 8
 // CHECK-NEXT:[[DEP_COUNTER_ADDR:%.*]] = alloca i64, align 8
-// CHECK-NEXT:[[DOTDEP_ARR_ADDR5:%.*]] = alloca [2 x %struct.kmp_depend_info], align 8
+// CHECK-NEXT:[[DOTDEP_ARR_ADDR5:%.*]] = alloca [3 x %struct.kmp_depend_info], align 8
 // CHECK-NEXT:[[DEP_COUNTER_ADDR6:%.*]] = alloca i64, align 8
+// CHECK-NEXT:[[DOTDEP_ARR_ADDR8:%.*]] = alloca [2 x %struct.kmp_depend_info], align 8
+// CHECK-NEXT:[[DEP_COUNTER_ADDR9:%.*]] = alloca i64, align 8
 // CHECK-NEXT:store i32 4, ptr [[DEVICE_ID]], align 4
 // CHECK-NEXT:[[OMP_GLOBAL_THREAD_NUM:%.*]] = call i32 @__kmpc_global_thread_num(ptr @[[GLOB1:[0-9]+]])
 // CHECK-NEXT:call void @__tgt_interop_init(ptr @[[GLOB1]], i32 [[OMP_GLOBAL_THREAD_NUM]], ptr [[INTEROP]], i32 1, i32 -1, i32 0, ptr null, i32 0)
@@ -86,7 +95,7 @@
 // CHECK-NEXT:store i64 2, ptr [[DEP_COUNTER_ADDR]], align 8
 // CHECK-NEXT:[[OMP_GLOBAL_THREAD_NUM4:%.*]] = call i32 @__kmpc_global_thread_num(ptr @[[GLOB1]])
 // CHECK-NEXT:call void @__tgt_interop_use(ptr @[[GLOB1]], i32 [[OMP_GLOBAL_THREAD_NUM4]], ptr [[INTEROP]], i32 -1, i32 2, ptr [[TMP2]], i32 1)
-// CHECK-NEXT:[[TMP13:%.*]] = getelementptr inbounds [2 x %struct.kmp_depend_info], ptr [[DOTDEP_ARR_ADDR5]], i64 0, i64 0
+// CHECK-NEXT:[[TMP13:%.*]] = getelementptr inbounds [3 x %struct.kmp_depend_info], ptr [[DOTDEP_ARR_ADDR5]], i64 0, i64 0
 // CHECK-NEXT:[[TMP14:%.*]] = ptrtoint ptr [[D0]] to i64
 // CHECK-NEXT:[[TMP15:%.*]] = getelementptr [[STRUCT_KMP_DEPEND_INFO]], ptr [[TMP13]], i64 0
 // CHECK-NEXT:[[TMP16:%.*]] = getelementptr inbounds [[STRUCT_KMP_DEPEND_INFO]], ptr [[TMP15]], i32 0, i32 0
@@ -102,10 +111,38 @@
 // CHECK-NEXT:[[TMP22:%.*]] = getelementptr inbounds [[STRUCT_KMP_DEPEND_INFO]], ptr [[TMP20]], i32 0, i32 1
 // CHECK-NEXT:store i64 4, ptr [[TMP22]], align 8
 // CHECK-NEXT:[[TMP23:%.*]] = getelementptr inbounds [[STRUCT_KMP_DEPEND_INFO]], ptr [[TMP20]], i32 0, i32 2
-// CHECK-NEXT:store i8 1, ptr [[TMP23]], align 8
-// CHECK-NEXT:store i64 2, ptr [[DEP_COUNTER_ADDR6]], align 8
+// CHECK-NEXT:store i8 3, ptr [[TMP23]], align 8
+// CHECK-NEXT:[[TMP24:%.*]] = ptrtoint ptr [[D2]] to i64
+// CHECK-NEXT:[[TMP25:%.*]] = getelementptr [[STRUCT_KMP_DEPEND_INFO]], ptr [[TMP13]], i64 2
+// CHECK-NEXT:[[TMP26:%.*]] = getelementptr inbounds [[STRUCT_KMP_DEPEND_INFO]], ptr [[TMP25]], i32 0, i32 0
+// CHECK-NEXT:store i64 [[TMP24]], ptr [[TMP26]], align 8
+// CHECK-NEXT:[[TMP27:%.*]] = getelementptr inbounds [[STRUCT_KMP_DEPEND_INFO]], ptr [[TMP25]], i32 0, i32 1
+// CHECK-NEXT:store i64 4, ptr [[TMP27]], align 8
+// CHECK-NEXT:[[TMP28:%.*]] = getelementptr inbounds [[STRUCT_KMP_DEPEND_INFO]], ptr [[TMP25]], i32 0, i32 2
+// CHECK-NEXT:store i8 3, ptr [[TMP28]], align 8
+// CHECK-NEXT:store i64 3, ptr [[DEP_COUNTER_ADDR6]], align 8
 // CHECK-NEXT:[[OMP_GLOBAL_THREAD_NUM7:%.*]] = call i32 @__kmpc_global_thread_num(ptr @[[GLOB1]])
-// CHECK-NEXT:call void @__tgt_interop_destroy(ptr @[[GLOB1]], i32 [[OMP_GLOBAL_THREAD_NUM7]], ptr [[INTEROP]], i32 -1, i32 2, ptr [[TMP13]], i32 0)
+// CHECK-NEXT:

[PATCH] D155692: [clang][OpenMP] Add interop support for multiple depend clauses

[Michael Halkenhäuser via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG7d4e14c76b4e: [clang][OpenMP] Add interop support for 
multiple depend clauses (authored by mhalk).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155692

Files:
  clang/lib/CodeGen/CGStmtOpenMP.cpp
  clang/test/OpenMP/interop_irbuilder.cpp

Index: clang/test/OpenMP/interop_irbuilder.cpp
===
--- clang/test/OpenMP/interop_irbuilder.cpp
+++ clang/test/OpenMP/interop_irbuilder.cpp
@@ -7,7 +7,7 @@
 void test1() {
 
   int device_id = 4;
-  int D0, D1;
+  int D0, D1, D2;
   omp_interop_t interop;
 
 #pragma omp interop init(target : interop)
@@ -20,6 +20,9 @@
 
 #pragma omp interop use(interop) depend(in : D0, D1) nowait
 
+#pragma omp interop use(interop) depend(in : D0) depend(inout : D1) \
+depend(out : D2) nowait
+
 #pragma omp interop destroy(interop) depend(in : D0, D1)
 }
 
@@ -31,7 +34,7 @@
 void S::member_test() {
 
   int device_id = 4;
-  int D0, D1;
+  int D0, D1, D2;
 
 #pragma omp interop init(target : interop)
 
@@ -43,6 +46,9 @@
 
 #pragma omp interop use(interop) depend(in : D0, D1) nowait
 
+#pragma omp interop use(interop) depend(in : D0) depend(inout : D1) \
+depend(out : D2) nowait
+
 #pragma omp interop destroy(interop) depend(in : D0, D1)
 }
 // CHECK-LABEL: @_Z5test1v(
@@ -50,11 +56,14 @@
 // CHECK-NEXT:[[DEVICE_ID:%.*]] = alloca i32, align 4
 // CHECK-NEXT:[[D0:%.*]] = alloca i32, align 4
 // CHECK-NEXT:[[D1:%.*]] = alloca i32, align 4
+// CHECK-NEXT:[[D2:%.*]] = alloca i32, align 4
 // CHECK-NEXT:[[INTEROP:%.*]] = alloca ptr, align 8
 // CHECK-NEXT:[[DOTDEP_ARR_ADDR:%.*]] = alloca [2 x %struct.kmp_depend_info], align 8
 // CHECK-NEXT:[[DEP_COUNTER_ADDR:%.*]] = alloca i64, align 8
-// CHECK-NEXT:[[DOTDEP_ARR_ADDR5:%.*]] = alloca [2 x %struct.kmp_depend_info], align 8
+// CHECK-NEXT:[[DOTDEP_ARR_ADDR5:%.*]] = alloca [3 x %struct.kmp_depend_info], align 8
 // CHECK-NEXT:[[DEP_COUNTER_ADDR6:%.*]] = alloca i64, align 8
+// CHECK-NEXT:[[DOTDEP_ARR_ADDR8:%.*]] = alloca [2 x %struct.kmp_depend_info], align 8
+// CHECK-NEXT:[[DEP_COUNTER_ADDR9:%.*]] = alloca i64, align 8
 // CHECK-NEXT:store i32 4, ptr [[DEVICE_ID]], align 4
 // CHECK-NEXT:[[OMP_GLOBAL_THREAD_NUM:%.*]] = call i32 @__kmpc_global_thread_num(ptr @[[GLOB1:[0-9]+]])
 // CHECK-NEXT:call void @__tgt_interop_init(ptr @[[GLOB1]], i32 [[OMP_GLOBAL_THREAD_NUM]], ptr [[INTEROP]], i32 1, i32 -1, i32 0, ptr null, i32 0)
@@ -86,7 +95,7 @@
 // CHECK-NEXT:store i64 2, ptr [[DEP_COUNTER_ADDR]], align 8
 // CHECK-NEXT:[[OMP_GLOBAL_THREAD_NUM4:%.*]] = call i32 @__kmpc_global_thread_num(ptr @[[GLOB1]])
 // CHECK-NEXT:call void @__tgt_interop_use(ptr @[[GLOB1]], i32 [[OMP_GLOBAL_THREAD_NUM4]], ptr [[INTEROP]], i32 -1, i32 2, ptr [[TMP2]], i32 1)
-// CHECK-NEXT:[[TMP13:%.*]] = getelementptr inbounds [2 x %struct.kmp_depend_info], ptr [[DOTDEP_ARR_ADDR5]], i64 0, i64 0
+// CHECK-NEXT:[[TMP13:%.*]] = getelementptr inbounds [3 x %struct.kmp_depend_info], ptr [[DOTDEP_ARR_ADDR5]], i64 0, i64 0
 // CHECK-NEXT:[[TMP14:%.*]] = ptrtoint ptr [[D0]] to i64
 // CHECK-NEXT:[[TMP15:%.*]] = getelementptr [[STRUCT_KMP_DEPEND_INFO]], ptr [[TMP13]], i64 0
 // CHECK-NEXT:[[TMP16:%.*]] = getelementptr inbounds [[STRUCT_KMP_DEPEND_INFO]], ptr [[TMP15]], i32 0, i32 0
@@ -102,10 +111,38 @@
 // CHECK-NEXT:[[TMP22:%.*]] = getelementptr inbounds [[STRUCT_KMP_DEPEND_INFO]], ptr [[TMP20]], i32 0, i32 1
 // CHECK-NEXT:store i64 4, ptr [[TMP22]], align 8
 // CHECK-NEXT:[[TMP23:%.*]] = getelementptr inbounds [[STRUCT_KMP_DEPEND_INFO]], ptr [[TMP20]], i32 0, i32 2
-// CHECK-NEXT:store i8 1, ptr [[TMP23]], align 8
-// CHECK-NEXT:store i64 2, ptr [[DEP_COUNTER_ADDR6]], align 8
+// CHECK-NEXT:store i8 3, ptr [[TMP23]], align 8
+// CHECK-NEXT:[[TMP24:%.*]] = ptrtoint ptr [[D2]] to i64
+// CHECK-NEXT:[[TMP25:%.*]] = getelementptr [[STRUCT_KMP_DEPEND_INFO]], ptr [[TMP13]], i64 2
+// CHECK-NEXT:[[TMP26:%.*]] = getelementptr inbounds [[STRUCT_KMP_DEPEND_INFO]], ptr [[TMP25]], i32 0, i32 0
+// CHECK-NEXT:store i64 [[TMP24]], ptr [[TMP26]], align 8
+// CHECK-NEXT:[[TMP27:%.*]] = getelementptr inbounds [[STRUCT_KMP_DEPEND_INFO]], ptr [[TMP25]], i32 0, i32 1
+// CHECK-NEXT:store i64 4, ptr [[TMP27]], align 8
+// CHECK-NEXT:[[TMP28:%.*]] = getelementptr inbounds [[STRUCT_KMP_DEPEND_INFO]], ptr [[TMP25]], i32 0, i32 2
+// CHECK-NEXT:store i8 3, ptr [[TMP28]], align 8
+// CHECK-NEXT:store i64 3, ptr [[DEP_COUNTER_ADDR6]], align 8
 // CHECK-NEXT:[[OMP_GLOBAL_THREAD_NUM7:%.*]] = call i32 @__kmpc_global_thread_num(ptr @[[GLOB1]])
-// CHECK-NEXT:call void @__tgt_interop_destroy(ptr @[[GLOB1]], i32 [[OMP_GLOBAL_THREAD_NUM7]], ptr [[INTEROP]], i32 -1, i32 2, ptr [[TMP13]], i32 0)
+// CHECK-NEXT:   

[PATCH] D156336: [clang][docs][OpenMP] Update status of OMPT support.

[Michael Halkenhäuser via Phabricator via cfe-commits]

mhalk created this revision.
mhalk added a reviewer: dhruvachak.
Herald added subscribers: sunshaoce, guansong, yaxunl.
Herald added a project: All.
mhalk requested review of this revision.
Herald added a reviewer: jdoerfert.
Herald added subscribers: cfe-commits, jplehr, sstefan1.
Herald added a project: clang.

Update documentation on implementation status of OMPT.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D156336

Files:
  clang/docs/OpenMPSupport.rst


Index: clang/docs/OpenMPSupport.rst
===
--- clang/docs/OpenMPSupport.rst
+++ clang/docs/OpenMPSupport.rst
@@ -140,7 +140,7 @@
 
+--+--+--+---+
 | OMPD | OMPD interfaces   
   | :part:`not upstream` | 
https://github.com/OpenMPToolsInterface/LLVM-openmp/tree/ompd-tests   |
 
+--+--+--+---+
-| OMPT | OMPT interfaces   
   | :part:`mostly done`  | 
  |
+| OMPT | OMPT interfaces (callback support)
   | :good:`done` | 
  |
 
+--+--+--+---+
 | thread affinity  | thread affinity   
   | :good:`done` | 
  |
 
+--+--+--+---+
@@ -355,7 +355,9 @@
 
+--+--+--+---+
 | OMPT | new barrier state values added to 
ompt_state_t enum  | :none:`unclaimed`| 
  |
 
+--+--+--+---+
-| OMPT | new 'emi' callbacks for external monitoring 
interfaces   | :none:`unclaimed`|   
|
+| OMPT | new 'emi' callbacks for external monitoring 
interfaces   | :good:`done` |   
|
++--+--+--+---+
+| OMPT | device tracing interface  
   | :none:`unclaimed`| 
  |
 
+--+--+--+---+
 | task | 'strict' modifier for taskloop construct  
   | :none:`unclaimed`| 
  |
 
+--+--+--+---+


Index: clang/docs/OpenMPSupport.rst
===
--- clang/docs/OpenMPSupport.rst
+++ clang/docs/OpenMPSupport.rst
@@ -140,7 +140,7 @@
 +--+--+--+---+
 | OMPD | OMPD interfaces  | :part:`not upstream` | https://github.com/OpenMPToolsInterface/LLVM-openmp/tree/ompd-tests   |
 +--+--+--+---

[PATCH] D156336: [clang][docs][OpenMP] Update status of OMPT support.

[Michael Halkenhäuser via Phabricator via cfe-commits]

mhalk added a comment.

Please, take a look as I've also marked the EMI callbacks as done and if the 
other added/changed descriptions match.
Just want to point out: the lower section is `5.1` (I was a bit confused as 
many people talked about 5.2).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156336

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

[PATCH] D156336: [clang][docs][OpenMP] Update status of OMPT support.

[Michael Halkenhäuser via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG99ce17b71c41: [clang][docs][OpenMP] Update status of OMPT 
support. (authored by mhalk).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156336

Files:
  clang/docs/OpenMPSupport.rst


Index: clang/docs/OpenMPSupport.rst
===
--- clang/docs/OpenMPSupport.rst
+++ clang/docs/OpenMPSupport.rst
@@ -140,7 +140,7 @@
 
+--+--+--+---+
 | OMPD | OMPD interfaces   
   | :part:`not upstream` | 
https://github.com/OpenMPToolsInterface/LLVM-openmp/tree/ompd-tests   |
 
+--+--+--+---+
-| OMPT | OMPT interfaces   
   | :part:`mostly done`  | 
  |
+| OMPT | OMPT interfaces (callback support)
   | :good:`done` | 
  |
 
+--+--+--+---+
 | thread affinity  | thread affinity   
   | :good:`done` | 
  |
 
+--+--+--+---+
@@ -355,7 +355,9 @@
 
+--+--+--+---+
 | OMPT | new barrier state values added to 
ompt_state_t enum  | :none:`unclaimed`| 
  |
 
+--+--+--+---+
-| OMPT | new 'emi' callbacks for external monitoring 
interfaces   | :none:`unclaimed`|   
|
+| OMPT | new 'emi' callbacks for external monitoring 
interfaces   | :good:`done` |   
|
++--+--+--+---+
+| OMPT | device tracing interface  
   | :none:`unclaimed`| 
  |
 
+--+--+--+---+
 | task | 'strict' modifier for taskloop construct  
   | :none:`unclaimed`| 
  |
 
+--+--+--+---+


Index: clang/docs/OpenMPSupport.rst
===
--- clang/docs/OpenMPSupport.rst
+++ clang/docs/OpenMPSupport.rst
@@ -140,7 +140,7 @@
 +--+--+--+---+
 | OMPD | OMPD interfaces  | :part:`not upstream` | https://github.com/OpenMPToolsInterface/LLVM-openmp/tree/ompd-tests   |
 +--+--+--+---+
-| OMPT | OM

[PATCH] D145591: [clang][HIP][OpenMP] Add warning if mixed HIP / OpenMP offloading

[Michael Halkenhäuser via Phabricator via cfe-commits]

mhalk added a comment.

In D145591#4182360 , @jhuber6 wrote:

> I'm not a fan of the same warning being copied in 24 places. Why do we set 
> `LangOpts.IsOpenMP` on the GPU compilation side, couldn't we just filter out 
> the `-fopenmp` or whatever it is for the HIP job?

If your concern was on the clang / SemA side, I found appropriate helper 
functions to reduce the number of checks (`if (getLangOpts().HIP) { Diag(...) 
}`) from 19 to 4.
But e.g. in the test we will still emit 24 messages, for affected directives.

I doubt reducing the number of diagnostic messages is worth the effort and esp. 
cost during compilation when tackled e.g. via clang-tooling?! (unsure)
The idea being: emission of a single diagnostic message and pointing out all 
(24) affected locations.

I may be completely wrong, but I suspect that users affected by "using HIP 
language but OpenMP target offloading behavior is altered" will (want to) 
disable this particular diagnostic once they are "aware".




Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:8635
+  "HIP does not support OpenMP target directives; directive has been ignored">,
+  InGroup;
+

>>! In D145591#4182945, @jdoerfert wrote:
> I would emit an error. A warning only if we can ensure the code does 
> something sensible. Right now, I doubt that is the case, similarly I doubt we 
> actually ignore things.

@yaxunl Just wanted to point out: If we added `DefaultError` here, the SemA 
would emit an error as preferred by Johannes.
But one could still disable it via `-Wno-hip-omp-target-directives` even with 
`-Werror` (or also demote it via `-Wno-error=hip-omp-target-directives`).

Would that be acceptable for everyone?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145591

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

[PATCH] D145591: [clang][HIP][OpenMP] Add warning if mixed HIP / OpenMP offloading

[Michael Halkenhäuser via Phabricator via cfe-commits]

mhalk updated this revision to Diff 507409.
mhalk added a comment.

Patch rework, implementing the mentioned changes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145591

Files:
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaOpenMP.cpp
  clang/test/SemaOpenMP/hip-omp-mix.cpp

Index: clang/test/SemaOpenMP/hip-omp-mix.cpp
===
--- /dev/null
+++ clang/test/SemaOpenMP/hip-omp-mix.cpp
@@ -0,0 +1,165 @@
+// REQUIRES: x86-registered-target
+// REQUIRES: amdgpu-registered-target
+
+// RUN: %clang_cc1 %s -x c++ -fopenmp -fsyntax-only -verify=host
+// host-no-diagnostics
+
+// RUN: %clang_cc1 %s -x hip -fopenmp -fsyntax-only -verify=device
+// device-error@#01 {{HIP does not support OpenMP target directives}}
+// device-error@#02 {{HIP does not support OpenMP target directives}}
+// device-error@#03 {{HIP does not support OpenMP target directives}}
+// device-error@#04 {{HIP does not support OpenMP target directives}}
+// device-error@#05 {{HIP does not support OpenMP target directives}}
+// device-error@#06 {{HIP does not support OpenMP target directives}}
+// device-error@#07 {{HIP does not support OpenMP target directives}}
+// device-error@#08 {{HIP does not support OpenMP target directives}}
+// device-error@#09 {{HIP does not support OpenMP target directives}}
+// device-error@#10 {{HIP does not support OpenMP target directives}}
+// device-error@#11 {{HIP does not support OpenMP target directives}}
+// device-error@#12 {{HIP does not support OpenMP target directives}}
+// device-error@#13 {{HIP does not support OpenMP target directives}}
+// device-error@#14 {{HIP does not support OpenMP target directives}}
+// device-error@#15 {{HIP does not support OpenMP target directives}}
+// device-error@#16 {{HIP does not support OpenMP target directives}}
+// device-error@#17 {{HIP does not support OpenMP target directives}}
+// device-error@#18 {{HIP does not support OpenMP target directives}}
+// device-error@#19 {{HIP does not support OpenMP target directives}}
+// device-error@#20 {{HIP does not support OpenMP target directives}}
+// device-error@#21 {{HIP does not support OpenMP target directives}}
+// device-error@#22 {{HIP does not support OpenMP target directives}}
+// device-error@#23 {{HIP does not support OpenMP target directives}}
+// device-error@#24 {{HIP does not support OpenMP target directives}}
+
+void test01() {
+#pragma omp target // #01
+  ;
+}
+
+
+void test02() {
+#pragma omp target parallel // #02
+  ;
+}
+
+void test03() {
+#pragma omp target parallel for // #03
+  for (int i = 0; i < 1; ++i);
+}
+
+void test04(int x) {
+#pragma omp target data map(x) // #04
+  ;
+}
+
+void test05(int * x, int n) {
+#pragma omp target enter data map(to:x[:n]) // #05
+}
+
+void test06(int * x, int n) {
+#pragma omp target exit data map(from:x[:n]) // #06
+}
+
+void test07(int * x, int n) {
+#pragma omp target update to(x[:n]) // #07
+}
+
+#pragma omp declare target (test07) // #08
+void test08() {
+
+}
+
+#pragma omp begin declare target // #09
+void test09_1() {
+
+}
+
+void test09_2() {
+
+}
+#pragma omp end declare target
+
+void test10(int n) {
+  #pragma omp target parallel // #10
+  for (int i = 0; i < n; ++i)
+;
+}
+
+void test11(int n) {
+  #pragma omp target parallel for // #11
+  for (int i = 0; i < n; ++i)
+;
+}
+
+void test12(int n) {
+  #pragma omp target parallel for simd // #12
+  for (int i = 0; i < n; ++i)
+;
+}
+
+void test13(int n) {
+  #pragma omp target parallel loop // #13
+  for (int i = 0; i < n; ++i)
+;
+}
+
+void test14(int n) {
+  #pragma omp target simd // #14
+  for (int i = 0; i < n; ++i)
+;
+}
+
+void test15(int n) {
+  #pragma omp target teams // #15
+  for (int i = 0; i < n; ++i)
+;
+}
+
+void test16(int n) {
+  #pragma omp target teams distribute // #16
+  for (int i = 0; i < n; ++i)
+;
+}
+
+void test17(int n) {
+  #pragma omp target teams distribute simd // #17
+  for (int i = 0; i < n; ++i)
+;
+}
+
+void test18(int n) {
+  #pragma omp target teams loop // #18
+  for (int i = 0; i < n; ++i)
+;
+}
+
+void test19(int n) {
+  #pragma omp target teams distribute parallel for // #19
+  for (int i = 0; i < n; ++i)
+;
+}
+
+void test20(int n) {
+  #pragma omp target teams distribute parallel for simd // #20
+  for (int i = 0; i < n; ++i)
+;
+}
+
+void test21() {
+#pragma omp target // #21
+  {
+#pragma omp teams // #22
+{}
+  }
+}
+
+void test22() {
+#pragma omp target // #23
+#pragma omp teams // #24
+  {}
+}
+
+void test23() {
+// host code
+#pragma omp teams
+  {}
+}
Index: clang/lib/Sema/SemaOpenMP.cpp
===
--- clang/lib/Sema/SemaOpenMP.cpp
+++ clang/lib/Sema/SemaOpenMP.cpp
@@ -6119,6 +6119,11 @@
 BindKind, StartLoc))
   

[PATCH] D145591: [clang][HIP][OpenMP] Add warning if mixed HIP / OpenMP offloading

[Michael Halkenhäuser via Phabricator via cfe-commits]

mhalk created this revision.
Herald added subscribers: guansong, yaxunl.
Herald added a project: All.
mhalk requested review of this revision.
Herald added a reviewer: jdoerfert.
Herald added subscribers: cfe-commits, sstefan1.
Herald added a project: clang.

Adds a warning, issued by the clang semantic analysis, if HIP and OpenMP target 
offloading is requested concurrently.
That is, if HIP language mode is active but OpenMP target directives are 
encountered.
Previously, a user might not have been aware that target directives are ignored 
in such a case.

Generation of this warning is (lit-)tested via "make check-clang-semaopenmp".
The warning can be ignored via "-Wno-hip-omp-target-directives".


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D145591

Files:
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaOpenMP.cpp
  clang/test/SemaOpenMP/hip-omp-mix.cpp

Index: clang/test/SemaOpenMP/hip-omp-mix.cpp
===
--- /dev/null
+++ clang/test/SemaOpenMP/hip-omp-mix.cpp
@@ -0,0 +1,165 @@
+// REQUIRES: x86-registered-target
+// REQUIRES: amdgpu-registered-target
+
+// RUN: %clang_cc1 %s -x c++ -fopenmp -fsyntax-only -verify=host
+// host-no-diagnostics
+
+// RUN: %clang_cc1 %s -x hip -fopenmp -fsyntax-only -verify=device
+// device-warning@#01 {{HIP does not support OpenMP target directives}}
+// device-warning@#02 {{HIP does not support OpenMP target directives}}
+// device-warning@#03 {{HIP does not support OpenMP target directives}}
+// device-warning@#04 {{HIP does not support OpenMP target directives}}
+// device-warning@#05 {{HIP does not support OpenMP target directives}}
+// device-warning@#06 {{HIP does not support OpenMP target directives}}
+// device-warning@#07 {{HIP does not support OpenMP target directives}}
+// device-warning@#08 {{HIP does not support OpenMP target directives}}
+// device-warning@#09 {{HIP does not support OpenMP target directives}}
+// device-warning@#10 {{HIP does not support OpenMP target directives}}
+// device-warning@#11 {{HIP does not support OpenMP target directives}}
+// device-warning@#12 {{HIP does not support OpenMP target directives}}
+// device-warning@#13 {{HIP does not support OpenMP target directives}}
+// device-warning@#14 {{HIP does not support OpenMP target directives}}
+// device-warning@#15 {{HIP does not support OpenMP target directives}}
+// device-warning@#16 {{HIP does not support OpenMP target directives}}
+// device-warning@#17 {{HIP does not support OpenMP target directives}}
+// device-warning@#18 {{HIP does not support OpenMP target directives}}
+// device-warning@#19 {{HIP does not support OpenMP target directives}}
+// device-warning@#20 {{HIP does not support OpenMP target directives}}
+// device-warning@#21 {{HIP does not support OpenMP target directives}}
+// device-warning@#22 {{HIP does not support OpenMP target directives}}
+// device-warning@#23 {{HIP does not support OpenMP target directives}}
+// device-warning@#24 {{HIP does not support OpenMP target directives}}
+
+void test01() {
+#pragma omp target // #01
+  ;
+}
+
+
+void test02() {
+#pragma omp target parallel // #02
+  ;
+}
+
+void test03() {
+#pragma omp target parallel for // #03
+  for (int i = 0; i < 1; ++i);
+}
+
+void test04(int x) {
+#pragma omp target data map(x) // #04
+  ;
+}
+
+void test05(int * x, int n) {
+#pragma omp target enter data map(to:x[:n]) // #05
+}
+
+void test06(int * x, int n) {
+#pragma omp target exit data map(from:x[:n]) // #06
+}
+
+void test07(int * x, int n) {
+#pragma omp target update to(x[:n]) // #07
+}
+
+#pragma omp declare target (test07) // #08
+void test08() {
+
+}
+
+#pragma omp begin declare target // #09
+void test09_1() {
+
+}
+
+void test09_2() {
+
+}
+#pragma omp end declare target
+
+void test10(int n) {
+  #pragma omp target parallel // #10
+  for (int i = 0; i < n; ++i)
+;
+}
+
+void test11(int n) {
+  #pragma omp target parallel for // #11
+  for (int i = 0; i < n; ++i)
+;
+}
+
+void test12(int n) {
+  #pragma omp target parallel for simd // #12
+  for (int i = 0; i < n; ++i)
+;
+}
+
+void test13(int n) {
+  #pragma omp target parallel loop // #13
+  for (int i = 0; i < n; ++i)
+;
+}
+
+void test14(int n) {
+  #pragma omp target simd // #14
+  for (int i = 0; i < n; ++i)
+;
+}
+
+void test15(int n) {
+  #pragma omp target teams // #15
+  for (int i = 0; i < n; ++i)
+;
+}
+
+void test16(int n) {
+  #pragma omp target teams distribute // #16
+  for (int i = 0; i < n; ++i)
+;
+}
+
+void test17(int n) {
+  #pragma omp target teams distribute simd // #17
+  for (int i = 0; i < n; ++i)
+;
+}
+
+void test18(int n) {
+  #pragma omp target teams loop // #18
+  for (int i = 0; i < n; ++i)
+;
+}
+
+void test19(int n) {
+  #pragma omp target teams distribute parallel for // #19
+  for (int i = 0; i < n; ++i)
+;
+}
+
+void test20(int n) {
+  #pragma omp targ

[PATCH] D145591: [clang][HIP][OpenMP] Add warning if mixed HIP / OpenMP offloading

[Michael Halkenhäuser via Phabricator via cfe-commits]

mhalk added a comment.

In D145591#4179144 , @tra wrote:

> How is this different from compiling a C++ file with opemnp directives in it? 
> AFAICT neither clang nor gcc issue anywarnings: 
> https://godbolt.org/z/5Me3dnTdr
>
> What makes the warnings necessary for HIP?

When using HIP target offloading it will "take precedence" over OpenMP target 
offloading.
That is, there is a use case which compiles "successfully" / without warning 
but will not execute code within OpenMP target directives.

For example: when using `-x hip -fopenmp --offload-arch=...` (+ other 
reasonable parameters) a HIP program with OpenMP target directives will compile 
without warning / error.
But only host-related OpenMP code will be executed properly.

Another hint that HIP and OpenMP target offloading do not work concurrently ATM 
is that when compiling with `-x hip --fopenmp-targets=...` clang will throw an 
error.

Hence, we want to signal this issue to users, whorun into problems and then 
have to debug their programs to find out about this.
AFAIK this issue can be alleviated by refactoring / splitting up your code, 
then linking it back togehter -- but you have to know about this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145591

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

[PATCH] D145591: [clang][HIP][OpenMP] Add warning if mixed HIP / OpenMP offloading

[Michael Halkenhäuser via Phabricator via cfe-commits]

mhalk added inline comments.
Herald added a subscriber: sunshaoce.



Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:8634
+def warn_hip_omp_target_directives : Warning<
+  "HIP does not support OpenMP target directives; directive has been ignored">,
+  InGroup;

jdoerfert wrote:
> I doubt the ignored part very much.
I just re-checked the LLVM IR of an example.
With -x c++ there are __kmpc_target_(de)init calls; with -x hip there are none.
https://godbolt.org/z/fhds46Pvc

From my point of view, "effectively", target directives seem to have been 
ignored.
Maybe this is a coincidence (there's definitely some uncertainty) or I am 
misinterpreting the IR.
I would be glad if you could shed some light on this, if possible / you find 
the time to do so.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145591

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