Bug 89651 - OpenMP private array uninitialized warning with -O flag
Summary: OpenMP private array uninitialized warning with -O flag
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: fortran (show other bugs)
Version: 8.2.1
: P4 normal
Target Milestone: 7.5
Assignee: Not yet assigned to anyone
URL:
Keywords: diagnostic, openmp
Depends on:
Blocks: Wuninitialized
  Show dependency treegraph
 
Reported: 2019-03-11 01:59 UTC by Jim Feng
Modified: 2019-08-30 13:35 UTC (History)
1 user (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2019-03-11 00:00:00


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Jim Feng 2019-03-11 01:59:47 UTC
A private array variable generates an uninitialized warning when compiled with -O, -O1, -O2, -O3. Without optimization flag, no error.
==============================================
program omp_array_private
  use omp_lib

  integer ::  n
  real, allocatable :: t(:)
  real :: s

  n=10
  allocate(t(n),source=0.0)

!$omp parallel private(t)

        s=sum(t)

!$omp end parallel

end program omp_array_private
==============================================

compile flag : gfortran -O3 -fopenmp   -Wall

++++++++++++++++++++++++++++++++++++++++++++++
Results:


Warning: ‘t.dim[0].lbound’ may be used uninitialized in this function [-Wmaybe-uninitialized]

Warning: ‘t.dim[0].ubound’ may be used uninitialized in this function [-Wmaybe-uninitialized]

 
Warning: ‘t.offset’ may be used uninitialized in this function [-Wmaybe-uninitialized]
Comment 1 Richard Biener 2019-03-11 08:39:28 UTC
Confirmed.  OMP expansion produces:

  <bb 36> :

  <bb 24> :
  D.3887 = .omp_data_i->t;
  D.3888 = D.3887->data;
  if (D.3888 != 0B)
    goto <bb 25>; [INV]
  else
    goto <bb 28>; [INV]

  <bb 29> :
  val.4 = 0.0;
  D.3891 = t.data;
  D.3892 = t.offset;
  D.3893 = t.dim[0].lbound;
  D.3894 = t.dim[0].ubound;
  S.5 = D.3893;

...
  <bb 28> :
  t.data = 0B;
  goto <bb 29>; [INV]

somehow t is not localized?
Comment 2 Dominique d'Humieres 2019-03-11 09:04:01 UTC
Related to pr87143 and probably others.
Comment 3 Jakub Jelinek 2019-03-11 15:26:27 UTC
t is privatized and the emitted code looks just fine to me.

The standard says for privatization clauses:
For a list item or the subobject of a list item with the ALLOCATABLE attribute:
- if the allocation status is “not currently allocated”, the new list item or the subobject of the new list item will have an initial allocation status of "not currently allocated".
- if the allocation status is “currently allocated”, the new list item or the subobject of the new list item will have an initial allocation status of "currently allocated".
- If the new list item or the subobject of the new list item is an array, its bounds will be the same as those of the original list item or the subobject of the original list item.

and that is what GCC implements.  If you e.g. look at omplower dump, there is:
                D.3953 = .omp_data_i->t;
                D.3954 = D.3953->data;
                if (D.3954 != 0B) goto <D.3955>; else goto <D.3956>;
                <D.3955>:
                D.3953 = .omp_data_i->t;
                t = *D.3953;
                D.3957 = t.dim[0].ubound;
                D.3958 = t.dim[0].lbound;
                D.3959 = D.3957 - D.3958;
                D.3960 = D.3959 + 1;
                D.3961 = D.3960 * 4;
                D.3951 = (unsigned long) D.3961;
                D.3962 = MAX_EXPR <D.3951, 1>;
                D.3952 = __builtin_malloc (D.3962);
                D.3963 = D.3952 == 0B;
                D.3964 = (integer(kind=8)) D.3963;
                D.3965 = .BUILTIN_EXPECT (D.3964, 0, 42);
                D.3966 = (logical(kind=1)) D.3965;
                if (D.3966 != 0) goto <D.3967>; else goto <D.3968>;
                <D.3967>:
                _gfortran_os_error (&"Allocation would exceed memory limit"[1]{lb: 1 sz: 1});
                <D.3968>:
                t.data = D.3952;
                goto <D.3969>;
                <D.3956>:
                t.data = 0B;
                <D.3969>:
where *.omp_data_i->t is the descriptor of the original t and t being assigned is the privatized t.  If the original t is not allocated, then we only clear t.data and leave the rest uninitialized, exactly like if in the testcase
you comment out the allocate and !$omp lines.  If t is allocated, then everything is copied and new array allocated.

To shut up the warning, we could e.g. do:
  D.3953 = .omp_data_i->t;
  t = *D.3953;
unconditionally instead of conditionally, but it would be a wasteful at runtime for the case when the original var is not allocated, copying 64 bytes that won't be really needed.

Another option (best) would be to propagate from the outer to inner outlined OpenMP regions information like that .omp_data_i->t.data is non-NULL etc.; while we should improve the IPA opts on the boundary of OpenMP regions and pass stuff like VRP info etc. down, I'm afraid because this is a pointer inside a large struct and we don't really do VRP or bitwise ccp, alignment info etc. for aggregate elts unless SRA optimized, this won't happen any time soon.

So yet another option would be to set TREE_NO_WARNING here on the privatized variable.
Comment 4 Jakub Jelinek 2019-03-11 17:01:07 UTC
On the other side, the testcase is invalid, because you are summing uninitialized data.  It is like if you did:
program pr89651
  integer :: n
  real, allocatable :: t(:)
  n = 10
  allocate (t(n))
  print *, sum (t)
end program pr89651
but it is true gfortran doesn't warn here either.  And we do warn even with firstprivate(t) when it is not undefined.
Comment 5 Jim Feng 2019-03-11 18:49:20 UTC
(In reply to Jakub Jelinek from comment #4)
> On the other side, the testcase is invalid, because you are summing
> uninitialized data.  It is like if you did:
> program pr89651
>   integer :: n
>   real, allocatable :: t(:)
>   n = 10
>   allocate (t(n))
>   print *, sum (t)
> end program pr89651
> but it is true gfortran doesn't warn here either.  And we do warn even with
> firstprivate(t) when it is not undefined.

In the test case, the array is initialized:

allocate(t(n),source=0.0)

It is puzzling that normal compile without optimization is OK. My concern is whether I should just disregard the warning or compile the program without optimization.
Comment 6 Jakub Jelinek 2019-03-11 18:56:16 UTC
(In reply to Jim Feng from comment #5)
> (In reply to Jakub Jelinek from comment #4)
> > On the other side, the testcase is invalid, because you are summing
> > uninitialized data.  It is like if you did:
> > program pr89651
> >   integer :: n
> >   real, allocatable :: t(:)
> >   n = 10
> >   allocate (t(n))
> >   print *, sum (t)
> > end program pr89651
> > but it is true gfortran doesn't warn here either.  And we do warn even with
> > firstprivate(t) when it is not undefined.
> 
> In the test case, the array is initialized:
> 
> allocate(t(n),source=0.0)

Outside of the OpenMP region yes, but you are using private clause, and the privatized variable is just allocated, but not initialized.  You'd need to use firstprivate(t) clause instead to copy the data too.
Comment 7 Jim Feng 2019-03-11 19:00:46 UTC
(In reply to Jakub Jelinek from comment #6)
> (In reply to Jim Feng from comment #5)
> > (In reply to Jakub Jelinek from comment #4)
> > > On the other side, the testcase is invalid, because you are summing
> > > uninitialized data.  It is like if you did:
> > > program pr89651
> > >   integer :: n
> > >   real, allocatable :: t(:)
> > >   n = 10
> > >   allocate (t(n))
> > >   print *, sum (t)
> > > end program pr89651
> > > but it is true gfortran doesn't warn here either.  And we do warn even with
> > > firstprivate(t) when it is not undefined.
> > 
> > In the test case, the array is initialized:
> > 
> > allocate(t(n),source=0.0)
> 
> Outside of the OpenMP region yes, but you are using private clause, and the
> privatized variable is just allocated, but not initialized.  You'd need to
> use firstprivate(t) clause instead to copy the data too.

I just did firstprivate  instead of private directive. Eaxct same error.
Comment 8 Jakub Jelinek 2019-03-11 22:28:10 UTC
Author: jakub
Date: Mon Mar 11 22:27:39 2019
New Revision: 269598

URL: https://gcc.gnu.org/viewcvs?rev=269598&root=gcc&view=rev
Log:
	PR fortran/89651
	* trans-openmp.c (gfc_omp_clause_default_ctor): Set TREE_NO_WARNING
	on decl if adding COND_EXPR for allocatable.
	(gfc_omp_clause_copy_ctor): Set TREE_NO_WARNING on dest.

	* gfortran.dg/gomp/pr89651.f90: New test.

Added:
    trunk/gcc/testsuite/gfortran.dg/gomp/pr89651.f90
Modified:
    trunk/gcc/fortran/ChangeLog
    trunk/gcc/fortran/trans-openmp.c
    trunk/gcc/testsuite/ChangeLog
Comment 9 Jakub Jelinek 2019-03-11 22:55:23 UTC
Fixed for gcc 9.1+ so far.
Comment 10 Jakub Jelinek 2019-04-30 20:40:49 UTC
Author: jakub
Date: Tue Apr 30 20:40:17 2019
New Revision: 270724

URL: https://gcc.gnu.org/viewcvs?rev=270724&root=gcc&view=rev
Log:
	Backported from mainline
	2019-03-11  Jakub Jelinek  <jakub@redhat.com>

	PR fortran/89651
	* trans-openmp.c (gfc_omp_clause_default_ctor): Set TREE_NO_WARNING
	on decl if adding COND_EXPR for allocatable.
	(gfc_omp_clause_copy_ctor): Set TREE_NO_WARNING on dest.

	* gfortran.dg/gomp/pr89651.f90: New test.

Added:
    branches/gcc-8-branch/gcc/testsuite/gfortran.dg/gomp/pr89651.f90
Modified:
    branches/gcc-8-branch/gcc/fortran/ChangeLog
    branches/gcc-8-branch/gcc/fortran/trans-openmp.c
    branches/gcc-8-branch/gcc/testsuite/ChangeLog
Comment 11 Jakub Jelinek 2019-05-01 07:12:01 UTC
Fixed for 8.4+ too.
Comment 12 Jakub Jelinek 2019-08-30 12:16:22 UTC
Author: jakub
Date: Fri Aug 30 12:15:50 2019
New Revision: 275127

URL: https://gcc.gnu.org/viewcvs?rev=275127&root=gcc&view=rev
Log:
	Backported from mainline
	2019-03-11  Jakub Jelinek  <jakub@redhat.com>

	PR fortran/89651
	* trans-openmp.c (gfc_omp_clause_default_ctor): Set TREE_NO_WARNING
	on decl if adding COND_EXPR for allocatable.
	(gfc_omp_clause_copy_ctor): Set TREE_NO_WARNING on dest.

	* gfortran.dg/gomp/pr89651.f90: New test.

Added:
    branches/gcc-7-branch/gcc/testsuite/gfortran.dg/gomp/pr89651.f90
Modified:
    branches/gcc-7-branch/gcc/fortran/ChangeLog
    branches/gcc-7-branch/gcc/fortran/trans-openmp.c
    branches/gcc-7-branch/gcc/testsuite/ChangeLog
Comment 13 Jakub Jelinek 2019-08-30 13:35:59 UTC
Fixed.