[Openmp-commits] [PATCH] D34701: [openmp-target-tests] OpenMP 4.5 Target data test cases

[Jonas Hahnfeld via Phabricator via Openmp-commits]

Hahnfeld added a comment.

1. Is there as specific reason for using both `puts` and `printf`? There are also calls to `printf` without formatting arguments...
2. Do these tests actually pass with a current version of clang?



================
Comment at: libomptarget/test/offloading/target_data/test_target_data_if.c:38-40
+        if (c[i] != 0) {
+          errors = 1;
+        }
----------------
Again thinking about this, the check may be wrong: The OpenMP standard says that the `target data` and `target` constructs execute on the host, if the `if` clause evaluates to false. This check is assuming that it doesn't execute at all, right?


================
Comment at: libomptarget/test/offloading/target_data/test_target_data_map_array_sections.c:428
+
+void init_2d(int a[N][2]) {
+  for (int i = 0; i < N; ++i) {
----------------
Please change this signature as well.


================
Comment at: libomptarget/test/offloading/target_data/test_target_data_map_array_sections.c:435
+
+void init_3d(int a[N][2][2]) {
+  for (int i = 0; i < N; ++i)
----------------
Same


================
Comment at: libomptarget/test/offloading/target_data/test_target_data_map_devices.c:130
+        else if (dev >= real_num_dev) {
+          // allows implementations that map extra devices to the host
+          extra_on_host = 1;
----------------
sergiop wrote:
> Hahnfeld wrote:
> > Does this really work with libomptarget? If so, is this mandated by the standard and do we want to test this?
> In the spec there is not restriction about assigning computation/data movement to a device that has an id higher than the number of actual devices. 
"The device expression must evaluate to a non-negative integer value less than the value of omp_get_num_devices()."
OpenMP 4.5, p. 97


https://reviews.llvm.org/D34701