Re: [PATCH][LTO] (Not) merge incompatible common decls

[Richard Guenther <rguenther at suse dot de>]

On Thu, 30 Jul 2009, Richard Guenther wrote:

> 
> This patch adds the support to keep track of multiple decls per
> symtab where we can merge duplicates into.  Thus for non-compatible
> decls we keep them separate but only emit the largest one into
> the final assembler file.
> 
> This fixes a bunch of issues in SPEC 2000 (one extra completely
> fixed benchmark and one where we now only run into type verification
> issues) and in SPEC 2006 (the compile is still running here, but
> 5 benchmarks showed the typical error pattern before).
> 
> The gfortran testsuite improves in that we now successfully link
> more of the C - Fortran mixed-language cases (but still fail there
> due to the extra warning we emit that is unexpected) and that
> at -O3 we miscompile the thing (because we do not merge int i; and
> struct { int i; })
> 
> I believe emulating linker behavior is a very good start to tackle
> all the problems (and if you could paste the C and Fortran source
> into one CU we would have the miscompile issue in the present state
> as well ...).  We can try to fixup common and sensible cases
> case by case afterwards.
> 
> Bootstrapped and tested on x86_64-unknown-linux-gnu, ok for LTO?
> 
> -# of expected passes           51698
> -# of unexpected failures       190
> +# of expected passes           51734
> +# of unexpected failures       196
> 
> (6 extra execute fails, a load less WARNINGs due to link failures,
> but those are not counted, the execute fails previously were
> link fails)
> 
> I'm not good at LTO testsuite magic - I failed to scan the assembler
> output for the correct sized .comm decl.  If anyone can give hints
> there that would be appreciated ;)

One small update - if we get extern int i; float i; int i; in that order
the following incremental fix is needed.  We have to properly do the
replacement of old_decl by new_decl, preserving the chain we have built up
sofar.

I'll re-boostrap / test with it.
Sofar the SPEC2006 compile results look promising.

Richard.


*************** lto_symtab_merge_decl (tree new_decl,
*** 602,611 ****
    if (resolution == LDPR_PREVAILING_DEF
        || resolution == LDPR_PREVAILING_DEF_IRONLY)
      {
        gcc_assert (old_resolution == LDPR_PREEMPTED_IR
                  || old_resolution ==  LDPR_RESOLVED_IR
                  || (old_resolution == resolution && !flag_no_common));
!       lto_symtab_set_identifier_decl (name, new_decl);
        return;
      }

--- 618,638 ----
    if (resolution == LDPR_PREVAILING_DEF
        || resolution == LDPR_PREVAILING_DEF_IRONLY)
      {
+       tree decl;
        gcc_assert (old_resolution == LDPR_PREEMPTED_IR
                  || old_resolution ==  LDPR_RESOLVED_IR
                  || (old_resolution == resolution && !flag_no_common));
!       TREE_CHAIN (new_decl) = TREE_CHAIN (old_decl);
!       TREE_CHAIN (old_decl) = NULL_TREE;
!       decl = lto_symtab_get_identifier_decl (name);
!       if (decl == old_decl)
!       {
!         lto_symtab_set_identifier_decl (name, new_decl);
!         return;
!       }
!       while (TREE_CHAIN (decl) != old_decl)
!       decl = TREE_CHAIN (decl);
!       TREE_CHAIN (decl) = new_decl;
        return;
      }