diff mbox series

[committed] libgomp.fortran/allocate-1.f90: Minor cleanup (was: Re: [PATCH] [gfortran] Add support for allocate clause (OpenMP 5.0).)

Message ID 195b52d9-0071-1801-dfd0-041d794e93a4@codesourcery.com
State New
Headers show
Series [committed] libgomp.fortran/allocate-1.f90: Minor cleanup (was: Re: [PATCH] [gfortran] Add support for allocate clause (OpenMP 5.0).) | expand

Commit Message

Tobias Burnus Feb. 4, 2022, 1:57 p.m. UTC
Hi Thomas,

On 04.02.22 10:37, Thomas Schwinge wrote:
>> I have attached a patch (not commited), which silences the three kind of
>> warnings and fixes the interface issue.
>> TODO: commit it.
> Still "TODO: commit it" ;-) -- and while I haven't reviewed the changes
> in detail, I did spot one item that should be addressed, I suppose:

I had also spotted the 'stop' which was a left over from -fsanitized=...
checking and had removed it locally. But good that you also keep
checking patches :-)

In any case, I have now _finally_ committed the patch.

Attached is the simplified (-w) diff, where I did exclude the
indentation changes to make the diff more readable.

For the full diff, see e.g. https://gcc.gnu.org/r12-7053

Tobias
-----------------
Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634 München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht München, HRB 106955

Comments

Thomas Schwinge Feb. 4, 2022, 3:33 p.m. UTC | #1
Hi Tobias!

On 2022-02-04T14:57:07+0100, Tobias Burnus <tobias@codesourcery.com> wrote:
> On 04.02.22 10:37, Thomas Schwinge wrote:
>>> I have attached a patch (not commited), which silences the three kind of
>>> warnings and fixes the interface issue.
>>> TODO: commit it.
>> Still "TODO: commit it" ;-) -- and while I haven't reviewed the changes
>> in detail, I did spot one item that should be addressed, I suppose:
>
> I had also spotted the 'stop' which was a left over from -fsanitized=...
> checking and had removed it locally.

Maybe removed locally, I can't tell ;-) -- but it's still in the commit
that you pushed.  See below.

Also, a commented-out '!$omp barrier'; not sure what that one is about.

> But good that you also keep
> checking patches :-)

I try!  :-)


Grüße
 Thomas


> In any case, I have now _finally_ committed the patch.
>
> Attached is the simplified (-w) diff, where I did exclude the
> indentation changes to make the diff more readable.
>
> For the full diff, see e.g. https://gcc.gnu.org/r12-7053
>
> Tobias

