Re: [Patch, Fortran + Testsuite] Fix coarray handling in modules

Re: [Patch, Fortran + Testsuite] Fix coarray handling in modules

[Dominique Dhumieres]

Happy New Year to all.

The test gfortran.dg/coarray/codimension_2.f90 fails on x86_64-apple-darwin14 with -m32
(see https://gcc.gnu.org/ml/gcc-testresults/2015-01/msg00185.html). The error is

gfc /opt/gcc/work/gcc/testsuite/gfortran.dg/coarray/codimension_2.f90 /opt/gcc/work/gcc/testsuite/gfortran.dg/coarray/codimension_2a.f90 -fcoarray=lib -O2 -lcaf_single -m32
/var/folders/8q/sh_swgz96r7f5vnn08f7fxr00000gn/T//ccPcznrm.s:18:non-relocatable subtraction expression, "__F.caf_token__global_coarrays_MOD_b" minus "L1$pb"
/var/folders/8q/sh_swgz96r7f5vnn08f7fxr00000gn/T//ccPcznrm.s:18:symbol: "__F.caf_token__global_coarrays_MOD_b" can't be undefined in a subtraction expression

Dominique

Re: [Patch, Fortran + Testsuite] Fix coarray handling in modules

[Tobias Burnus]

[-- Attachment #1: Type: text/plain, Size: 729 bytes --]

Dominique Dhumieres wrote:
> The test gfortran.dg/coarray/codimension_2.f90 fails on x86_64-apple-darwin14 with -m32
> (see https://gcc.gnu.org/ml/gcc-testresults/2015-01/msg00185.html). The error is
> [...]

Yes, there seems to be something wrong. The module has:

                  U _F.caf_token__global_coarrays_MOD_b
0000000000000000 B __global_coarrays_MOD_b

where the token should not be "U"ndefined but "B". On the other hand, 
the USEr of the module has:

0000000000000000 B _F.caf_token__global_coarrays_MOD_b
                  U __global_coarrays_MOD_b

but it should have a "U" – with two users one even would get: "multiple 
definition of `_F.caf_token__global_coarrays_MOD_b'".


Untested patch attached.

Tobias

[-- Attachment #2: foo.diff --]
[-- Type: text/x-patch, Size: 991 bytes --]

diff --git a/gcc/fortran/trans-decl.c b/gcc/fortran/trans-decl.c
index 9ef6bfc..84a8a6e 100644
--- a/gcc/fortran/trans-decl.c
+++ b/gcc/fortran/trans-decl.c
@@ -830,14 +830,23 @@ gfc_build_qualified_array (tree decl, gfc_symbol * sym)
 			IDENTIFIER_POINTER (gfc_sym_mangled_identifier (sym))));
 	  token = build_decl (DECL_SOURCE_LOCATION (decl), VAR_DECL, token_name,
 			      token_type);
-	  TREE_PUBLIC (token) = 1;
+	  if (sym->attr.use_assoc)
+	    DECL_EXTERNAL (token) = 1;
+	  else
+	    TREE_STATIC (token) = 1;
+
+	  if (sym->attr.use_assoc || sym->attr.access != ACCESS_PRIVATE ||
+	      sym->attr.public_used)
+	    TREE_PUBLIC (token) = 1;
 	}
       else
-	token = gfc_create_var_np (token_type, "caf_token");
+	{
+	  token = gfc_create_var_np (token_type, "caf_token");
+	  TREE_STATIC (token) = 1;
+	}
 
       GFC_TYPE_ARRAY_CAF_TOKEN (type) = token;
       DECL_ARTIFICIAL (token) = 1;
-      TREE_STATIC (token) = 1;
       gfc_add_decl_to_function (token);
     }
 

Re: [Patch, Fortran + Testsuite] Fix coarray handling in modules

[Dominique d'Humières]

From a quick test, with the patch I still see the error with -m32

/var/folders/8q/sh_swgz96r7f5vnn08f7fxr00000gn/T//cc8Yz3Jr.s:18:non-relocatable subtraction expression, "__F.caf_token__global_coarrays_MOD_b" minus "L1$pb"
/var/folders/8q/sh_swgz96r7f5vnn08f7fxr00000gn/T//cc8Yz3Jr.s:18:symbol: "__F.caf_token__global_coarrays_MOD_b" can't be undefined in a subtraction expression

but also the test fails at link time with -m64

[Book15] f90/bug% gfc /opt/gcc/work/gcc/testsuite/gfortran.dg/coarray/codimension_2.f90 /opt/gcc/work/gcc/testsuite/gfortran.dg/coarray/codimension_2a.f90 -fcoarray=lib -O2 -lcaf_single
Undefined symbols for architecture x86_64:
  "__F.caf_token__global_coarrays_MOD_b", referenced from:
      __caf_init.0 in ccljvUii.o
      _MAIN__ in ccmqTPwK.o
ld: symbol(s) not found for architecture x86_64
collect2: error: ld returned 1 exit status

Dominique

> Le 3 janv. 2015 à 21:50, Tobias Burnus <burnus@net-b.de> a écrit :
> 
> Dominique Dhumieres wrote:
>> The test gfortran.dg/coarray/codimension_2.f90 fails on x86_64-apple-darwin14 with -m32
>> (see https://gcc.gnu.org/ml/gcc-testresults/2015-01/msg00185.html). The error is
>> [...]
> 
> Yes, there seems to be something wrong. The module has:
> 
>                 U _F.caf_token__global_coarrays_MOD_b
> 0000000000000000 B __global_coarrays_MOD_b
> 
> where the token should not be "U"ndefined but "B". On the other hand, the USEr of the module has:
> 
> 0000000000000000 B _F.caf_token__global_coarrays_MOD_b
>                 U __global_coarrays_MOD_b
> 
> but it should have a "U" – with two users one even would get: "multiple definition of `_F.caf_token__global_coarrays_MOD_b'".
> 
> 
> Untested patch attached.
> 
> Tobias
> <foo.diff>

Re: [Patch, Fortran + Testsuite] Fix coarray handling in modules

[Tobias Burnus]

[-- Attachment #1: Type: text/plain, Size: 310 bytes --]

Dominique d'Humières wrote:
>  From a quick test, with the patch I still see the error with -m32

It helps if one actually adds the decl. The following (still untested) 
should help. I also marked the token as nonaliasing (it really should!) 
and added for proc pointers the tree-public optimization.

Tobias

[-- Attachment #2: foo.diff --]
[-- Type: text/x-patch, Size: 1722 bytes --]

diff --git a/gcc/fortran/trans-decl.c b/gcc/fortran/trans-decl.c
index 9ef6bfc..976db2b 100644
--- a/gcc/fortran/trans-decl.c
+++ b/gcc/fortran/trans-decl.c
@@ -830,15 +830,32 @@ gfc_build_qualified_array (tree decl, gfc_symbol * sym)
 			IDENTIFIER_POINTER (gfc_sym_mangled_identifier (sym))));
 	  token = build_decl (DECL_SOURCE_LOCATION (decl), VAR_DECL, token_name,
 			      token_type);
-	  TREE_PUBLIC (token) = 1;
+	  if (sym->attr.use_assoc)
+	    DECL_EXTERNAL (token) = 1;
+	  else
+	    TREE_STATIC (token) = 1;
+
+	  if (sym->attr.use_assoc || sym->attr.access != ACCESS_PRIVATE ||
+	      sym->attr.public_used)
+	    TREE_PUBLIC (token) = 1;
 	}
       else
-	token = gfc_create_var_np (token_type, "caf_token");
+	{
+	  token = gfc_create_var_np (token_type, "caf_token");
+	  TREE_STATIC (token) = 1;
+	}
 
       GFC_TYPE_ARRAY_CAF_TOKEN (type) = token;
       DECL_ARTIFICIAL (token) = 1;
-      TREE_STATIC (token) = 1;
-      gfc_add_decl_to_function (token);
+      DECL_NONALIASED (token) = 1;
+
+      if (sym->module && !sym->attr.use_assoc)
+	{
+	  DECL_CONTEXT (token) = sym->ns->proc_name->backend_decl;
+	  gfc_module_add_decl (cur_module, token);
+	}
+      else
+	gfc_add_decl_to_function (token);
     }
 
   for (dim = 0; dim < GFC_TYPE_ARRAY_RANK (type); dim++)
@@ -1664,7 +1681,9 @@ get_proc_pointer_decl (gfc_symbol *sym)
   else if (sym->module && sym->ns->proc_name->attr.flavor == FL_MODULE)
     {
       /* This is the declaration of a module variable.  */
-      TREE_PUBLIC (decl) = 1;
+      if (sym->ns->proc_name->attr.flavor == FL_MODULE
+	  && (sym->attr.access != ACCESS_PRIVATE || sym->attr.public_used))
+	TREE_PUBLIC (decl) = 1;
       TREE_STATIC (decl) = 1;
     }
 

Re: [Patch, Fortran + Testsuite] Fix coarray handling in modules

[Dominique d'Humières]

Compilation with the new patch fails with

../../work/gcc/fortran/trans-decl.c: In function 'void gfc_build_qualified_array(tree, gfc_symbol*)':
../../work/gcc/fortran/trans-decl.c:855:25: error: 'cur_module' was not declared in this scope
    gfc_module_add_decl (cur_module, token);

Dominique

> Le 3 janv. 2015 à 23:30, Tobias Burnus <burnus@net-b.de> a écrit :
> 
> Dominique d'Humières wrote:
>> From a quick test, with the patch I still see the error with -m32
> 
> It helps if one actually adds the decl. The following (still untested) should help. I also marked the token as nonaliasing (it really should!) and added for proc pointers the tree-public optimization.
> 
> Tobias
> <foo.diff>

[Patch, Fortran] Fix previous patch (was: Re: [Patch, Fortran + Testsuite] Fix coarray handling in modules)

[Tobias Burnus]

[-- Attachment #1: Type: text/plain, Size: 1319 bytes --]

Attached is a regtested patch, which fixes the issue. Additionally, the 
variable visibility (TREE_PUBLIC) is now depending on the private 
attribute (copied from the module var generation) and I mark the tree as 
DECL_NONALIASED. The former I also did for proc-pointers, which is an 
unrelated patch.

Build and regtested on x86-64-gnu-linux.
OK for the trunk?


Dominique d'Humières wrote:
> Compilation with the new patch fails with
> ../../work/gcc/fortran/trans-decl.c: In function 'void gfc_build_qualified_array(tree, gfc_symbol*)':
> ../../work/gcc/fortran/trans-decl.c:855:25: error: 'cur_module' was not declared in this scope
>      gfc_module_add_decl (cur_module, token);

Which shows that w/o compiling and testing, one (usually) cannot write 
patches. Solution is to move the declaration up in the file. However, it 
turned out that that's not sufficient: the pushdecl is required.

Tobias

>> Le 3 janv. 2015 à 23:30, Tobias Burnus <burnus@net-b.de> a écrit :
>>
>> Dominique d'Humières wrote:
>>>  From a quick test, with the patch I still see the error with -m32
>> It helps if one actually adds the decl. The following (still untested) should help. I also marked the token as nonaliasing (it really should!) and added for proc pointers the tree-public optimization.
>>
>> Tobias
>> <foo.diff>
>


[-- Attachment #2: caf.diff --]
[-- Type: text/x-patch, Size: 4482 bytes --]

2015-01-02  Tobias Burnus  <burnus@net-b.de>

        * trans-decl.c (gfc_build_qualified_array): Fix coarray tokens
        for module coarrays with -fcoarray=lib.
	(get_proc_pointer_decl): As module variable, make only public
	when not marked as private.

	* gfortran.dg/coarray/codimension_2b.f90: New file.
	* gfortran.dg/coarray/codimension_2.f90: Add it to dg-extra-sources.
	* gfortran.dg/coarray/codimension_2.f90: Call its subroutine.

diff --git a/gcc/fortran/trans-decl.c b/gcc/fortran/trans-decl.c
index 9ef6bfc..dfc0f23 100644
--- a/gcc/fortran/trans-decl.c
+++ b/gcc/fortran/trans-decl.c
@@ -87,6 +87,8 @@ static gfc_namespace *module_namespace;
 /* The currently processed procedure symbol.  */
 static gfc_symbol* current_procedure_symbol = NULL;
 
+/* The currently processed module.  */
+static struct module_htab_entry *cur_module;
 
 /* With -fcoarray=lib: For generating the registering call
    of static coarrays.  */
@@ -830,15 +832,33 @@ gfc_build_qualified_array (tree decl, gfc_symbol * sym)
 			IDENTIFIER_POINTER (gfc_sym_mangled_identifier (sym))));
 	  token = build_decl (DECL_SOURCE_LOCATION (decl), VAR_DECL, token_name,
 			      token_type);
-	  TREE_PUBLIC (token) = 1;
+	  if (sym->attr.use_assoc)
+	    DECL_EXTERNAL (token) = 1;
+	  else
+	    TREE_STATIC (token) = 1;
+
+	  if (sym->attr.use_assoc || sym->attr.access != ACCESS_PRIVATE ||
+	      sym->attr.public_used)
+	    TREE_PUBLIC (token) = 1;
 	}
       else
-	token = gfc_create_var_np (token_type, "caf_token");
+	{
+	  token = gfc_create_var_np (token_type, "caf_token");
+	  TREE_STATIC (token) = 1;
+	}
 
       GFC_TYPE_ARRAY_CAF_TOKEN (type) = token;
       DECL_ARTIFICIAL (token) = 1;
-      TREE_STATIC (token) = 1;
-      gfc_add_decl_to_function (token);
+      DECL_NONALIASED (token) = 1;
+
+      if (sym->module && !sym->attr.use_assoc)
+	{
+	  pushdecl (token);
+	  DECL_CONTEXT (token) = sym->ns->proc_name->backend_decl;
+	  gfc_module_add_decl (cur_module, token);
+	}
+      else
+	gfc_add_decl_to_function (token);
     }
 
   for (dim = 0; dim < GFC_TYPE_ARRAY_RANK (type); dim++)
