From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 84764 invoked by alias); 10 Mar 2015 15:30:02 -0000 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 Received: (qmail 84724 invoked by uid 89); 10 Mar 2015 15:30:01 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=0.6 required=5.0 tests=AWL,BAYES_00,FREEMAIL_FROM,KAM_FROM_URIBL_PCCC,RCVD_IN_DNSWL_LOW,SPF_PASS autolearn=no version=3.3.2 X-Spam-User: qpsmtpd, 2 recipients X-HELO: mail-lb0-f170.google.com Received: from mail-lb0-f170.google.com (HELO mail-lb0-f170.google.com) (209.85.217.170) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-GCM-SHA256 encrypted) ESMTPS; Tue, 10 Mar 2015 15:30:00 +0000 Received: by lbiz12 with SMTP id z12so2561095lbi.5; Tue, 10 Mar 2015 08:29:57 -0700 (PDT) MIME-Version: 1.0 X-Received: by 10.112.211.200 with SMTP id ne8mr30476536lbc.73.1426001397104; Tue, 10 Mar 2015 08:29:57 -0700 (PDT) Received: by 10.112.189.70 with HTTP; Tue, 10 Mar 2015 08:29:57 -0700 (PDT) In-Reply-To: <54FE9B30.3010004@net-b.de> References: <54F9FDEE.6090002@net-b.de> <54FE9B30.3010004@net-b.de> Date: Tue, 10 Mar 2015 15:30:00 -0000 Message-ID: Subject: Re: [PATCH, Fortran] Sync memory action delegated to OpenCoarrays From: Alessandro Fanfarillo To: Tobias Burnus Cc: gfortran , gcc-patches Content-Type: text/plain; charset=UTF-8 X-IsSubscribed: yes X-SW-Source: 2015-03/txt/msg00569.txt.bz2 Totally fine with me. Thanks. 2015-03-10 0:20 GMT-07:00 Tobias Burnus : > Hi Alessandro, > > Alessandro Fanfarillo wrote: >> >> Thanks for the quick response. Patch built and regtested on >> x86_64-unknown-linux-gnu. > > > Actually, I also was about to send a reworked patch myself - see attachment. > Additional changes compared to your last patch: > * Add the assembler statement to libcaf_single also for the other syncs. > * Tested also the passing of the other arguments in the test case. > * Removed the call to sync_synchronize in the compiler - leaving it > completely to the library. (Existed for end of program, STOP and SYNC ...; > was only issued with -fcoarray=lib) > > Are the changes okay from your side? Then I'd prefer my patch - and I'll > commit it this evening. > > * * * > >> Currently the stub for sync memory in single.c invokes "asm volatile >> ("" : : : "memory")" but _gfortran_caf_sync_memory is not called when >> the code is compiled with -fcoarray=single. > > > I misremembered that we do call __sync_synchronize for -fcoarray=single. As > it turned out, we only call it for -fcoarray=lib - and it makes sense (in my > opinion) to leave such calls to the library. > > As you, I also added the "asm" to the library - which probably has no effect > - except when the library and the program are both compiled with LTO. Still, > it somehow looks more complete. > > > Regarding: Looks good to me - except that I prefer the additional items of > my patch - and that the ChangeLog doesn't conform to the GCC style: > - The file names shall be relative to the change log, i.e. "caf/single.c" > (as the ChangeLog file is in libgfortran/ChangeLog), similarly a > "gfortran.dg/" is missing for the test case. > - There should be a single "* " before the file names, not two tabs and > no "* ". > - After the file name, the changed symbols/function names shall be put in > parentheses. > - The item describing the change should end with a full stop. > - The email address should be enclosed in <>. > (Often missed, but not by you: date-name-email separation is two spaces.) > > Tobias > > >> Please let me know if I have to change the patch for -fcoarray=single. >> >> >> >> 2015-03-06 11:20 GMT-08:00 Tobias Burnus : >>> >>> Dear Alessandro, >>> >>> Alessandro Fanfarillo wrote: >>> >>> so far a "sync memory" statement is translated into a local >>> "__sync_synchronize ()". >>> The attached draft patch delegates the action for sync memory (when >>> -fcoarray=lib is used) to the external function >>> _gfortran_caf_sync_memory() implemented in the OpenCoarrays library. >>> >>> >>> Looks good to me. However, you should add a test case with 'dg-options >>> "-fdump-tree-original -fcoarray=lib"' to check that this works; cf. >>> gcc/testsuite/gfortran.dg/coarray*.f90 for examples. >>> >>> And you have to provide a stub implementation in >>> libgfortran/caf/{libcaf.h,single.c}. >>> >>> Tobias >>> >>> PS: I wonder whether it makes sense to remove the __sync_synchronize for >>> -fcoarray=single and replace it by the equivalent to "asm volatile ("" : >>> : : >>> "memory")". It almost certainly does. > >