Re: PR fortran/51727: make module files reproducible, question on C++ in gcc

[Janne Blomqvist <blomqvist dot janne at gmail dot com>]

On Sat, Oct 13, 2012 at 4:26 PM, Tobias SchlÃter
<tobias.schlueter@physik.uni-muenchen.de> wrote:
>
> Hi,
>
> first a question also to non-gfortraners: if I want to use std::map, where
> do I "#include <map>"?  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 need
> 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 build
> 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 <map> (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 problem
> 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 code!

- 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<void*, pointer_info*> pmap; // Or could be std::unordered_map
if available
std::map<int, pointer_info*> 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.


-- 
Janne Blomqvist