[PATCH] Add map clauses to libgomp test device-3.f90

[PATCH] Add map clauses to libgomp test device-3.f90

[Martin Jambor]

Hi,

yesterday I forgot to send out the following patch.  The test
libgomp/testsuite/libgomp.fortran/examples-4/device-3.f90 was failing
for me when I was testing the HSA branch merge but I believe the test
itself is wrong and the failure is due to us now adhering to OpenMP
4.5 default mapping of scalars (i.e. firstprivate, as opposed to
tofrom in 4.0) and the test itself needs to be fixed in the following
way.

OK for trunk?  Thanks,

Martin


2016-11-11  Martin Jambor  <mjambor@suse.cz>

	* device-3.f90 (e_57_3): Add a mapping clause to target construct.

diff --git a/libgomp/testsuite/libgomp.fortran/examples-4/device-3.f90 b/libgomp/testsuite/libgomp.fortran/examples-4/device-3.f90
index a29f1b5..95d9f44 100644
--- a/libgomp/testsuite/libgomp.fortran/examples-4/device-3.f90
+++ b/libgomp/testsuite/libgomp.fortran/examples-4/device-3.f90
@@ -8,13 +8,13 @@ program e_57_3
   integer :: default_device
 
   default_device = omp_get_default_device ()
-  !$omp target
+  !$omp target map(from:res)
     res = omp_is_initial_device ()
   !$omp end target
   if (res) call abort
 
   call omp_set_default_device (omp_get_num_devices ())
-  !$omp target
+  !$omp target map(from:res)
     res = omp_is_initial_device ()
   !$omp end target
   if (.not. res) call abort

Re: [PATCH] Add map clauses to libgomp test device-3.f90

[Alexander Monakov]

On Mon, 14 Nov 2016, Martin Jambor wrote:

> Hi,
> 
> yesterday I forgot to send out the following patch.  The test
> libgomp/testsuite/libgomp.fortran/examples-4/device-3.f90 was failing
> for me when I was testing the HSA branch merge but I believe the test
> itself is wrong and the failure is due to us now adhering to OpenMP
> 4.5 default mapping of scalars (i.e. firstprivate, as opposed to
> tofrom in 4.0) and the test itself needs to be fixed in the following
> way.

From inspection, I believe device-1.f90 in the same directory has the same
issue?  Here's the corresponding patch.

I also think there's the same issue in target-3.f90 and target5.f90
(compile-only test).

diff --git a/libgomp/testsuite/libgomp.fortran/examples-4/device-1.f90 b/libgomp/testsuite/libgomp.fortran/examples-4/device-1.f90
index a411db4..3404f01 100644
--- a/libgomp/testsuite/libgomp.fortran/examples-4/device-1.f90
+++ b/libgomp/testsuite/libgomp.fortran/examples-4/device-1.f90
@@ -9,11 +9,11 @@ program e_57_1
   a = 100
   b = 0

-  !$omp target if(a > 200 .and. a < 400)
+  !$omp target map(from: c) if(a > 200 .and. a < 400)
     c = omp_is_initial_device ()
   !$omp end target

-  !$omp target data map(to: b) if(a > 200 .and. a < 400)
+  !$omp target data map(to: b) map(from: d) if(a > 200 .and. a < 400)
     !$omp target
       b = 100
       d = omp_is_initial_device ()
@@ -25,11 +25,11 @@ program e_57_1
   a = a + 200
   b = 0

-  !$omp target if(a > 200 .and. a < 400)
+  !$omp target map(from: c) if(a > 200 .and. a < 400)
     c = omp_is_initial_device ()
   !$omp end target

-  !$omp target data map(to: b) if(a > 200 .and. a < 400)
+  !$omp target data map(to: b) map(from: d) if(a > 200 .and. a < 400)
     !$omp target
       b = 100
       d = omp_is_initial_device ()
@@ -41,11 +41,11 @@ program e_57_1
   a = a + 200
   b = 0

