Re: First Patch

[Daniel Carrera <dcarrera at gmail dot com>]

Hi Tobias,

Aha... we're getting there. I fixed my LD_LIBRARY_PATH and now all the 
tests pass except for the new one I just added. So now the issue is in 
either my patch (most likely) or my test (less likely).

For completeness I'm attaching both the test case (sync_1.f90) and the 
patch (sans ChangeLog).


-------------------------%<-------------------------

==== sync_1.f90 =====

Backtrace for this error:

Backtrace for this error:

Backtrace for this error:
 #0  /usr/local/lib/libgfortran.so.3(+0x10904)[0xe71904]
 #1  /usr/local/lib/libgfortran.so.3(+0x113bd)[0xe723bd]
 #2  [0xaff400]
 #3 
/home/daniel/Other/GCC-GSoC/trunk/gcc/testsuite/gfortran.dg/coarray/a.out[0x8049121] 
in _gfortran_caf_sync_all
 #4 
/home/daniel/Other/GCC-GSoC/trunk/gcc/testsuite/gfortran.dg/coarray/a.out[0x8048D5B] 
in MAIN__ at sync_1.f90:0
--------------------------------------------------------------------------
mpiexec noticed that process rank 1 with PID 21216 on node daniel-lenovo 
exited on signal 11 (Segmentation fault).
--------------------------------------------------------------------------
 #0  /usr/local/lib/libgfortran.so.3(+0x10904)[0xd51904]
 #1  /usr/local/lib/libgfortran.so.3(+0x113bd)[0xd523bd]
 #2  [0x9b8400]
 #3 
/home/daniel/Other/GCC-GSoC/trunk/gcc/testsuite/gfortran.dg/coarray/a.out[0x8049121] 
in _gfortran_caf_sync_all
 #4 
/home/daniel/Other/GCC-GSoC/trunk/gcc/testsuite/gfortran.dg/coarray/a.out[0x8048D5B] 
in MAIN__ at sync_1.f90:0
 #0  /usr/local/lib/libgfortran.so.3(+0x10904)[0x120904]
 #1  /usr/local/lib/libgfortran.so.3(+0x113bd)[0x1213bd]
 #2  [0xbfd400]
 #3 
/home/daniel/Other/GCC-GSoC/trunk/gcc/testsuite/gfortran.dg/coarray/a.out[0x8049121] 
in _gfortran_caf_sync_all
 #4 
/home/daniel/Other/GCC-GSoC/trunk/gcc/testsuite/gfortran.dg/coarray/a.out[0x8048D5B] 
in MAIN__ at sync_1.f90:0

-------------------------%<-------------------------

Ok, so it's complaining about _gfortran_caf_sync_all. That's probably 
just because "sync all" is the first thing I test. So I commented out 
the "sync all" tests and ran the test again, and sure enough, it then 
complains about "sync images":


-------------------------%<-------------------------
Backtrace for this error:
 #0  /usr/local/lib/libgfortran.so.3(+0x10904)[0x166904]
 #1  /usr/local/lib/libgfortran.so.3(+0x113bd)[0x1673bd]
 #2  [0x5c9400]
 #3 
/home/daniel/Other/GCC-GSoC/trunk/gcc/testsuite/gfortran.dg/coarray/a.out[0x804913E] 
in _gfortran_caf_sync_images
 #4 
/home/daniel/Other/GCC-GSoC/trunk/gcc/testsuite/gfortran.dg/coarray/a.out[0x8048D4E] 
in MAIN__ at sync_1.f90:0
--------------------------------------------------------------------------
mpiexec noticed that process rank 1 with PID 5326 on node daniel-lenovo 
exited on signal 11 (Segmentation fault).
--------------------------------------------------------------------------

-------------------------%<-------------------------



I tried to trace the problem, and I suspect that I'm not running the 
latest version of my patch:

You see, I added some "debug" statements to _gfortran_caf_sync_all to at 
least identify which line causes the seg fault. I recompiled, 
reinstalled and re-ran the tests, but I don't see the debug statements.

So I suspect that when I run "make" in the build directory it might not 
be recompiling the right files. If I don't hear any better suggestions, 
I will delete my entire build directory and recompile from scratch. But 
on my computer recompiling GCC will take basically all day.

