Hello Kwok,

On 01.03.21 23:28, Kwok Cheung Yeung wrote:
When two or more nested loops are collapsed using the OpenMP collapse
clause, a single iteration variable is used to represent the combined
iteration space. In the usual case (i.e. statically scheduled, no
ordered clause), the type of this variable is picked by taking the
unsigned version of the largest of the iterator types in the loop nest
[...] However, this can be problematic if the combined iteration space
of the collapsed loops is larger than can be represented by the type
of the largest individual loop iterator type.

Unfortunately, we are deeply in the implementation-defined realm.
I have now filled an OpenMP spec issue to make it possible to
avoid/mitigate those issues as user: OpenMP Issue 2693.
Namely, a way to specify the used integer type/precision
and/or refine what the user can expect.

In any case, the following applies currently:

"The iterations of some number of associated loops can be
 collapsed into one larger iteration space that is called
 the logical iteration space. The particular integer type
 used to compute the iteration count for the collapsed
 loop is implementation defined."
(2.11.1 Canonical Loop Nest Form)

Hence, both is sensible – the current behavior of GCC
and the one you have proposed and implemented:

This patch attempts to avoid this problem by setting the precision of
the combined iteration variable to the sum of the precision of the
collapsed iterators, rounded up to the nearest power of 2. This is
capped at the size of a long long (i.e. 64 bits) to avoid an excessive
performance hit. If any of the loops use a larger type (e.g.
__int128), then that is used instead.
* * *
I believe OpenACC suffers from a similar problem, but it uses a
different code-path and should be dealt with separately.
Regarding OpenACC, I think diff_type → expand_oacc_collapse_init,
expand_oacc_collapse_vars would be the equivalent. In the spec, either a
note similar to OpenMP is missing or I have just not found it.
Okay for OG10?

I think that's fine for the OG10 branch for now. Another question is how
to handle it best on mainline, which is then also Jakub's call. And also
how the OpenMP issue will be resolved.

Can you add a comment to the two testcases that the internal variable,
used to calculate the iteration count, needs a higher precision than
(32bit) 'int'? Otherwise it is not obvious.

Tobias
+         {
+           int loop_precision = TYPE_PRECISION (TREE_TYPE (loop->v));
+           int iter_type_precision = 0;
+           const int max_accum_precision
+             = TYPE_PRECISION (long_long_unsigned_type_node);
+
+           accum_iter_precision += loop_precision;
+
+           if (i == 0
+               || (loop_precision >= max_accum_precision
+                   && loop_precision >= TYPE_PRECISION (iter_type)))
+             iter_type_precision = loop_precision;
+           else if (TYPE_PRECISION (iter_type) < max_accum_precision)
+             iter_type_precision
+               = MIN (1 << ceil_log2 (accum_iter_precision),
+                      max_accum_precision);
+
+           if (iter_type_precision)
+             iter_type = build_nonstandard_integer_type
+                           (iter_type_precision, 1);
+         }
      }
        else if (iter_type != long_long_unsigned_type_node)
      {
diff --git a/libgomp/ChangeLog.omp b/libgomp/ChangeLog.omp
index d1dcf20..0e3fd12 100644
--- a/libgomp/ChangeLog.omp
+++ b/libgomp/ChangeLog.omp
@@ -1,3 +1,8 @@
+2021-03-01  Kwok Cheung Yeung<k...@codesourcery.com>
+
+     * testsuite/libgomp.c-c++-common/collapse-4.c: New.
+     * testsuite/libgomp.fortran/collapse5.f90: New.
+
  2021-02-24  Julian Brown<jul...@codesourcery.com>

      Backport from mainline
diff --git a/libgomp/testsuite/libgomp.c-c++-common/collapse-4.c 
b/libgomp/testsuite/libgomp.c-c++-common/collapse-4.c
new file mode 100644
index 0000000..04cf747
--- /dev/null
+++ b/libgomp/testsuite/libgomp.c-c++-common/collapse-4.c
@@ -0,0 +1,19 @@
+/* { dg-do run } */
+
+#include <stdlib.h>
+
+int
+main (void)
+{
+  int i, j;
+  int count = 0;
+
+  #pragma omp parallel for collapse(2)
+    for (i = 0; i < 80000; i++)
+      for (j = 0; j < 80000; j++)
+     if (i == 66666 && j == 77777)
+       count++;
+
+  if (count != 1)
+    abort ();
+}
diff --git a/libgomp/testsuite/libgomp.fortran/collapse5.f90 
b/libgomp/testsuite/libgomp.fortran/collapse5.f90
new file mode 100644
index 0000000..c76424d
--- /dev/null
+++ b/libgomp/testsuite/libgomp.fortran/collapse5.f90
@@ -0,0 +1,14 @@
+! { dg-do run }
+
+  integer :: i, j
+  integer :: count = 0
+
+  !$omp parallel do collapse (2)
+    do i = 0, 80000
+      do j = 0, 80000
+        if (i .eq. 66666 .and. j .eq. 77777) count = count + 1
+      end do
+    end do
+
+  if (count .ne. 1) stop 1
+end
-- 2.8.1
-----------------
Mentor Graphics (Deutschland) GmbH, Arnulfstrasse 201, 80634 München 
Registergericht München HRB 106955, Geschäftsführer: Thomas Heurung, Frank 
Thürauf

Reply via email to