-  !$omp target if(a > 200 .and. a < 400)
+  !$omp target map(from: c) if(a > 200 .and. a < 400)
     c = omp_is_initial_device ()
   !$omp end target

-  !$omp target data map(to: b) if(a > 200 .and. a < 400)
+  !$omp target data map(to: b) map(from: d) if(a > 200 .and. a < 400)
     !$omp target
       b = 100
       d = omp_is_initial_device ()

Re: [PATCH] Add map clauses to libgomp test device-3.f90

[Alexander Monakov]

On Mon, 14 Nov 2016, Alexander Monakov wrote:
> On Mon, 14 Nov 2016, Martin Jambor wrote:
> 
> > Hi,
> > 
> > yesterday I forgot to send out the following patch.  The test
> > libgomp/testsuite/libgomp.fortran/examples-4/device-3.f90 was failing
> > for me when I was testing the HSA branch merge but I believe the test
> > itself is wrong and the failure is due to us now adhering to OpenMP
> > 4.5 default mapping of scalars (i.e. firstprivate, as opposed to
> > tofrom in 4.0) and the test itself needs to be fixed in the following
> > way.
> 
> From inspection, I believe device-1.f90 in the same directory has the same
> issue?

Yep, I do see new test execution failures with both Intel MIC and PTX offloading
on device-1.f90, device-3.f90 and target2.f90.  Here's an actually-tested patch
for the first two (on target2.f90 there's a different problem).

	Martin Jambor  <mjambor@suse.cz>
	Alexander Monakov  <amonakov@ispras.ru>

	* testsuite/libgomp.fortran/examples-4/device-1.f90 (e_57_1): Add
	mapping clauses to target constructs.
	* testsuite/libgomp.fortran/examples-4/device-3.f90 (e_57_3): Ditto.

diff --git a/libgomp/testsuite/libgomp.fortran/examples-4/device-1.f90 b/libgomp/testsuite/libgomp.fortran/examples-4/device-1.f90
index a411db4..30148f1 100644
--- a/libgomp/testsuite/libgomp.fortran/examples-4/device-1.f90
+++ b/libgomp/testsuite/libgomp.fortran/examples-4/device-1.f90
@@ -9,12 +9,12 @@ program e_57_1
   a = 100
   b = 0
 
-  !$omp target if(a > 200 .and. a < 400)
+  !$omp target map(from: c) if(a > 200 .and. a < 400)
     c = omp_is_initial_device ()
   !$omp end target
 
   !$omp target data map(to: b) if(a > 200 .and. a < 400)
-    !$omp target
+    !$omp target map(from: b, d)
       b = 100
       d = omp_is_initial_device ()
     !$omp end target
@@ -25,12 +25,12 @@ program e_57_1
   a = a + 200
   b = 0
 
-  !$omp target if(a > 200 .and. a < 400)
+  !$omp target map(from: c) if(a > 200 .and. a < 400)
     c = omp_is_initial_device ()
   !$omp end target
 
   !$omp target data map(to: b) if(a > 200 .and. a < 400)
-    !$omp target
+    !$omp target map(from: b, d)
       b = 100
       d = omp_is_initial_device ()
     !$omp end target
@@ -41,12 +41,12 @@ program e_57_1
   a = a + 200
   b = 0
 
-  !$omp target if(a > 200 .and. a < 400)
+  !$omp target map(from: c) if(a > 200 .and. a < 400)
     c = omp_is_initial_device ()
   !$omp end target
 
   !$omp target data map(to: b) if(a > 200 .and. a < 400)
-    !$omp target
+    !$omp target map(from: b, d)
       b = 100
       d = omp_is_initial_device ()
     !$omp end target
diff --git a/libgomp/testsuite/libgomp.fortran/examples-4/device-3.f90 b/libgomp/testsuite/libgomp.fortran/examples-4/device-3.f90
index a29f1b5..d770b91 100644
--- a/libgomp/testsuite/libgomp.fortran/examples-4/device-3.f90
+++ b/libgomp/testsuite/libgomp.fortran/examples-4/device-3.f90
@@ -8,13 +8,13 @@ program e_57_3
   integer :: default_device
 
   default_device = omp_get_default_device ()
-  !$omp target
+  !$omp target map(from: res)
     res = omp_is_initial_device ()
   !$omp end target
   if (res) call abort
 
   call omp_set_default_device (omp_get_num_devices ())
-  !$omp target
+  !$omp target map(from: res)
     res = omp_is_initial_device ()
   !$omp end target
   if (.not. res) call abort

Re: [PATCH] Add map clauses to libgomp test device-3.f90

[Jakub Jelinek]

On Tue, Nov 15, 2016 at 07:52:56PM +0300, Alexander Monakov wrote:
> On Mon, 14 Nov 2016, Alexander Monakov wrote:
> > On Mon, 14 Nov 2016, Martin Jambor wrote:
> > 
> > > Hi,
> > > 
> > > yesterday I forgot to send out the following patch.  The test
> > > libgomp/testsuite/libgomp.fortran/examples-4/device-3.f90 was failing
> > > for me when I was testing the HSA branch merge but I believe the test
> > > itself is wrong and the failure is due to us now adhering to OpenMP
> > > 4.5 default mapping of scalars (i.e. firstprivate, as opposed to
> > > tofrom in 4.0) and the test itself needs to be fixed in the following
> > > way.
> > 
> > From inspection, I believe device-1.f90 in the same directory has the same
> > issue?
> 
> Yep, I do see new test execution failures with both Intel MIC and PTX offloading
> on device-1.f90, device-3.f90 and target2.f90.  Here's an actually-tested patch
> for the first two (on target2.f90 there's a different problem).
> 
> 	Martin Jambor  <mjambor@suse.cz>
> 	Alexander Monakov  <amonakov@ispras.ru>
> 
> 	* testsuite/libgomp.fortran/examples-4/device-1.f90 (e_57_1): Add
> 	mapping clauses to target constructs.
> 	* testsuite/libgomp.fortran/examples-4/device-3.f90 (e_57_3): Ditto.

Ok, thanks.

	Jakub

Re: [PATCH] Add map clauses to libgomp test device-3.f90

[Alexander Monakov]

On Tue, 15 Nov 2016, Alexander Monakov wrote:
> Yep, I do see new test execution failures with both Intel MIC and PTX offloading
> on device-1.f90, device-3.f90 and target2.f90.  Here's an actually-tested patch
> for the first two (on target2.f90 there's a different problem).

And here's a patch for target2.f90.  I don't have a perfect understanding of
mapping clauses, but the test appears to need to explicitly map pointer
variables, at a minimum.  Also, 'map (from: r)' is missing on the last target
region.

	* testsuite/libgomp.fortran/target2.f90 (foo): Add mapping clauses to
	target construct.

diff --git a/libgomp/testsuite/libgomp.fortran/target2.f90 b/libgomp/testsuite/libgomp.fortran/target2.f90
index 42f704f..7119774 100644
--- a/libgomp/testsuite/libgomp.fortran/target2.f90
+++ b/libgomp/testsuite/libgomp.fortran/target2.f90
@@ -63,7 +63,7 @@ contains
       r = r .or. (any (k(5:n-5) /= 17)) .or. (lbound (k, 1) /= 4) .or. (ubound (k, 1) /= n)
     !$omp end target
     if (r) call abort
-    !$omp target map (to: d(2:n+1), n)
+    !$omp target map (to: d(2:n+1), f, j) map (from: r)
       r = a /= 7
       r = r .or. (any (b /= 8)) .or. (lbound (b, 1) /= 3) .or. (ubound (b, 1) /= n)
       r = r .or. (any (c /= 9)) .or. (lbound (c, 1) /= 5) .or. (ubound (c, 1) /= n + 4)

Re: [PATCH] Add map clauses to libgomp test device-3.f90

[Alexander Monakov]

On Tue, 15 Nov 2016, Alexander Monakov wrote:
> On Tue, 15 Nov 2016, Alexander Monakov wrote:
> > Yep, I do see new test execution failures with both Intel MIC and PTX offloading
> > on device-1.f90, device-3.f90 and target2.f90.  Here's an actually-tested patch
> > for the first two (on target2.f90 there's a different problem).
> 
> And here's a patch for target2.f90.  I don't have a perfect understanding of
> mapping clauses, but the test appears to need to explicitly map pointer
> variables, at a minimum.  Also, 'map (from: r)' is missing on the last target
> region.
> 
> 	* testsuite/libgomp.fortran/target2.f90 (foo): Add mapping clauses to
> 	target construct.

Ping.

> diff --git a/libgomp/testsuite/libgomp.fortran/target2.f90 b/libgomp/testsuite/libgomp.fortran/target2.f90
> index 42f704f..7119774 100644
> --- a/libgomp/testsuite/libgomp.fortran/target2.f90
> +++ b/libgomp/testsuite/libgomp.fortran/target2.f90
> @@ -63,7 +63,7 @@ contains
>        r = r .or. (any (k(5:n-5) /= 17)) .or. (lbound (k, 1) /= 4) .or. (ubound (k, 1) /= n)
>      !$omp end target
>      if (r) call abort
> -    !$omp target map (to: d(2:n+1), n)
> +    !$omp target map (to: d(2:n+1), f, j) map (from: r)
>        r = a /= 7
>        r = r .or. (any (b /= 8)) .or. (lbound (b, 1) /= 3) .or. (ubound (b, 1) /= n)
>        r = r .or. (any (c /= 9)) .or. (lbound (c, 1) /= 5) .or. (ubound (c, 1) /= n + 4)
> 
> 

Re: [PATCH] Add map clauses to libgomp test device-3.f90

[Jakub Jelinek]

On Tue, Nov 15, 2016 at 08:01:32PM +0300, Alexander Monakov wrote:
> On Tue, 15 Nov 2016, Alexander Monakov wrote:
> > Yep, I do see new test execution failures with both Intel MIC and PTX offloading
> > on device-1.f90, device-3.f90 and target2.f90.  Here's an actually-tested patch
> > for the first two (on target2.f90 there's a different problem).
> 
> And here's a patch for target2.f90.  I don't have a perfect understanding of
> mapping clauses, but the test appears to need to explicitly map pointer
> variables, at a minimum.  Also, 'map (from: r)' is missing on the last target
> region.
> 
> 	* testsuite/libgomp.fortran/target2.f90 (foo): Add mapping clauses to
> 	target construct.
> 
> diff --git a/libgomp/testsuite/libgomp.fortran/target2.f90 b/libgomp/testsuite/libgomp.fortran/target2.f90
> index 42f704f..7119774 100644
> --- a/libgomp/testsuite/libgomp.fortran/target2.f90
> +++ b/libgomp/testsuite/libgomp.fortran/target2.f90
> @@ -63,7 +63,7 @@ contains
>        r = r .or. (any (k(5:n-5) /= 17)) .or. (lbound (k, 1) /= 4) .or. (ubound (k, 1) /= n)
>      !$omp end target
>      if (r) call abort
> -    !$omp target map (to: d(2:n+1), n)
> +    !$omp target map (to: d(2:n+1), f, j) map (from: r)

The missing map (from: r) is obvious.  Explicitly mentioned f and j is
tested in another part of the test, here I believe it should be implicitly
firstprivate, and then
"If the original list item has the POINTER attribute, the new list items
receive the same association status of the original list item as if by
pointer assignment."
shall apply.  I think I'd prefer the test to be broken over forgetting this
isn't handled correctly.

	Jakub