From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 19855 invoked by alias); 14 Oct 2012 21:35:35 -0000 Received: (qmail 19750 invoked by uid 22791); 14 Oct 2012 21:35:35 -0000 X-SWARE-Spam-Status: No, hits=-5.0 required=5.0 tests=AWL,BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,FREEMAIL_FROM,KHOP_RCVD_TRUST,KHOP_THREADED,RCVD_IN_DNSWL_LOW,RCVD_IN_HOSTKARMA_YE X-Spam-Check-By: sourceware.org Received: from mail-la0-f47.google.com (HELO mail-la0-f47.google.com) (209.85.215.47) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Sun, 14 Oct 2012 21:35:29 +0000 Received: by mail-la0-f47.google.com with SMTP id h5so3064419lam.20 for ; Sun, 14 Oct 2012 14:35:27 -0700 (PDT) MIME-Version: 1.0 Received: by 10.152.146.101 with SMTP id tb5mr4983410lab.44.1350250527435; Sun, 14 Oct 2012 14:35:27 -0700 (PDT) Received: by 10.152.1.102 with HTTP; Sun, 14 Oct 2012 14:35:27 -0700 (PDT) In-Reply-To: <50796BF5.4060100@physik.uni-muenchen.de> References: <50796BF5.4060100@physik.uni-muenchen.de> Date: Sun, 14 Oct 2012 22:37:00 -0000 Message-ID: Subject: Re: PR fortran/51727: make module files reproducible, question on C++ in gcc From: Janne Blomqvist To: =?UTF-8?Q?Tobias_Schl=C3=BCter?= Cc: Fortran List , gcc-patches Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable 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: 2012-10/txt/msg01365.txt.bz2 On Sat, Oct 13, 2012 at 4:26 PM, Tobias Schl=C3=BCter wrote: > > Hi, > > first a question also to non-gfortraners: if I want to use std::map, where > do I "#include "? In system.h? > > Now to the patch-specific part: in this PR, module files are produced with > random changes because the order in which symbols are written can depend = on > the memory layout. This patch fixes this by recording which symbols need= to > be written and then processing them in order. The patch doesn't make the > more involved effort of putting all symbols into the module in an easily > predicted order, instead it only makes sure that the order remains fixed > across the compiler invocations. The reason why the former is difficult = is > that during the process of writing a symbol, it can turn out that other > symbols will have to be written as well (say, because they appear in array > specifications). Since the module-writing code determines which symbols = to > output while actually writing the file, recording all the symbols that ne= ed > to be written before writing to the file would mean a lot of surgery. > > I'm putting forward two patches. One uses a C++ map to very concisely bu= ild > up and handle the ordered list of symbols. This has three problems: > 1) gfortran maintainers may not want C++isms (even though in this case > it's very localized, and in my opinion very transparent), and > 2) it can't be backported to old release branches which are still > compiled as C. Joost expressed interested in a backport. > 3) I don't know where to #include (see above) > Therefore I also propose a patch where I added the necessary ~50 lines of > boilerplate code and added the necessary traversal function to use > gfortran's GFC_BBT to maintain the ordered tree of symbols. > > Both patches pass the testsuite and Joost confirms that they fix the prob= lem > with CP2K. I also verified with a few examples that they both produce > identical .mod files as they should. > > Is the C++ patch, modified to do the #include correctly, ok for the trunk? > If not, the C-only patch? Can I put the C-only patch on the release > branches? And which? Hi, I'm pleasantly surprised that you managed to fix this PR with so little cod= e! - Personally, I'd prefer the C++ version; The C++ standard library is widely used and documented and using it in favour of rolling our own is IMHO a good idea. - I'd be vary wrt backporting, in my experience the module.c code is somewhat fragile and easily causes regressions. In any case, AFAICS PR 51727 is not a regression. - I think one could go a step further and get rid of the BBT stuff in pointer_info, replacing it with two file-level maps std::map pmap; // Or could be std::unordered_map if available std::map imap; So when writing a module, use pmap similar to how pointer_info BBT is used now, and then use imap to get a consistent order per your patch. When reading, lookup/create mostly via imap, creating a pmap entry also when creating a new imap entry; this avoids having to do a brute-force search when looking up via pointer when reading (see find_pointer2()). (This 3rd point is mostly an idea for further work, and is not meant as a requirement for accepting the patch) Ok for trunk, although wait for a few days in case there is a storm of protest on the C vs. C++ issue from other gfortran maintainers. --=20 Janne Blomqvist