ssquare08 added inline comments.
================
Comment at: clang/test/OpenMP/declare_target_only_one_side_compilation.cpp:61
// DEVICE-DAG: @G1 = {{.*}}global i32 0, align 4
-// DEVICE-DAG: @_ZL2G2 = internal {{.*}}global i32 0, align 4
+// DEVICE-DAG: @_ZL2G2 = {{.*}}global i32 0, align 4
// DEVICE-DAG: @G3 = {{.*}}global i32 0, align 4
----------------
I wasn't expecting this to change. For some reason G2 gets the
`OMPDeclareTargetDeclAttr::DT_Any` attribute instead of
`OMPDeclareTargetDeclAttr::DT_NoHost` and because of that the visibility
changes. @jdoerfert, is `OMPDeclareTargetDeclAttr::DT_Any` attribute expected
here?
================
Comment at: clang/test/OpenMP/declare_target_visibility_codegen.cpp:11-12
// HOST: @z = global i32 0
-// HOST-NOT: @.omp_offloading.entry.c
-// HOST-NOT: @.omp_offloading.entry.x
+// HOST: @.omp_offloading.entry.c__static__{{[0-9a-z]+_[0-9a-z]+}}
+// HOST: @.omp_offloading.entry.x__static__{{[0-9a-z]+_[0-9a-z]+}}
// HOST-NOT: @.omp_offloading.entry.y
----------------
ssquare08 wrote:
> jhuber6 wrote:
> > If there are no updates between the host and device we can keep these
> > static without emitting an offloading entry.
> That 's a good point. I'll fix that.
I thought about this more and I think the behavior for these declare target
static globals should be the same as the other declare target. Checking for
update is not enough because users could also map these variables. For update,
it could be mapped with a pointer or the users could pass address of these
variables to an external function. Please let me know what you think of these
cases below:
```
#pragma omp declare target
static int x[10];
#pragma omp end declare target
//case 1
#pragma omp target update to(x)
//case 2
int* y = &x[2];
#pragma omp target update to(y[0])
//case 3
#pragma omp target map(always to:x)
{
x[0]= 111;
}
//case 4
#pragma omp target
{
foo(&x[3]);
}
```
================
Comment at: clang/test/OpenMP/target_update_messages.cpp:17
-static int y;
-#pragma omp declare target(y)
-
-void yyy() {
-#pragma omp target update to(y) // expected-error {{the host cannot update a
declare target variable that is not externally visible.}}
-}
-
int __attribute__((visibility("hidden"))) z;
#pragma omp declare target(z)
----------------
jhuber6 wrote:
> jdoerfert wrote:
> > There is no test to show you can actually write the update now, is there?
> We should probably take the deleted code above and put it in an OpenMP
> runtime test to make sure it actually works now.
I have now added a test as suggested.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D129694/new/
https://reviews.llvm.org/D129694
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
