From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 13134 invoked by alias); 10 Jun 2011 09:23:48 -0000 Received: (qmail 13118 invoked by uid 22791); 10 Jun 2011 09:23:47 -0000 X-SWARE-Spam-Status: No, hits=-2.6 required=5.0 tests=AWL,BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,FREEMAIL_FROM,RCVD_IN_DNSWL_LOW,RFC_ABUSE_POST,TW_CP X-Spam-Check-By: sourceware.org Received: from mail-fx0-f47.google.com (HELO mail-fx0-f47.google.com) (209.85.161.47) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Fri, 10 Jun 2011 09:23:27 +0000 Received: by fxm19 with SMTP id 19so1780817fxm.20 for ; Fri, 10 Jun 2011 02:23:26 -0700 (PDT) Received: by 10.223.143.67 with SMTP id t3mr1752103fau.105.1307697152226; Fri, 10 Jun 2011 02:12:32 -0700 (PDT) Received: from [130.235.236.18] (ip236-18.wireless.lu.se [130.235.236.18]) by mx.google.com with ESMTPS id e15sm971449faa.23.2011.06.10.02.12.30 (version=SSLv3 cipher=OTHER); Fri, 10 Jun 2011 02:12:31 -0700 (PDT) Message-ID: <4DF1E006.1070504@gmail.com> Date: Fri, 10 Jun 2011 09:58:00 -0000 From: Daniel Carrera User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.2.17) Gecko/20110424 Thunderbird/3.1.10 MIME-Version: 1.0 To: gfortran , gcc-patches@gcc.gnu.org Subject: [PATCH, Fortran] (Coarray) Change declaration of CAF sync functions. Content-Type: multipart/mixed; boundary="------------060601060303070400020900" Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org X-SW-Source: 2011-06/txt/msg00813.txt.bz2 This is a multi-part message in MIME format. --------------060601060303070400020900 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Content-length: 2077 This is the patch recently discussed in the GFortran list, now ready for official submission. As my first submission as a GSoC student, this is a simple patch mainly intended to familiarize me with GFortran. I changed the signature of the functions _gfortran_caf_sync_all and _gfortran_caf_sync_images so that the "stat" (status) is a function parameter. For example: ============== BEFORE: void * _gfortran_caf_sync_all (char *errmsg, int errmsg_len) ============== AFTER: void _gfortran_caf_sync_all (int *stat, char *errmsg, int errmsg_len) -------------- This will be necessary to implement SYNC ALL and SYNC IMAGES correctly. In the process I also fixed a couple other issues: The parameters "errmsg" and "images" (caf_sync_images) were not passed correctly. Attached is my patch, along with the corresponding test cases, which is destined for gcc/testsuite/gfortran.dg/coarray. Lastly, here is my ChangeLog: =================== ./gcc/fortran/ChangeLog =================== 2011-06-07 Daniel Carrera * trans-decl.c (gfc_build_builtin_function_decls): Updated declaration of caf_sync_all and caf_sync_images. * trans-stmt.c (gfc_trans_sync): Function can now handle a "stat" variable that has an integer type different from integer_type_node. =================== ./libgfortran/ChangeLog =================== 2011-06-07 Daniel Carrera * caf/mpi.c (_gfortran_caf_sync_all, _gfortran_caf_sync_images): Functions have void return type and move status into parameter list. * caf/single.c (_gfortran_caf_sync_all, _gfortran_caf_sync_images): Functions have void return type and move status into parameter list. * caf/libcaf.h (_gfortran_caf_sync_all, _gfortran_caf_sync_images): Functions have void return type and move status into parameter list. =================== ./gcc/testsuite/ChangeLog =================== 2011-06-07 Daniel Carrera * gfortran.dg/coarray/sync_1.f90: New Test "SYNC ALL", "SYNC MEMORY" and "SYNC IMAGES". -- I'm not overweight, I'm undertall. --------------060601060303070400020900 Content-Type: text/x-patch; name="CAF-sync_all+sync_images.patch" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="CAF-sync_all+sync_images.patch" Content-length: 9162 Index: gcc/fortran/trans-stmt.c =================================================================== --- gcc/fortran/trans-stmt.c (revision 174722) +++ gcc/fortran/trans-stmt.c (working copy) @@ -683,6 +683,8 @@ 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) @@ -691,7 +693,7 @@ gfc_init_se (&argse, NULL); gfc_conv_expr (&argse, code->expr3); gfc_conv_string_parameter (&argse); - errmsg = argse.expr; + errmsg = gfc_build_addr_expr (NULL, argse.expr); errmsglen = argse.string_length; } else if (gfc_option.coarray == GFC_FCOARRAY_LIB && type != EXEC_SYNC_MEMORY) @@ -743,12 +745,32 @@ } 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" has the wrong integer type, use a temp variable of + the right type and later cast the result back into "stat". */ + if (stat == null_pointer_node || TREE_TYPE (stat) == integer_type_node) + { + if (TREE_TYPE (stat) == integer_type_node) + stat = gfc_build_addr_expr (NULL, stat); + + 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, gfc_build_addr_expr (NULL, 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 { @@ -790,13 +812,34 @@ 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 IMAGES(imgs) => stat == null_pointer_node + SYNC IMAGES(imgs,stat=s) => stat has an integer type + + If "stat" has the wrong integer type, use a temp variable of + the right type and later cast the result back into "stat". */ + if (stat == null_pointer_node || TREE_TYPE (stat) == integer_type_node) + { + if (TREE_TYPE (stat) == integer_type_node) + stat = gfc_build_addr_expr (NULL, stat); + + tmp = build_call_expr_loc (input_location, gfor_fndecl_caf_sync_images, + 5, fold_convert (integer_type_node, len), + images, 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_images, + 5, fold_convert (integer_type_node, len), + images, gfc_build_addr_expr (NULL, 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)); + } } 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, integer_type_node, pint_type, 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, + 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, 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) @@ -92,42 +92,50 @@ } -/* 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); + /* TODO: Is ierr correct? When should STAT_STOPPED_IMAGE be used? */ + int ierr = MPI_Barrier (MPI_COMM_WORLD); - if (ierr && errmsg_len > 0) + 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; } /* SYNC IMAGES. Note: SYNC IMAGES(*) is passed as count == -1 while 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, + is not equivalent to SYNC ALL. */ +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; - + { + if (stat) + *stat = 0; + return; + } + #ifdef GFC_CAF_CHECK { int i; @@ -151,20 +159,28 @@ } /* Handle SYNC IMAGES(*). */ + /* TODO: Is ierr correct? When should STAT_STOPPED_IMAGE be used? */ ierr = MPI_Barrier (MPI_COMM_WORLD); + if (stat) + *stat = ierr; - if (ierr && errmsg_len > 0) + 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; } --------------060601060303070400020900 Content-Type: text/x-fortran; name="sync_1.f90" Content-Transfer-Encoding: base64 Content-Disposition: attachment; filename="sync_1.f90" Content-length: 1147 ISB7IGRnLWRvIHJ1biB9CiEgeyBkZy1zaG91bGRmYWlsICJlcnJvciBzdG9w IiB9CiEgCiEgQ29hcnJheSBzdXBwb3J0CiEgUFIgZm9ydHJhbi8xODkxOAoK aW1wbGljaXQgbm9uZQppbnRlZ2VyIDo6IG4KY2hhcmFjdGVyKGxlbj0zMCkg Ojogc3RyCmNyaXRpY2FsCmVuZCBjcml0aWNhbApteUNyOiBjcml0aWNhbApl bmQgY3JpdGljYWwgbXlDcgoKIQohIFRlc3QgU1lOQyBBTEwKIQpzeW5jIGFs bApzeW5jIGFsbCAoICkKc3luYyBhbGwgKGVycm1zZz1zdHIpCgpuID0gNQpz eW5jIGFsbCAoc3RhdD1uKQppZiAobiAvPSAwKSBjYWxsIGFib3J0KCkKCm4g PSA1CnN5bmMgYWxsIChzdGF0PW4sZXJybXNnPXN0cikKaWYgKG4gLz0gMCkg Y2FsbCBhYm9ydCgpCgoKIQohIFRlc3QgU1lOQyBNRU1PUlkKIQpzeW5jIG1l bW9yeQpzeW5jIG1lbW9yeSAoICkKc3luYyBtZW1vcnkgKGVycm1zZz1zdHIp CgpuID0gNQpzeW5jIG1lbW9yeSAoc3RhdD1uKQppZiAobiAvPSAwKSBjYWxs IGFib3J0KCkKCm4gPSA1CnN5bmMgbWVtb3J5IChlcnJtc2c9c3RyLHN0YXQ9 bikKaWYgKG4gLz0gMCkgY2FsbCBhYm9ydCgpCgoKIQohIFRlc3QgU1lOQyBJ TUFHRVMKIQpzeW5jIGltYWdlcyAoKikKaWYgKHRoaXNfaW1hZ2UoKSA9PSAx KSB0aGVuCiAgICBzeW5jIGltYWdlcyAoMSkKICAgIHN5bmMgaW1hZ2VzICgx LCBlcnJtc2c9c3RyKQogICAgc3luYyBpbWFnZXMgKFsxXSkKZW5kIGlmCgpu ID0gNQpzeW5jIGltYWdlcyAoKiwgc3RhdD1uKQppZiAobiAvPSAwKSBjYWxs IGFib3J0KCkKCm4gPSA1CnN5bmMgaW1hZ2VzICgqLGVycm1zZz1zdHIsc3Rh dD1uKQppZiAobiAvPSAwKSBjYWxsIGFib3J0KCkKCmVuZA== --------------060601060303070400020900--