> commit 6d4981350168f1eb3f72149bd7e05b9ba6bec1fd
> Author: Tobias Burnus <tobias@codesourcery.com>
> Date:   Fri Feb 4 14:51:01 2022 +0100
>
>     libgomp.fortran/allocate-1.f90: Minor cleanup
>
>     libgomp/ChangeLog:
>             * testsuite/libgomp.fortran/allocate-1.c (is_64bit_aligned): Renamed
>             from is_64bit_aligned_.
>             * testsuite/libgomp.fortran/allocate-1.f90: Fix interface decl
>             and use it, more implicit none, remove unused argument.
>
> diff --git a/libgomp/testsuite/libgomp.fortran/allocate-1.c b/libgomp/testsuite/libgomp.fortran/allocate-1.c
> index d33acc6feef..cb6d355afc6 100644
> --- a/libgomp/testsuite/libgomp.fortran/allocate-1.c
> +++ b/libgomp/testsuite/libgomp.fortran/allocate-1.c
> @@ -1,7 +1,7 @@
>  #include <stdint.h>
>
>  int
> -is_64bit_aligned_ (uintptr_t a)
> +is_64bit_aligned (uintptr_t a)
>  {
>    return ( (a & 0x3f) == 0);
>  }
> diff --git a/libgomp/testsuite/libgomp.fortran/allocate-1.f90 b/libgomp/testsuite/libgomp.fortran/allocate-1.f90
> index 35d1750b878..062278f9908 100644
> --- a/libgomp/testsuite/libgomp.fortran/allocate-1.f90
> +++ b/libgomp/testsuite/libgomp.fortran/allocate-1.f90
> @@ -5,30 +5,30 @@
>  module m
>    use omp_lib
>    use iso_c_binding
> -  implicit none
> +  implicit none (type, external)
>
>    interface
>      integer(c_int) function is_64bit_aligned (a) bind(C)
>        import :: c_int
> -      integer  :: a
> +      type(*)  :: a
>      end
>    end interface
> -end module m
>
> -subroutine foo (x, p, q, px, h, fl)
> +contains
> +
> +subroutine foo (x, p, q, h, fl)
>    use omp_lib
>    use iso_c_binding
>    integer  :: x
>    integer, dimension(4) :: p
>    integer, dimension(4) :: q
> -  integer  :: px
>    integer (kind=omp_allocator_handle_kind) :: h
>    integer  :: fl
>
>    integer  :: y
>    integer  :: r, i, i1, i2, i3, i4, i5
>    integer  :: l, l3, l4, l5, l6
> -  integer  :: n, n1, n2, n3, n4
> +  integer  :: n, n2, n3, n4
>    integer  :: j2, j3, j4
>    integer, dimension(4) :: l2
>    integer, dimension(4) :: r2
> @@ -74,6 +74,8 @@ subroutine foo (x, p, q, px, h, fl)
>    if (x /= 42) then
>      stop 1
>    end if
> +
> +  !!$omp barrier
>    v(1) = 7
>    if ( (and(fl, 2) /= 0) .and.          &
>         ((is_64bit_aligned(x) == 0) .or. &
> @@ -95,7 +97,7 @@ subroutine foo (x, p, q, px, h, fl)
>      stop 4
>    end if
>    !$omp end parallel
> -
> +stop
>    !$omp teams
>    !$omp parallel private (y) firstprivate (x, w) allocate (h: x, y, w)
>
> @@ -305,11 +307,13 @@ subroutine foo (x, p, q, px, h, fl)
>        .or. r2(1) /= (5 * p(3)) .or. r2(4) /= (6 * p(3))) then
>      stop 25
>    end if
> -
>  end subroutine
> +end module m
>
>  program main
>    use omp_lib
> +  use m
> +  implicit none (type, external)
>    integer, dimension(4) :: p
>    integer, dimension(4) :: q
>
> @@ -323,11 +327,11 @@ program main
>    if (a == omp_null_allocator) stop 1
>
>    call omp_set_default_allocator (omp_default_mem_alloc);
> -  call foo (42, p, q, 2, a, 0);
> -  call foo (42, p, q, 2, omp_default_mem_alloc, 0);
> -  call foo (42, p, q, 2, a, 1);
> +  call foo (42, p, q, a, 0);
> +  call foo (42, p, q, omp_default_mem_alloc, 0);
> +  call foo (42, p, q, a, 1);
>    call omp_set_default_allocator (a);
> -  call foo (42, p, q, 2, omp_null_allocator, 3);
> -  call foo (42, p, q, 2, omp_default_mem_alloc, 2);
> +  call foo (42, p, q, omp_null_allocator, 3);
> +  call foo (42, p, q, omp_default_mem_alloc, 2);
>    call omp_destroy_allocator (a);
>  end
-----------------
Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634 München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht München, HRB 106955
Tobias Burnus Feb. 4, 2022, 4:34 p.m. UTC | #2
On 04.02.22 16:33, Thomas Schwinge wrote:
> Maybe removed locally, I can't tell ;-) -- but it's still in the
> commit that you pushed. See below.
> Also, a commented-out '!$omp barrier'; not sure what that one is about.

I shall not do commits after one week of 6h+/day virtual OpenMP
Face2Face meeting.

Corrected with commit as shown in the attachment.

Tobias
-----------------
Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634 München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht München, HRB 106955
diff mbox series

Patch

commit 6d4981350168f1eb3f72149bd7e05b9ba6bec1fd
Author: Tobias Burnus <tobias@codesourcery.com>
Date:   Fri Feb 4 14:51:01 2022 +0100

    libgomp.fortran/allocate-1.f90: Minor cleanup
    
    libgomp/ChangeLog:
            * testsuite/libgomp.fortran/allocate-1.c (is_64bit_aligned): Renamed
            from is_64bit_aligned_.
            * testsuite/libgomp.fortran/allocate-1.f90: Fix interface decl
            and use it, more implicit none, remove unused argument.

diff --git a/libgomp/testsuite/libgomp.fortran/allocate-1.c b/libgomp/testsuite/libgomp.fortran/allocate-1.c
index d33acc6feef..cb6d355afc6 100644
--- a/libgomp/testsuite/libgomp.fortran/allocate-1.c
+++ b/libgomp/testsuite/libgomp.fortran/allocate-1.c
@@ -1,7 +1,7 @@ 
 #include <stdint.h>
 
 int
-is_64bit_aligned_ (uintptr_t a)
+is_64bit_aligned (uintptr_t a)
 {
   return ( (a & 0x3f) == 0);
 }
diff --git a/libgomp/testsuite/libgomp.fortran/allocate-1.f90 b/libgomp/testsuite/libgomp.fortran/allocate-1.f90
index 35d1750b878..062278f9908 100644
--- a/libgomp/testsuite/libgomp.fortran/allocate-1.f90
+++ b/libgomp/testsuite/libgomp.fortran/allocate-1.f90
@@ -5,30 +5,30 @@ 
 module m
   use omp_lib
   use iso_c_binding
-  implicit none
+  implicit none (type, external)
 
   interface
     integer(c_int) function is_64bit_aligned (a) bind(C)
       import :: c_int
-      integer  :: a
+      type(*)  :: a
     end
   end interface
-end module m
 
-subroutine foo (x, p, q, px, h, fl)
+contains
+
+subroutine foo (x, p, q, h, fl)
   use omp_lib
   use iso_c_binding
   integer  :: x
   integer, dimension(4) :: p
   integer, dimension(4) :: q
-  integer  :: px
   integer (kind=omp_allocator_handle_kind) :: h
   integer  :: fl
 
   integer  :: y
   integer  :: r, i, i1, i2, i3, i4, i5
   integer  :: l, l3, l4, l5, l6
-  integer  :: n, n1, n2, n3, n4
+  integer  :: n, n2, n3, n4
   integer  :: j2, j3, j4
   integer, dimension(4) :: l2
   integer, dimension(4) :: r2
@@ -74,6 +74,8 @@  subroutine foo (x, p, q, px, h, fl)
   if (x /= 42) then
     stop 1
   end if
+
+  !!$omp barrier
   v(1) = 7
   if ( (and(fl, 2) /= 0) .and.          &
        ((is_64bit_aligned(x) == 0) .or. &
@@ -95,7 +97,7 @@  subroutine foo (x, p, q, px, h, fl)
     stop 4
   end if
   !$omp end parallel
-
+stop
   !$omp teams
   !$omp parallel private (y) firstprivate (x, w) allocate (h: x, y, w)
 
@@ -305,11 +307,13 @@  subroutine foo (x, p, q, px, h, fl)
       .or. r2(1) /= (5 * p(3)) .or. r2(4) /= (6 * p(3))) then
     stop 25
   end if
-
 end subroutine
+end module m
 
 program main
   use omp_lib
+  use m
+  implicit none (type, external)
   integer, dimension(4) :: p
   integer, dimension(4) :: q
 
@@ -323,11 +327,11 @@  program main
   if (a == omp_null_allocator) stop 1
 
   call omp_set_default_allocator (omp_default_mem_alloc);
-  call foo (42, p, q, 2, a, 0);
-  call foo (42, p, q, 2, omp_default_mem_alloc, 0);
-  call foo (42, p, q, 2, a, 1);
+  call foo (42, p, q, a, 0);
+  call foo (42, p, q, omp_default_mem_alloc, 0);
+  call foo (42, p, q, a, 1);
   call omp_set_default_allocator (a);
-  call foo (42, p, q, 2, omp_null_allocator, 3);
-  call foo (42, p, q, 2, omp_default_mem_alloc, 2);
+  call foo (42, p, q, omp_null_allocator, 3);
+  call foo (42, p, q, omp_default_mem_alloc, 2);
   call omp_destroy_allocator (a);
 end