Btw, I'll compile with --enable-languages=c,fortran --disable-bootstrap


Cheers,
Daniel.
--
I'm not overweight, I'm undertall.

! { dg-do run }
! { dg-shouldfail "error stop" }
! 
! Coarray support
! PR fortran/18918

implicit none
integer :: n
character(len=30) :: str
critical
end critical
myCr: critical
end critical myCr

!
! Test SYNC ALL
!
!sync all
!sync all ( )
!sync all (errmsg=str)
!
!n = 5
!sync all (stat=n)
!if (n /= 0) call abort()
!
!n = 5
!sync all (stat=n,errmsg=str)
!if (n /= 0) call abort()


!
! Test SYNC MEMORY
!
sync memory
sync memory ( )
sync memory (errmsg=str)

n = 5
sync memory (stat=n)
if (n /= 0) call abort()

n = 5
sync memory (errmsg=str,stat=n)
if (n /= 0) call abort()


!
! Test SYNC IMAGES
!
sync images (*)
sync images (1)
sync images (1, errmsg=str)
sync images ([1])

n = 5
sync images (*, stat=n)
if (n /= 0) call abort()

n = 5
sync images ([1],errmsg=str,stat=n)
if (n /= 0) call abort()

end

Index: gcc/fortran/trans-stmt.c
===================================================================
--- gcc/fortran/trans-stmt.c	(revision 174722)
+++ gcc/fortran/trans-stmt.c	(working copy)
@@ -683,7 +683,9 @@
       gfc_conv_expr_val (&argse, code->expr2);
       stat = argse.expr;
     }
-
+  else
+    stat = null_pointer_node;
+   
   if (code->expr3 && gfc_option.coarray == GFC_FCOARRAY_LIB
       && type != EXEC_SYNC_MEMORY)
     {
@@ -743,13 +745,33 @@
     }
   else if (type == EXEC_SYNC_ALL)
     {
-      tmp = build_call_expr_loc (input_location, gfor_fndecl_caf_sync_all,
-				 2, errmsg, errmsglen);
-      if (code->expr2)
-	gfc_add_modify (&se.pre, stat, fold_convert (TREE_TYPE (stat), tmp));
+      /* SYNC ALL           =>   stat == null_pointer_node
+	 SYNC ALL(stat=s)   =>   stat has an integer type
+	 
+	 If "stat" is null or has the correct integer type, we use
+	 pass it to caf_sync_all directly.
+	 
+	 Otherwise, make a temp variable with the correct integer type,
+	 pass that to caf_sync_all, and then assign the result to "stat"
+	 casting it to the correct type.  */
+      if (stat == null_pointer_node || TREE_TYPE (stat) == integer_type_node)
+	{
+	  tmp = build_call_expr_loc (input_location, gfor_fndecl_caf_sync_all,
+				     3, stat, errmsg, errmsglen);
+	  gfc_add_expr_to_block (&se.pre, tmp);
+	}
       else
-	gfc_add_expr_to_block (&se.pre, tmp);
+	{
+	  tree tmp_stat = gfc_create_var (integer_type_node, "stat");
+	  
+	  tmp = build_call_expr_loc (input_location, gfor_fndecl_caf_sync_all,
+				     3, tmp_stat, errmsg, errmsglen);
+	  gfc_add_expr_to_block (&se.pre, tmp);
+	  
+	  gfc_add_modify (&se.pre, stat, fold_convert (TREE_TYPE (stat), tmp_stat));
+	}
     }
+  
   else
     {
       tree len;
@@ -790,15 +812,35 @@
           len = fold_convert (integer_type_node, len);
 	}
 
