This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH, Fortran] Wrong invocation of caf_atomic_op
- From: Alessandro Fanfarillo <fanfarillo dot gcc at gmail dot com>
- To: Tobias Burnus <burnus at net-b dot de>
- Cc: gfortran <fortran at gcc dot gnu dot org>, gcc-patches <gcc-patches at gcc dot gnu dot org>
- Date: Mon, 15 Sep 2014 14:23:24 -0600
- Subject: Re: [PATCH, Fortran] Wrong invocation of caf_atomic_op
- Authentication-results: sourceware.org; auth=none
- References: <CAHqFgjXx_T=wG88mTAcCT2xK01XkZG3CTCBm44moELAYBMbJmg at mail dot gmail dot com> <540E9E97 dot 6040602 at net-b dot de> <CAHqFgjW=Gs4kOAwNb=Hciqd3v9S=YhsLeHPSWLAh=XQobvV+yA at mail dot gmail dot com> <54173607 dot 7010008 at net-b dot de>
In attachment a test case which fails with the current gcc-trunk
version but works when the patch is applied. coarray_35.f90 is my
attempt to write a gcc test case.
The problem is related with atomic_add.
2014-09-15 12:55 GMT-06:00 Tobias Burnus <burnus@net-b.de>:
> On 15.09.2014 19:18, Alessandro Fanfarillo wrote:
>
> New patch after the update.
>
>
> 2014-09-09 0:30 GMT-06:00 Tobias Burnus <burnus@net-b.de>:
>
>> I think I'd prefer the following patch, which avoids a temporary if none
>> is
>> required. "value" is a pointer if the kind is the same (see kind check
>> before) and if it is not a literal. Otherwise, it isn't a pointer and one
>> needs to generate a temporary.
>>
>> I do not quite understand why the current check doesn't work as both are
>> integer(kind=4) but for some reasons one has a variant.
>>
>> Additionally, I wonder whether one should add a test case â one probably
>> should do â and of which kind (run test + fdump-tree-original?).
>
>> @@ -8398,3 +8398,3 @@ conv_intrinsic_atomic_op (gfc_code *code)
>> - if (TREE_TYPE (TREE_TYPE (atom)) != TREE_TYPE (TREE_TYPE (value)))
>> + if (!POINTER_TYPE_P (TREE_TYPE (value)))
>
> 2014-09-15 Alessandro Fanfarillo <fanfarillo.gcc@gmail.com>
> Tobias Burnus <burnus@net-b.de>
>
> * trans-intrinsic.c (conv_intrinsic_atomic_op):
> Check for indirect reference for caf_atomic_op value.
>
> diff --git a/gcc/fortran/trans-intrinsic.c b/gcc/fortran/trans-intrinsic.c
> index a13b113..2d7241a 100644
> --- a/gcc/fortran/trans-intrinsic.c
> +++ b/gcc/fortran/trans-intrinsic.c
> @@ -8396,9 +8396,11 @@ conv_intrinsic_atomic_op (gfc_code *code)
> else
> image_index = integer_zero_node;
>
> - if (TREE_TYPE (TREE_TYPE (atom)) != TREE_TYPE (TREE_TYPE (value)))
> + if (!POINTER_TYPE_P (TREE_TYPE (value)))
> {
> tmp = gfc_create_var (TREE_TYPE (TREE_TYPE (atom)), "value");
> + if (POINTER_TYPE_P (TREE_TYPE (value)))
> + value = build_fold_indirect_ref_loc (input_location, value);
>
>
> The second part makes no sense: If "value" is not a pointer (which is the
> first condition), it can never be a pointer (second condition).
>
> Otherwise, the patch is okay. The reason I hadn't committed it myself was
> that I wanted to include a test case; I was wondering whether it should be a
> run test â or a -fdump-tree-original + scan-tree test â or both.
>
> Can you create a test case?
>
> Tobias
! { dg-do compile }
! { dg-options "-fcoarray=lib" }
!
use iso_fortran_env
implicit none
integer :: me,np,res
integer(atomic_int_kind) :: atom[*]
me = this_image()
np = num_images()
call atomic_define(atom[1],0)
sync all
call ATOMIC_ADD (atom[1], me)
sync all
if(me == 1) then
call atomic_ref(res,atom[1])
if(res /= (np*(np+1))/2) then
{ dg-error "Passing pointer address" }
endif
endif
end
program atomic
use iso_fortran_env
implicit none
integer :: me,np,res
integer(atomic_int_kind) :: atom[*]
me = this_image()
np = num_images()
call atomic_define(atom[1],0)
sync all
call ATOMIC_ADD (atom[1], me)
sync all
if(me == 1) then
call atomic_ref(res,atom[1])
if(res /= (np*(np+1))/2) then
write(*,*) 'res',res
call abort()
endif
write(*,*) 'OK'
endif
end program