diff mbox

[Fortran] Wrong invocation of caf_atomic_op

Message ID 541D8BC3.8030602@net-b.de
State New
Headers show

Commit Message

Tobias Burnus Sept. 20, 2014, 2:14 p.m. UTC
Hi Alessandro et al.,

I have now committed (Rev. 215421) the attached patch, which includes a 
test case for it.

Thanks for the report and the patch Alessandro!

Tobias

On 15.09.2014 23:29, Tobias Burnus wrote:
> On 15.09.2014 22:23, Alessandro Fanfarillo wrote:
>> 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.
>
> Well, if it is a "dg-do compile" test, it won't exercise the issue: 
> Even without the patch, it was compiling.
>
> It should either be a run test (dg-do run) – and then under 
> gfortran.dg/coarray/ - which will automatically link libcaf_single to 
> it. (And do another run with -fcoarray=single). – Or you have to 
> additionally use -fdump-tree-original and scan for the strings, using 
> "dg-final { scan-tree-dump-times ... or scan-tree-dump-not. See other 
> test cases there.
>
> I think ATOMIC_ADD should also fail without the patch, i.e. the run 
> test really would test whether it works. Thus, that might be the 
> simpler option. Or you do both – but the dump one shouldn't be under 
> gfortran.dg/coarray/ as that is also run with -fcoarray=single, but 
> under gfortran.dg/ directly.
>
> Tobias
>

Comments

Alessandro Fanfarillo Sept. 23, 2014, 3:29 a.m. UTC | #1
Thanks a lot. I'm moving and I didn't have time to make a test case.
Regards

2014-09-20 8:14 GMT-06:00 Tobias Burnus <burnus@net-b.de>:
> Hi Alessandro et al.,
>
> I have now committed (Rev. 215421) the attached patch, which includes a test
> case for it.
>
> Thanks for the report and the patch Alessandro!
>
> Tobias
>
>
> On 15.09.2014 23:29, Tobias Burnus wrote:
>>
>> On 15.09.2014 22:23, Alessandro Fanfarillo wrote:
>>>
>>> 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.
>>
>>
>> Well, if it is a "dg-do compile" test, it won't exercise the issue: Even
>> without the patch, it was compiling.
>>
>> It should either be a run test (dg-do run) – and then under
>> gfortran.dg/coarray/ - which will automatically link libcaf_single to it.
>> (And do another run with -fcoarray=single). – Or you have to additionally
>> use -fdump-tree-original and scan for the strings, using "dg-final {
>> scan-tree-dump-times ... or scan-tree-dump-not. See other test cases there.
>>
>> I think ATOMIC_ADD should also fail without the patch, i.e. the run test
>> really would test whether it works. Thus, that might be the simpler option.
>> Or you do both – but the dump one shouldn't be under gfortran.dg/coarray/ as
>> that is also run with -fcoarray=single, but under gfortran.dg/ directly.
>>
>> Tobias
>>
>
diff mbox

Patch

2014-09-20  Alessandro Fanfarillo  <fanfarillo.gcc@gmail.com>
	    Tobias Burnus  <burnus@net-b.de>

gcc/fortran
	* trans-intrinsic.c (conv_intrinsic_atomic_op): Fix issue with
	generating temporary for value argument.

gcc/testsuite/
	* gfortran.dg/coarray_atomic_5.f90: New

diff --git a/gcc/fortran/trans-intrinsic.c b/gcc/fortran/trans-intrinsic.c
index 0a83ad0..0a3315d 100644
--- a/gcc/fortran/trans-intrinsic.c
+++ b/gcc/fortran/trans-intrinsic.c
@@ -8396,7 +8405,7 @@  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");
 	  gfc_add_modify (&block, tmp, fold_convert (TREE_TYPE (tmp), value));
diff --git a/gcc/testsuite/gfortran.dg/coarray_atomic_5.f90 b/gcc/testsuite/gfortran.dg/coarray_atomic_5.f90
new file mode 100644
index 0000000..11d52ba
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/coarray_atomic_5.f90
@@ -0,0 +1,26 @@ 
+! { dg-do compile }
+! { dg-options "-fdump-tree-original -fcoarray=lib" }
+!
+! Argument passing was wrong
+!
+
+program atomic
+  use iso_fortran_env
+  implicit none
+
+  integer :: me
+  integer(atomic_int_kind) :: atom[*]
+  me = this_image()
+  call atomic_define(atom[1],0)
+  sync all
+  call ATOMIC_ADD (atom[1], me)
+  if(me == 1) call atomic_ref(me,atom[1])
+  sync all
+  write(*,*) me
+end program
+
+! { dg-final { scan-tree-dump-times "value.. = 0;" 1 "original" } }
+! { dg-final { scan-tree-dump-times "_gfortran_caf_atomic_define \\(caf_token.0, 0, 1, &value.., 0B, 1, 4\\);" 1 "original" } }
+! { dg-final { scan-tree-dump-times "_gfortran_caf_atomic_op \\(1, caf_token.0, 0, 1, &me, 0B, 0B, 1, 4\\);" 1 "original" } }
+! { dg-final { scan-tree-dump-times "_gfortran_caf_atomic_ref \\(caf_token.0, 0, 1, &me, 0B, 1, 4\\);" 1 "original" } }
+! { dg-final { cleanup-tree-dump "original" } }