-      tmp = build_call_expr_loc (input_location, gfor_fndecl_caf_sync_images, 4,
-				 fold_convert (integer_type_node, len), images,
-				 errmsg, errmsglen);
-      if (code->expr2)
-	gfc_add_modify (&se.pre, stat, fold_convert (TREE_TYPE (stat), tmp));
+      /* SYNC ALL           =>   stat == null_pointer_node
+	 SYNC ALL(stat=s)   =>   stat has an integer type
+	 
+	 If "stat" is null or has the correct integer type, we use
+	 pass it to caf_sync_all directly.
+	 
+	 Otherwise, make a temp variable with the correct integer type,
+	 pass that to caf_sync_all, and then assign the result to "stat"
+	 casting it to the correct type.  */
+      if (stat == null_pointer_node || TREE_TYPE (stat) == integer_type_node)
+	{
+	  tmp = build_call_expr_loc (input_location, gfor_fndecl_caf_sync_images, 
+				     5, stat, fold_convert (integer_type_node, len),
+				     images, errmsg, errmsglen);
+	  gfc_add_expr_to_block (&se.pre, tmp);
+	}
       else
-	gfc_add_expr_to_block (&se.pre, tmp);
+	{
+	  tree tmp_stat = gfc_create_var (integer_type_node, "stat");
+	  
+	  tmp = build_call_expr_loc (input_location, gfor_fndecl_caf_sync_images, 
+				     5, tmp_stat, fold_convert (integer_type_node, len),
+				     images, errmsg, errmsglen);
+	  gfc_add_expr_to_block (&se.pre, tmp);
+	  
+	  gfc_add_modify (&se.pre, stat, fold_convert (TREE_TYPE (stat), tmp_stat));
+	}
     }
-
+  
   return gfc_finish_block (&se.pre);
 }
 
Index: gcc/fortran/trans-decl.c
===================================================================
--- gcc/fortran/trans-decl.c	(revision 174722)
+++ gcc/fortran/trans-decl.c	(working copy)
@@ -3059,13 +3059,13 @@
 	get_identifier (PREFIX("caf_end_critical")), void_type_node, 0);
 
       gfor_fndecl_caf_sync_all = gfc_build_library_function_decl_with_spec (
-	get_identifier (PREFIX("caf_sync_all")), ".W", integer_type_node,
-	2, build_pointer_type (pchar_type_node), integer_type_node);
+	get_identifier (PREFIX("caf_sync_all")), ".WW", void_type_node,
+	3, pint_type, build_pointer_type (pchar_type_node), integer_type_node);
 
       gfor_fndecl_caf_sync_images = gfc_build_library_function_decl_with_spec (
-	get_identifier (PREFIX("caf_sync_images")), ".RRW", integer_type_node,
-	4, integer_type_node, pint_type, build_pointer_type (pchar_type_node),
-	integer_type_node);
+	get_identifier (PREFIX("caf_sync_images")), ".RRWW", void_type_node,
+	5, pint_type, integer_type_node, pint_type,
+	build_pointer_type (pchar_type_node), integer_type_node);
 
       gfor_fndecl_caf_error_stop = gfc_build_library_function_decl (
 	get_identifier (PREFIX("caf_error_stop")),
Index: libgfortran/caf/single.c
===================================================================
--- libgfortran/caf/single.c	(revision 174722)
+++ libgfortran/caf/single.c	(working copy)
@@ -69,16 +69,19 @@
 }
 
 
-int
-_gfortran_caf_sync_all (char *errmsg __attribute__ ((unused)),
+void
+_gfortran_caf_sync_all (int *stat __attribute__ ((unused)),
+			char *errmsg __attribute__ ((unused)),
 			int errmsg_len __attribute__ ((unused)))
 {
-  return 0;
+  if (stat)
+    *stat = 0;
 }
 
-int
+void
 _gfortran_caf_sync_images (int count __attribute__ ((unused)),
 			   int images[] __attribute__ ((unused)),
+			   int *stat __attribute__ ((unused)),
 			   char *errmsg __attribute__ ((unused)),
 			   int errmsg_len __attribute__ ((unused)))
 {
@@ -94,7 +97,8 @@
       }
 #endif
 
-  return 0;
+  if (stat)
+    *stat = 0;
 }
 
 
Index: libgfortran/caf/libcaf.h
===================================================================
--- libgfortran/caf/libcaf.h	(revision 174722)
+++ libgfortran/caf/libcaf.h	(working copy)
@@ -54,8 +54,8 @@
 int _gfortran_caf_deregister (void **);
 
 