@@ -1664,7 +1684,9 @@ get_proc_pointer_decl (gfc_symbol *sym)
   else if (sym->module && sym->ns->proc_name->attr.flavor == FL_MODULE)
     {
       /* This is the declaration of a module variable.  */
-      TREE_PUBLIC (decl) = 1;
+      if (sym->ns->proc_name->attr.flavor == FL_MODULE
+	  && (sym->attr.access != ACCESS_PRIVATE || sym->attr.public_used))
+	TREE_PUBLIC (decl) = 1;
       TREE_STATIC (decl) = 1;
     }
 
@@ -4326,8 +4348,6 @@ gfc_module_add_decl (struct module_htab_entry *entry, tree decl)
     *slot = decl;
 }
 
-static struct module_htab_entry *cur_module;
-
 
 /* Generate debugging symbols for namelists. This function must come after
    generate_local_decl to ensure that the variables in the namelist are
diff --git a/gcc/testsuite/gfortran.dg/coarray/codimension_2.f90 b/gcc/testsuite/gfortran.dg/coarray/codimension_2.f90
index b211f9b..45d3374 100644
--- a/gcc/testsuite/gfortran.dg/coarray/codimension_2.f90
+++ b/gcc/testsuite/gfortran.dg/coarray/codimension_2.f90
@@ -1,5 +1,5 @@
 ! { dg-do link }
-! { dg-additional-sources codimension_2a.f90 }
+! { dg-additional-sources "codimension_2a.f90 codimension_2b.f90" }
 !
 ! To be used with codimension_2a.f90
 ! Check that the coarray declared in the module is accessible
diff --git a/gcc/testsuite/gfortran.dg/coarray/codimension_2a.f90 b/gcc/testsuite/gfortran.dg/coarray/codimension_2a.f90
index 8eb472c..3dec4aa 100644
--- a/gcc/testsuite/gfortran.dg/coarray/codimension_2a.f90
+++ b/gcc/testsuite/gfortran.dg/coarray/codimension_2a.f90
@@ -11,6 +11,7 @@
 program testmod
   use global_coarrays
   implicit none
+  external ttest
   
   integer :: me
 
@@ -21,6 +22,8 @@ program testmod
   if(me==1) then
      b(:) = b(:)[2]
      write(*,*) b
+  elseif (me == 3) then
+     call ttest()
   end if
 
 end program testmod
diff --git a/gcc/testsuite/gfortran.dg/coarray/codimension_2b.f90 b/gcc/testsuite/gfortran.dg/coarray/codimension_2b.f90
new file mode 100644
index 0000000..c30d051
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/coarray/codimension_2b.f90
@@ -0,0 +1,13 @@
+! { dg-do compile { target { ! *-*-* } } }
+! SKIP THIS FILE
+!
+! Used by codimension_2.f90
+!
+! Additional file to check that using the module doesn't generate
+! a token symbol. (The module is also used by codimension_2.f90.)
+!
+subroutine ttest
+  use global_coarrays
+  implicit none
+  b(:) = b(:)[2]
+end

Re: [Patch, Fortran] Fix previous patch (was: Re: [Patch, Fortran + Testsuite] Fix coarray handling in modules)

[Dominique d'Humières]

Dear Tobias,

I have done a clean bootstrap with your patch and run

make -k check-gfortran RUNTESTFLAGS="caf.exp --target_board=unix'{-m32,-m64}’" 

without regression.

Thanks,

Dominique

> Le 4 janv. 2015 à 19:57, Tobias Burnus <burnus@net-b.de> a écrit :
> 
> Attached is a regtested patch, which fixes the issue. Additionally, the variable visibility (TREE_PUBLIC) is now depending on the private attribute (copied from the module var generation) and I mark the tree as DECL_NONALIASED. The former I also did for proc-pointers, which is an unrelated patch.
> 
> Build and regtested on x86-64-gnu-linux.
> OK for the trunk?
> 

Re: [Patch, Fortran] Fix previous patch

[Tobias Burnus]

Early PING: https://gcc.gnu.org/ml/gcc-patches/2015-01/msg00090.html

Tobias Burnus wrote:
> Attached is a regtested patch, which fixes the issue. Additionally, 
> the variable visibility (TREE_PUBLIC) is now depending on the private 
> attribute (copied from the module var generation) and I mark the tree 
> as DECL_NONALIASED. The former I also did for proc-pointers, which is 
> an unrelated patch.
>
> Build and regtested on x86-64-gnu-linux.
> OK for the trunk?
>
>
> Dominique d'Humières wrote:
>> Compilation with the new patch fails with
>> ../../work/gcc/fortran/trans-decl.c: In function 'void 
>> gfc_build_qualified_array(tree, gfc_symbol*)':
>> ../../work/gcc/fortran/trans-decl.c:855:25: error: 'cur_module' was 
>> not declared in this scope
>>      gfc_module_add_decl (cur_module, token);
>
> Which shows that w/o compiling and testing, one (usually) cannot write 
> patches. Solution is to move the declaration up in the file. However, 
> it turned out that that's not sufficient: the pushdecl is required.
>
> Tobias
>
>>> Le 3 janv. 2015 à 23:30, Tobias Burnus <burnus@net-b.de> a écrit :
>>>
>>> Dominique d'Humières wrote:
>>>>  From a quick test, with the patch I still see the error with -m32
>>> It helps if one actually adds the decl. The following (still 
>>> untested) should help. I also marked the token as nonaliasing (it 
>>> really should!) and added for proc pointers the tree-public 
>>> optimization.
>>>
>>> Tobias
>>> <foo.diff>
>>
>

Re: [Patch, Fortran] Fix previous patch

[Tobias Burnus]

Paul Richard Thomas wrote:
> It looks to me as if you had to do a fair amount of detective work 
> there! The patch is OK for trunk.

Thanks for the review - which didn't make it to mailing list for some 
reasons (text+HTML email?).

Committed as Rev. 219354

Tobias

> Thanks for your efforts
>
> Paul
>
> On 7 January 2015 at 22:37, Tobias Burnus <burnus@net-b.de 
> <mailto:burnus@net-b.de>> wrote:
>
>     Early PING: https://gcc.gnu.org/ml/gcc-patches/2015-01/msg00090.html
>
>     Tobias Burnus wrote:
>
>         Attached is a regtested patch, which fixes the issue.
>         Additionally, the variable visibility (TREE_PUBLIC) is now
>         depending on the private attribute (copied from the module var
>         generation) and I mark the tree as DECL_NONALIASED. The former
>         I also did for proc-pointers, which is an unrelated patch.
>
>         Build and regtested on x86-64-gnu-linux.
>         OK for the trunk?
>
>
>         Dominique d'Humières wrote:
>
>             Compilation with the new patch fails with
>             ../../work/gcc/fortran/trans-decl.c: In function 'void
>             gfc_build_qualified_array(tree, gfc_symbol*)':
>             ../../work/gcc/fortran/trans-decl.c:855:25: error:
>             'cur_module' was not declared in this scope
>                  gfc_module_add_decl (cur_module, token);
>
>
>         Which shows that w/o compiling and testing, one (usually)
>         cannot write patches. Solution is to move the declaration up
>         in the file. However, it turned out that that's not
>         sufficient: the pushdecl is required.
>
>         Tobias
>
>                 Le 3 janv. 2015 à 23:30, Tobias Burnus
>                 <burnus@net-b.de <mailto:burnus@net-b.de>> a écrit :
>
>                 Dominique d'Humières wrote:
>
>                      From a quick test, with the patch I still see the
>                     error with -m32
>
>                 It helps if one actually adds the decl. The following
>                 (still untested) should help. I also marked the token
>                 as nonaliasing (it really should!) and added for proc
>                 pointers the tree-public optimization.
>
>                 Tobias
>                 <foo.diff>
>
>
>
>
>
>
>
> -- 
> Outside of a dog, a book is a man's best friend. Inside of a dog it's 
> too dark to read.
>
> Groucho Marx