Re: [PATCH] Add map clauses to libgomp test device-3.f90
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
Re: [PATCH] Add map clauses to libgomp test device-3.f90
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
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
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 > Alexander Monakov > > * 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
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 Alexander Monakov * 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
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 ()
[PATCH] Add map clauses to libgomp test device-3.f90
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 * 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