-int _gfortran_caf_sync_all (char *, int);
-int _gfortran_caf_sync_images (int, int[], char *, int);
+void _gfortran_caf_sync_all (int *, char *, int);
+void _gfortran_caf_sync_images (int, int[], int *, char *, int);
 
 /* FIXME: The CRITICAL functions should be removed;
    the functionality is better represented using Coarray's lock feature.  */
Index: libgfortran/caf/mpi.c
===================================================================
--- libgfortran/caf/mpi.c	(revision 174722)
+++ libgfortran/caf/mpi.c	(working copy)
@@ -94,24 +94,32 @@
 
 /* SYNC ALL - the return value matches Fortran's STAT argument.  */
 
-int
-_gfortran_caf_sync_all (char *errmsg, int errmsg_len)
+void
+_gfortran_caf_sync_all (int *stat, char *errmsg, int errmsg_len)
 {
-  int ierr;
-  ierr = MPI_Barrier (MPI_COMM_WORLD);
-
-  if (ierr && errmsg_len > 0)
+  /* TODO: Is ierr correct? When should STAT_STOPPED_IMAGE be used?  */
+  int ierr = MPI_Barrier (MPI_COMM_WORLD);
+  
+  if (stat)
+    *stat = ierr;
+  
+  if (ierr)
     {
       const char msg[] = "SYNC ALL failed";
-      int len = ((int) sizeof (msg) > errmsg_len) ? errmsg_len
-						  : (int) sizeof (msg);
-      memcpy (errmsg, msg, len);
-      if (errmsg_len > len)
-	memset (&errmsg[len], ' ', errmsg_len-len);
+      if (errmsg_len > 0)
+	{
+	  int len = ((int) sizeof (msg) > errmsg_len) ? errmsg_len
+	    : (int) sizeof (msg);
+	  memcpy (errmsg, msg, len);
+	  if (errmsg_len > len)
+	    memset (&errmsg[len], ' ', errmsg_len-len);
+	}
+      else
+	{
+	  fprintf (stderr,"SYNC ALL failed\n");
+	  error_stop (ierr);
+	}
     }
-
-  /* TODO: Is ierr correct? When should STAT_STOPPED_IMAGE be used?  */
-  return ierr;
 }
 
 
@@ -119,14 +127,13 @@
    SYNC IMAGES([]) has count == 0. Note further that SYNC IMAGES(*)
    is not equivalent to SYNC ALL.  The return value matches Fortran's
    STAT argument.  */
-int
-_gfortran_caf_sync_images (int count, int images[], char *errmsg,
+void
+_gfortran_caf_sync_images (int count, int images[], int *stat, char *errmsg,
 			   int errmsg_len)
 {
   int ierr;
-
   if (count == 0 || (count == 1 && images[0] == caf_this_image))
-    return 0;
+    *stat = 0;
 
 #ifdef GFC_CAF_CHECK
   {
@@ -151,20 +158,28 @@
     }
 
   /* Handle SYNC IMAGES(*).  */
+  /* TODO: Is ierr correct? When should STAT_STOPPED_IMAGE be used?  */
   ierr = MPI_Barrier (MPI_COMM_WORLD);
-
-  if (ierr && errmsg_len > 0)
+  if (stat)
+    *stat = ierr;
+  
+  if (ierr)
     {
       const char msg[] = "SYNC IMAGES failed";
-      int len = ((int) sizeof (msg) > errmsg_len) ? errmsg_len
-						  : (int) sizeof (msg);
-      memcpy (errmsg, msg, len);
-      if (errmsg_len > len)
-	memset (&errmsg[len], ' ', errmsg_len-len);
+      if (errmsg_len > 0)
+	{
+	  int len = ((int) sizeof (msg) > errmsg_len) ? errmsg_len
+	    : (int) sizeof (msg);
+	  memcpy (errmsg, msg, len);
+	  if (errmsg_len > len)
+	    memset (&errmsg[len], ' ', errmsg_len-len);
+	}
+      else
+	{
+	  fprintf (stderr,"SYNC IMAGES failed\n");
+	  error_stop (ierr);
+	}
     }
-
-  /* TODO: Is ierr correct? When should STAT_STOPPED_IMAGE be used?  */
-  return ierr;
 }