[Bug ipa/65028] New: [5 Regression] 450.soplex in SPEC CPU 2006 is miscompiled

[Bug ipa/65028] New: [5 Regression] 450.soplex in SPEC CPU 2006 is miscompiled

[hjl.tools at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65028

            Bug ID: 65028
           Summary: [5 Regression] 450.soplex in SPEC CPU 2006 is
                    miscompiled
           Product: gcc
           Version: 5.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: ipa
          Assignee: unassigned at gcc dot gnu.org
          Reporter: hjl.tools at gmail dot com
                CC: hubicka at ucw dot cz

On Linux/x86-64, r220521 miscompiled 450.soplex in SPEC CPU 2006:

g++  -O3 -funroll-loops -ffast-math -fwhole-program -flto=jobserver
-fuse-linker-plugin  -DSPEC_CPU_LP64        changesoplex.o didxset.o dsvector.o
dvector.o enter.o example.o factor.o forest.o idxset.o leave.o lpcolset.o
lprow.o lprowset.o message.o mpsinput.o nameset.o slufactor.o solve.o soplex.o
spxaggregatesm.o spxbasis.o spxbounds.o spxchangebasis.o spxdefaultpr.o
spxdefaultrt.o spxdefines.o spxdesc.o spxdevexpr.o spxequilisc.o spxfastrt.o
spxgeneralsm.o spxharrisrt.o spxhybridpr.o spxid.o spxio.o spxlp.o spxlpfread.o
spxmpsread.o spxmpswrite.o spxparmultpr.o spxquality.o spxredundantsm.o
spxrem1sm.o spxscaler.o spxshift.o spxsolve.o spxsolver.o spxstarter.o
spxsteeppr.o spxsumst.o spxvecs.o spxvectorst.o spxweightpr.o spxweightst.o
ssvector.o svector.o svset.o timer.o unitvector.o update.o updatevector.o
vector.o vsolve.o                     -o soplex
...
 Running 450.soplex ref peak lto default

450.soplex: copy 0 non-zero return code (exit code=11, signal=0)

[Bug ipa/65028] [5 Regression] 450.soplex in SPEC CPU 2006 is miscompiled

[hubicka at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65028

Jan Hubicka <hubicka at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |ASSIGNED
   Last reconfirmed|                            |2015-02-11
                 CC|                            |hubicka at gcc dot gnu.org
           Assignee|unassigned at gcc dot gnu.org      |hubicka at gcc dot gnu.org
     Ever confirmed|0                           |1

--- Comment #1 from Jan Hubicka <hubicka at gcc dot gnu.org> ---
thanks for checking the eon. I will take a look at soplex.

[Bug ipa/65028] [5 Regression] 450.soplex in SPEC CPU 2006 is miscompiled

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65028

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |wrong-code
           Priority|P3                          |P1
   Target Milestone|---                         |5.0

[Bug ipa/65028] [5 Regression] 450.soplex in SPEC CPU 2006 is miscompiled

[jamborm at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65028

Martin Jambor <jamborm at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|mjambor at suse dot cz             |jamborm at gcc dot gnu.org
           Assignee|hubicka at gcc dot gnu.org         |jamborm at gcc dot gnu.org

--- Comment #3 from Martin Jambor <jamborm at gcc dot gnu.org> ---
I've managed to reproduce the issue, I will have a look.

[Bug ipa/65028] [5 Regression] 450.soplex in SPEC CPU 2006 is miscompiled

[jamborm at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65028

--- Comment #4 from Martin Jambor <jamborm at gcc dot gnu.org> ---
OK, so here are my findings.  Switching off IPA-CP helps because the
pass then does not propagate polymorphic context from
_ZN8MySoPlexC2EN6soplex6SoPlex4TypeENS1_14RepresentationE/5887 to
_ZN6soplex9SPxSolverC2ENS_6SoPlex4TypeENS1_14RepresentationE/51162
(both are constructors) for the 0th parameter.  This propagation
clears the dynamic flag and speculation from the context on the edge
and the result is stored to a lattice describing the callee.

Later on, inlining comes along and calls ipa_context_from_jfunc which
picks up this value from the lattice and uses it as a base for a jump
function on the edge being inlined.  In this process, it also checks
for dynamic type changes in constructors and destructors but
apparently it relies on edge flag in_polymorphic_cdtor being correct.

And that is the problem, apparently it is not.  When we are inlining
into a function that has itself already been inlined into the
constructor, the flag is false, although we are basing jump function
calculations on lattices corresponding to the constructor.

Therefore I'd suggest the following fix.  (Alternatively we could
easily do this propagation in update_jump_functions_after_inlining,
without introducing another recursion.)


2015-02-13  Martin Jambor  <mjambor@suse.cz>

    PR ipa/65028
    * ipa-inline-transform.c (mark_all_inlined_calls_cdtor): New function.
    (inline_call): Use it.

diff --git a/gcc/ipa-inline-transform.c b/gcc/ipa-inline-transform.c
index 235219d..61229ac 100644
--- a/gcc/ipa-inline-transform.c
+++ b/gcc/ipa-inline-transform.c
@@ -261,6 +261,21 @@ clone_inlined_nodes (struct cgraph_edge *e, bool
duplicate,
     }
 }

+/* Mark all call graph edges coming out of NODE and all nodes that have been
+   inlined to it as in_polymorphic_cdtor.  */
+
+static void
+mark_all_inlined_calls_cdtor (cgraph_node *node)
+{
+  for (cgraph_edge *cs = node->callees; cs; cs = cs->next_callee)
+    {
+      cs->in_polymorphic_cdtor = true;
+      if (!cs->inline_failed)
+    mark_all_inlined_calls_cdtor (cs->callee);
+    }
+  for (cgraph_edge *cs = node->indirect_calls; cs; cs = cs->next_callee)
+    cs->in_polymorphic_cdtor = true;
+}

 /* Mark edge E as inlined and update callgraph accordingly.  UPDATE_ORIGINAL
    specify whether profile of original function should be updated.  If any new
@@ -332,6 +347,8 @@ inline_call (struct cgraph_edge *e, bool update_original,

   old_size = inline_summaries->get (to)->size;
   inline_merge_summary (e);
+  if (e->in_polymorphic_cdtor)
+    mark_all_inlined_calls_cdtor (e->callee);
   if (opt_for_fn (e->caller->decl, optimize))
     new_edges_found = ipa_propagate_indirect_call_infos (curr, new_edges);
   if (update_overall_summary)

[Bug ipa/65028] [5 Regression] 450.soplex in SPEC CPU 2006 is miscompiled

[hubicka at ucw dot cz]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65028

--- Comment #5 from Jan Hubicka <hubicka at ucw dot cz> ---
> 
> 2015-02-13  Martin Jambor  <mjambor@suse.cz>
> 
>     PR ipa/65028
>     * ipa-inline-transform.c (mark_all_inlined_calls_cdtor): New function.
>     (inline_call): Use it.

Oops, that is quite an oversight at my side. The patch is OK

Honza

[Bug ipa/65028] [5 Regression] 450.soplex in SPEC CPU 2006 is miscompiled

[hubicka at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65028

--- Comment #6 from Jan Hubicka <hubicka at gcc dot gnu.org> ---
Author: hubicka
Date: Fri Feb 13 20:04:32 2015
New Revision: 220693

URL: https://gcc.gnu.org/viewcvs?rev=220693&root=gcc&view=rev
Log:

    PR ipa/65028
    * ipa-inline-transform.c (mark_all_inlined_calls_cdtor): New function.
    (inline_call): Use it.

Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/ipa-inline-transform.c

[Bug ipa/65028] [5 Regression] 450.soplex in SPEC CPU 2006 is miscompiled

[hubicka at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65028

--- Comment #7 from Jan Hubicka <hubicka at gcc dot gnu.org> ---
Author: hubicka
Date: Fri Feb 13 20:05:39 2015
New Revision: 220694

URL: https://gcc.gnu.org/viewcvs?rev=220694&root=gcc&view=rev
Log:
    PR ipa/65028
    * ipa-prop.c (update_indirect_edges_after_inlining): Do not drop
    polymorphic call info when type is not known to be preserved.

Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/ipa-prop.c

[Bug ipa/65028] [5 Regression] 450.soplex in SPEC CPU 2006 is miscompiled

[hubicka at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65028

Jan Hubicka <hubicka at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
         Resolution|---                         |FIXED

--- Comment #8 from Jan Hubicka <hubicka at gcc dot gnu.org> ---
Fixed.

[Bug ipa/65028] [5 Regression] 450.soplex in SPEC CPU 2006 is miscompiled

[jamborm at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65028

--- Comment #10 from Martin Jambor <jamborm at gcc dot gnu.org> ---
(In reply to H.J. Lu from comment #9)
> The same bug affects 252.eon in SPEC CPU 2000 on x32:

...

> 
> The fix isn't sufficient since adding -fno-ipa-cp fixes eon on x32.

I really doubt that it is "the same bug."  Just that -fno-ipa-cp
avoids it certainly does not suffice to make that conclusion.

Anyway, I will try to find some time to look soon, although setting up
x32 environment will take me some time.  I suppose this bug also
manifests itself with -O3 and LTO?

[Bug ipa/65028] [5 Regression] 450.soplex in SPEC CPU 2006 is miscompiled

[hjl.tools at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65028

--- Comment #11 from H.J. Lu <hjl.tools at gmail dot com> ---
(In reply to Martin Jambor from comment #10)
> (In reply to H.J. Lu from comment #9)
> > The same bug affects 252.eon in SPEC CPU 2000 on x32:
> 
> ...
> 
> > 
> > The fix isn't sufficient since adding -fno-ipa-cp fixes eon on x32.
> 
> I really doubt that it is "the same bug."  Just that -fno-ipa-cp
> avoids it certainly does not suffice to make that conclusion.

The symptom is very similar, that is eon died very early.

> Anyway, I will try to find some time to look soon, although setting up
> x32 environment will take me some time.  I suppose this bug also
> manifests itself with -O3 and LTO?

It ONLY shows up with -O3 -funroll-loops -ffast-math -flto

[Bug ipa/65028] [5 Regression] 450.soplex in SPEC CPU 2006 is miscompiled

[hubicka at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65028

--- Comment #12 from Jan Hubicka <hubicka at gcc dot gnu.org> ---
I agree it is probably differnt issue, but lets see. It would help if you can
look at the backtract and see if there is a problem between local/non-local
calling conventions.  I.e. function called with parameters passed on stack and
consuming them in registers.

[Bug ipa/65028] [5 Regression] 450.soplex in SPEC CPU 2006 is miscompiled

[hjl.tools at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65028

--- Comment #13 from H.J. Lu <hjl.tools at gmail dot com> ---
(In reply to Jan Hubicka from comment #12)
> I agree it is probably differnt issue, but lets see. It would help if you
> can look at the backtract and see if there is a problem between
> local/non-local calling conventions.  I.e. function called with parameters
> passed on stack and consuming them in registers.

X32 passes parameters in registers, just like x86-64.  I got

(gdb) disass
Dump of assembler code for function operator*(ggSpectrum const&, ggSpectrum
const&):
   0x00420780 <+0>:    movups (%esi),%xmm0
   0x00420784 <+4>:    mov    %rdi,%rax
   0x00420787 <+7>:    movups 0x10(%esi),%xmm1
=> 0x0042078c <+12>:    mulps  (%edx),%xmm0
                                ^^^^^ It isn't aligned to 32 bytes.
   0x00420790 <+16>:    mulps  0x10(%edx),%xmm1
   0x00420795 <+21>:    movups %xmm0,(%edi)
   0x00420799 <+25>:    movups %xmm1,0x10(%edi)
   0x0042079e <+30>:    retq   
End of assembler dump.
(gdb) p/x $edx
$2 = 0xffffcc48
(gdb) f 1
#1  0x004452d4 in __base_ctor  (v=..., z=..., y=..., x=..., bPrimary=..., 
    gPrimary=..., rPrimary=..., this=0xffffbf60) at ./ggPhotometer.h:53
53                      colorMatrix[0][0] = (cieX * rPrimary).area();
(gdb) p cieX
$3 = {data = {0.146008939, 0.279831082, 0.0505069345, 0.202513069, 
    0.729846954, 0.957131386, 0.414069265, 0.0614523999}}
(gdb) p rPrimary
$4 = (const struct ggSpectrum &) @0xffffcc48: {data = {0, 0, 0, 0, 
    0.0899555609, 0.550044477, 0.959999979, 1}}
(gdb) ...
   0x004452c4 <+1620>:    mov    (%esp),%rdx
   0x004452c9 <+1625>:    mov    %r15,%rsi
   0x004452cc <+1628>:    mov    %r14,%rdi
   0x004452cf <+1631>:    callq  0x420780 <operator*(ggSpectrum const&,
ggSpectrum const&)>
=> 0x004452d4 <+1636>:    mov    %r14,%rdi
...
Breakpoint 1, 0x004452cf in __base_ctor  (v=..., z=..., y=..., x=..., 
    bPrimary=..., gPrimary=..., rPrimary=..., this=0xffffbf60)
    at ./ggPhotometer.h:53
53                      colorMatrix[0][0] = (cieX * rPrimary).area();
(gdb) p/x $rdi
$6 = 0xffffbf40
(gdb) p/x $rsi
$7 = 0xffffbfa8
(gdb) p/x $rdx
$8 = 0xffffcc48
(gdb) p this
$9 = (struct ggPhotometer * const) 0xffffbf60
(gdb) p &cieX
$10 = (struct ggSpectrum *) 0xffffbfa8
(gdb) p/x &rPrimary
$11 = 0xffffcc48
(gdb) p/x $pc
$13 = 0x4452cf
(gdb) p rPrimary
$1 = (const struct ggSpectrum &) @0xffffcc48: {data = {0, 0, 0, 0, 
    0.0899555609, 0.550044477, 0.959999979, 1}}
(gdb) 

Why do we think rPrimary is 16-byte aligned when it is only 8-byte aligned?

[Bug ipa/65028] [5 Regression] 450.soplex in SPEC CPU 2006 is miscompiled

[hubicka at ucw dot cz]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65028

--- Comment #14 from Jan Hubicka <hubicka at ucw dot cz> ---
Thanks for looking into that!
> 
> Why do we think rPrimary is 16-byte aligned when it is only 8-byte aligned?

ipa-cp newly does alignment propagation.  You may try to just disable it
by short cirucuiting ipcp_store_alignment_results
(also you ought to be able to find the wrong alignment upgrade in the ipa-cp
dumps)

Honza

[Bug ipa/65028] [5 Regression] 450.soplex in SPEC CPU 2006 is miscompiled

[hjl.tools at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65028

--- Comment #15 from H.J. Lu <hjl.tools at gmail dot com> ---
(In reply to Jan Hubicka from comment #14)
> Thanks for looking into that!
> > 
> > Why do we think rPrimary is 16-byte aligned when it is only 8-byte aligned?
> 
> ipa-cp newly does alignment propagation.  You may try to just disable it
> by short cirucuiting ipcp_store_alignment_results

Disable ipcp_store_alignment_results fixes the bug.

> (also you ought to be able to find the wrong alignment upgrade in the ipa-cp
> dumps)

I will do it next.

[Bug ipa/65028] [5 Regression] 450.soplex in SPEC CPU 2006 is miscompiled

[hjl.tools at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65028

--- Comment #16 from H.J. Lu <hjl.tools at gmail dot com> ---
Created attachment 34791
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=34791&action=edit
A patch to add -fipa-cp-alignment

[Bug ipa/65028] [5 Regression] 450.soplex in SPEC CPU 2006 is miscompiled

[hjl.tools at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65028

H.J. Lu <hjl.tools at gmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #34791|0                           |1
        is obsolete|                            |

--- Comment #17 from H.J. Lu <hjl.tools at gmail dot com> ---
Created attachment 34792
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=34792&action=edit
A corrected patch to add -fipa-cp-alignment

[Bug ipa/65028] [5 Regression] 450.soplex in SPEC CPU 2006 is miscompiled

[hjl.tools at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65028

--- Comment #18 from H.J. Lu <hjl.tools at gmail dot com> ---
There are

;; Function operator* (_ZmlRK10ggSpectrumS1_, funcdef_no=48, decl_uid=4292,
cgraph_uid=5, symbol_order=46520)

Modification phase of node operator*/46520
  Adjusting alignment of param 1 to 16, misalignment to 0

[Bug ipa/65028] [5 Regression] 450.soplex in SPEC CPU 2006 is miscompiled

[jamborm at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65028

--- Comment #19 from Martin Jambor <jamborm at gcc dot gnu.org> ---
HJ, how do I configure build an x32 gcc?  I have working x32 debian
chroot environment but building gcc fails with:

checking for int64_t underlying type... long long
configure: error: error verifying int64_t uses long long
Makefile:4053: recipe for target 'configure-gcc' failed

is there a howto somewhere?  Thanks.

[Bug ipa/65028] [5 Regression] 450.soplex in SPEC CPU 2006 is miscompiled

[hubicka at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65028

--- Comment #20 from Jan Hubicka <hubicka at gcc dot gnu.org> ---
H.J. Can you, please attach the full .cp dump file and release_ssa dump of the
corresponding functions?
The -fipa-cp-alignment patch looks like a resonable idea.  These days you want
to check opt_for_fn (decl, flag_ipa_cp_alignment). Ok with that change and
documentation update.

Honza

[Bug ipa/65028] [5 Regression] 450.soplex in SPEC CPU 2006 is miscompiled

[hjl.tools at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65028

--- Comment #21 from H.J. Lu <hjl.tools at gmail dot com> ---
(In reply to Martin Jambor from comment #19)
> HJ, how do I configure build an x32 gcc?  I have working x32 debian
> chroot environment but building gcc fails with:

I suggest you use Ubuntu 14.04 or newer.

> checking for int64_t underlying type... long long
> configure: error: error verifying int64_t uses long long
> Makefile:4053: recipe for target 'configure-gcc' failed

What does gcc/config.log say?

> is there a howto somewhere?  Thanks.

You just need to install gcc x32 as well as libc x32 packages.

[Bug ipa/65028] [5 Regression] 450.soplex in SPEC CPU 2006 is miscompiled

[hjl.tools at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65028

--- Comment #22 from H.J. Lu <hjl.tools at gmail dot com> ---
Created attachment 34793
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=34793&action=edit
Dumps from -fdump-ipa-cp-details

[Bug ipa/65028] [5 Regression] 450.soplex in SPEC CPU 2006 is miscompiled

[hjl.tools at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65028

--- Comment #23 from H.J. Lu <hjl.tools at gmail dot com> ---
(In reply to Jan Hubicka from comment #20)
> The -fipa-cp-alignment patch looks like a resonable idea.  These days you
> want to check opt_for_fn (decl, flag_ipa_cp_alignment). Ok with that change
> and documentation update.
> 

A patch is posted at:

https://gcc.gnu.org/ml/gcc-patches/2015-02/msg01043.html

[Bug ipa/65028] [5 Regression] 450.soplex in SPEC CPU 2006 is miscompiled

[hjl.tools at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65028

--- Comment #24 from H.J. Lu <hjl.tools at gmail dot com> ---
(In reply to H.J. Lu from comment #18)
> There are
> 
> ;; Function operator* (_ZmlRK10ggSpectrumS1_, funcdef_no=48, decl_uid=4292,
> cgraph_uid=5, symbol_order=46520)
> 
> Modification phase of node operator*/46520
>   Adjusting alignment of param 1 to 16, misalignment to 0

IPA-CP missed _ZN18eonImageCalculatorC1Ev which calls
_ZmlRK10ggSpectrumS1_ with 8-byte aligned rPrimary.

[Bug ipa/65028] [5 Regression] 450.soplex in SPEC CPU 2006 is miscompiled

[hjl.tools at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65028

--- Comment #25 from H.J. Lu <hjl.tools at gmail dot com> ---
IPA dump uses node->name () instead of node->asm_name ().
node->name () is kind of useless here.

[Bug ipa/65028] [5 Regression] 450.soplex in SPEC CPU 2006 is miscompiled

[hjl.tools at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65028

--- Comment #26 from H.J. Lu <hjl.tools at gmail dot com> ---
(In reply to H.J. Lu from comment #24)
> (In reply to H.J. Lu from comment #18)
> > There are
> > 
> > ;; Function operator* (_ZmlRK10ggSpectrumS1_, funcdef_no=48, decl_uid=4292,
> > cgraph_uid=5, symbol_order=46520)
> > 
> > Modification phase of node operator*/46520
> >   Adjusting alignment of param 1 to 16, misalignment to 0
> 
> IPA-CP missed _ZN18eonImageCalculatorC1Ev which calls
> _ZmlRK10ggSpectrumS1_ with 8-byte aligned rPrimary.

We are using a clone:

Propagated alignment info for function _ZN18eonImageCalculatorC2Ev/50390:
  param 0: align: 16, misalign: 0

[Bug ipa/65028] [5 Regression] 450.soplex in SPEC CPU 2006 is miscompiled

[hjl.tools at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65028

--- Comment #27 from H.J. Lu <hjl.tools at gmail dot com> ---
Created attachment 34794
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=34794&action=edit
A patch

We should check all aliases for parameter alignments.

[Bug ipa/65028] [5 Regression] 450.soplex in SPEC CPU 2006 is miscompiled

[hjl.tools at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65028

--- Comment #28 from H.J. Lu <hjl.tools at gmail dot com> ---
(In reply to H.J. Lu from comment #26)
> (In reply to H.J. Lu from comment #24)
> > (In reply to H.J. Lu from comment #18)
> > > There are
> > > 
> > > ;; Function operator* (_ZmlRK10ggSpectrumS1_, funcdef_no=48, decl_uid=4292,
> > > cgraph_uid=5, symbol_order=46520)
> > > 
> > > Modification phase of node operator*/46520
> > >   Adjusting alignment of param 1 to 16, misalignment to 0
> > 
> > IPA-CP missed _ZN18eonImageCalculatorC1Ev which calls
> > _ZmlRK10ggSpectrumS1_ with 8-byte aligned rPrimary.
> 
> We are using a clone:
> 
> Propagated alignment info for function _ZN18eonImageCalculatorC2Ev/50390:
>   param 0: align: 16, misalign: 0

I uploaded a patch to check all aliases for parameter alignments.  It
fixes eon on x32.

[Bug ipa/65028] [5 Regression] 450.soplex in SPEC CPU 2006 is miscompiled

[hjl.tools at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65028

--- Comment #29 from H.J. Lu <hjl.tools at gmail dot com> ---
Created attachment 34797
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=34797&action=edit
A reorg patch

This patch should be a no-op.  But it changes parameter
alignments for _ZmlRK10ggSpectrumS1_.  It looks like that
the behavior of -fipa-cp-alignment also depends on the
compiler sources.  There is a bug somewhere in
-fipa-cp-alignment implementation.

[Bug ipa/65028] [5 Regression] 450.soplex in SPEC CPU 2006 is miscompiled

[hubicka at ucw dot cz]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65028

--- Comment #30 from Jan Hubicka <hubicka at ucw dot cz> ---
> 
> I uploaded a patch to check all aliases for parameter alignments.  It
> fixes eon on x32.

Thanks for looking into this.  The bug is however a bit earlier. The
propagation
stage should never put anything into lattices of aliases (we probably should
add
checking assert that those are never used) and instead of that always go across
the alias into the final symbol.

I guess somewhere there is missing call to ultimate_alias_target (or
function_symbol)

Honza

[Bug ipa/65028] [5 Regression] 450.soplex in SPEC CPU 2006 is miscompiled

[hjl.tools at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65028

H.J. Lu <hjl.tools at gmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|REOPENED                    |NEW

--- Comment #31 from H.J. Lu <hjl.tools at gmail dot com> ---
If I disable ipa_read_jump_function in:

  for (e = node->callees; e; e = e->next_callee)
    {
      struct ipa_edge_args *args = IPA_EDGE_REF (e);
      int count = streamer_read_uhwi (ib);
      bool contexts_computed = count & 1;
      count /= 2;

      if (!count)
        continue;
      vec_safe_grow_cleared (args->jump_functions, count);
      if (contexts_computed)
        vec_safe_grow_cleared (args->polymorphic_call_contexts, count);

      for (k = 0; k < ipa_get_cs_argument_count (args); k++)
        {
          ipa_read_jump_function (ib, ipa_get_ith_jump_func (args, k), e,
                                  data_in);
          if (contexts_computed)
            ipa_get_ith_polymorhic_call_context (args, k)->stream_in (ib,
data_in);
        }
    }

it works.

[Bug ipa/65028] [5 Regression] 450.soplex in SPEC CPU 2006 is miscompiled

[hjl.tools at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65028

--- Comment #32 from H.J. Lu <hjl.tools at gmail dot com> ---
_ZN18eonImageCalculatorC2Ev has an alias, _ZN18eonImageCalculatorC1Ev.
It is only called once in main.  _ZN18eonImageCalculatorC1Ev calls
_ZmlRK10ggSpectrumS1_.  But _ZN18eonImageCalculatorC1Ev is ignored
by ipa_read_node_info.

[Bug ipa/65028] [5 Regression] 450.soplex in SPEC CPU 2006 is miscompiled

[hjl.tools at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65028

--- Comment #33 from H.J. Lu <hjl.tools at gmail dot com> ---
There are many calls to _ZmlRK10ggSpectrumS1_ in LTO IR. Some calls have
parameters with unknown alignment.  But they are ignored by IPA-CP, which
applies parameter alignment from calls with known parameter alignment.
When _ZmlRK10ggSpectrumS1_ is called from callers with 8-byte parameter
alignment, it crashes.

[Bug ipa/65028] [5 Regression] 450.soplex in SPEC CPU 2006 is miscompiled

[hjl.tools at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65028

--- Comment #34 from H.J. Lu <hjl.tools at gmail dot com> ---
propagate_alignment_accross_jump_function seems wrong:

 if (cur.known)
    {    
      if (!dest_lat->alignment.known)
        {
          dest_lat->alignment = cur; 
          return true;
        }

We can't change argument alignment from unknown to know.
We can only reduce argument alignment.  Callee can only
assume the smallest argument alignment passed down from
all callers.

[Bug ipa/65028] [5 Regression] 450.soplex in SPEC CPU 2006 is miscompiled

[hubicka at ucw dot cz]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65028

--- Comment #35 from Jan Hubicka <hubicka at ucw dot cz> ---
> propagate_alignment_accross_jump_function seems wrong:
> 
>  if (cur.known)
>     {    
>       if (!dest_lat->alignment.known)
>         {
>           dest_lat->alignment = cur; 
>           return true;
>         }
> 
> We can't change argument alignment from unknown to know.
> We can only reduce argument alignment.  Callee can only
> assume the smallest argument alignment passed down from
> all callers.

Using top/bottom would be probably less confusing here, but looking
into print_all_lattices I think !known represent TOP and BOTTOM
is known & align == 0. So the code above seems OK.
I think it is missing code to actually merge the values.  Let me see if I can
repreoduce testcase.

Honza

[Bug ipa/65028] [5 Regression] 450.soplex in SPEC CPU 2006 is miscompiled

[hubicka at ucw dot cz]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65028

--- Comment #36 from Jan Hubicka <hubicka at ucw dot cz> ---
Hi,
I do not really see the reason for wrong code, but the merging logic seems
weird for me.  There is no merging done when we get two different alignments
and also we seem to immediately drop lattice to bottom when the incomming
parameter alignment has no .known bit set.  This means we do pesimistic
propagation.

I..e in testcase

void *b;
__attribute__ ((noinline))
static void a (void *a)
{
  memcpy (a,b,1000000);
}
static void aa (void *a) __attribute__ ((alias("a")));

tt(void)
{
  int i[10000];
  short c;

  aa(&c);
  a(i);
}

We end up with algnment unkonwn instead of a. (did not managed to reproduce
the wrong alignment here).  What about the following:
Index: ipa-cp.c
===================================================================
--- ipa-cp.c    (revision 220789)
+++ ipa-cp.c    (working copy)
@@ -1409,6 +1409,60 @@ propagate_context_accross_jump_function
   return ret;
 }

+/* Decrease alignment info DEST to be at most CUR.  */
+
+static bool
+decrease_alignment (ipa_alignment *dest, ipa_alignment cur)
+{
+  bool changed = false;
+
+  if (!cur.known)
+    return false;
+  if (!dest->known)
+    {
+      *dest = cur;
+      return true;
+    }
+  if (dest->align == cur.align
+      && dest->misalign == cur.misalign)
+    return false;
+
+  if (dest->align > cur.align)
+    {
+      dest->align = cur.align;
+      if (cur.align)
+    dest->misalign
+      = dest->misalign % cur.align;
+      changed = true;
+    }
+  if (dest->align && (dest->misalign != (cur.misalign % dest->align)))
+    {
+      int diff = abs (dest->misalign
+              - (cur.misalign % dest->align));
+      dest->align = MIN (dest->align, (unsigned)diff & - diff);
+      if (dest->align)
+    dest->misalign
+      = dest->misalign % dest->align;
+      changed = true;
+    }
+  return changed;
+}
+
+/* Increase alignment info DEST to be at least CUR.  */
+
+static bool
+increase_alignment (ipa_alignment *dest, ipa_alignment cur)
+{
+  if (!dest->known)
+    return false;
+  if (!cur.known || dest->align < cur.align)
+    {
+      *dest = cur;
+      return true;
+    }
+  return false;
+}
+
 /* Propagate alignments across jump function JFUNC that is associated with
    edge CS and update DEST_LAT accordingly.  */

@@ -1420,17 +1474,17 @@ propagate_alignment_accross_jump_functio
   if (alignment_bottom_p (dest_lat))
     return false;

-  ipa_alignment cur;
-  cur.known = false;
-  if (jfunc->alignment.known)
-    cur = jfunc->alignment;
-  else if (jfunc->type == IPA_JF_PASS_THROUGH
-       || jfunc->type == IPA_JF_ANCESTOR)
+  ipa_alignment cur = jfunc->alignment;
+
+  if (jfunc->type == IPA_JF_PASS_THROUGH
+      || jfunc->type == IPA_JF_ANCESTOR)
     {
       struct ipa_node_params *caller_info = IPA_NODE_REF (cs->caller);
       struct ipcp_param_lattices *src_lats;
       HOST_WIDE_INT offset = 0;
       int src_idx;
+      ipa_alignment incomming_cur;
+      bool ok = true;

       if (jfunc->type == IPA_JF_PASS_THROUGH)
     {
@@ -1440,10 +1494,10 @@ propagate_alignment_accross_jump_functio
           if (op != POINTER_PLUS_EXPR
           && op != PLUS_EXPR
           && op != MINUS_EXPR)
-        goto prop_fail;
+        ok = false;
           tree operand = ipa_get_jf_pass_through_operand (jfunc);
           if (!tree_fits_shwi_p (operand))
-        goto prop_fail;
+        ok = false;
           offset = tree_to_shwi (operand);
         }
       src_idx = ipa_get_jf_pass_through_formal_id (jfunc);
@@ -1454,30 +1508,19 @@ propagate_alignment_accross_jump_functio
       offset = ipa_get_jf_ancestor_offset (jfunc);
     }

-      src_lats = ipa_get_parm_lattices (caller_info, src_idx);
-      if (!src_lats->alignment.known
-      || alignment_bottom_p (src_lats))
-    goto prop_fail;
-
-      cur = src_lats->alignment;
-      cur.misalign = (cur.misalign + offset) % cur.align;
-    }
-
-  if (cur.known)
-    {
-      if (!dest_lat->alignment.known)
+      if (ok)
     {
-      dest_lat->alignment = cur;
-      return true;
+      src_lats = ipa_get_parm_lattices (caller_info, src_idx);
+
+      incomming_cur = src_lats->alignment;
+      if (incomming_cur.known && incomming_cur.align)
+        incomming_cur.misalign = (incomming_cur.misalign + offset)
+                     % incomming_cur.align;
+      increase_alignment (&cur, incomming_cur);
     }
-      else if (dest_lat->alignment.align == cur.align
-           && dest_lat->alignment.misalign == cur.misalign)
-    return false;
     }

- prop_fail:
-  set_alignment_to_bottom (dest_lat);
-  return true;
+  return decrease_alignment (&dest_lat->alignment, cur);
 }

 /* If DEST_PLATS already has aggregate items, check that aggs_by_ref matches

[Bug ipa/65028] [5 Regression] 450.soplex in SPEC CPU 2006 is miscompiled

[hjl.tools at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65028

--- Comment #37 from H.J. Lu <hjl.tools at gmail dot com> ---
(In reply to Jan Hubicka from comment #36)
> Hi,
> I do not really see the reason for wrong code, but the merging logic seems
> weird for me.  There is no merging done when we get two different alignments
> and also we seem to immediately drop lattice to bottom when the incomming
> parameter alignment has no .known bit set.  This means we do pesimistic
> propagation.
> 
> I..e in testcase
> 
> void *b;
> __attribute__ ((noinline))
> static void a (void *a)
> {
>   memcpy (a,b,1000000);
> }
> static void aa (void *a) __attribute__ ((alias("a")));
> 
> tt(void)
> {
>   int i[10000];
>   short c;
> 
>   aa(&c);
>   a(i);
> }
> 
> We end up with algnment unkonwn instead of a. (did not managed to reproduce
> the wrong alignment here).  What about the following:

Can you upload the patch as an attachment so that I can try it on x32? Thanks.

[Bug ipa/65028] [5 Regression] 450.soplex in SPEC CPU 2006 is miscompiled

[hubicka at ucw dot cz]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65028

--- Comment #40 from Jan Hubicka <hubicka at ucw dot cz> ---
Do you know why propagate_constants_accross_call skippes the call? Is it
because it is never called on it or is it because it quits on one of the early
exits?

It may be a case that we produce wrong strongly connected components with
presence of aliases...

Honza

[Bug ipa/65028] [5 Regression] 450.soplex in SPEC CPU 2006 is miscompiled

[jamborm at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65028

--- Comment #41 from Martin Jambor <jamborm at gcc dot gnu.org> ---
(In reply to H.J. Lu from comment #24)
> 
> IPA-CP missed _ZN18eonImageCalculatorC1Ev which calls
> _ZmlRK10ggSpectrumS1_ with 8-byte aligned rPrimary.

If I am not mistaken and this call is in between nodes
__comp_ctor/50391 -> operator*/46520 (the numbers are symtab symbol
order numbers) then the weird thing is that I do not see this edge
among jump function listing.  Could we please see
-fdump-tree-release_ssa dumps of the caller
_ZN18eonImageCalculatorC1Ev please?

[Bug ipa/65028] [5 Regression] 450.soplex in SPEC CPU 2006 is miscompiled

[hjl.tools at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65028

--- Comment #39 from H.J. Lu <hjl.tools at gmail dot com> ---
(In reply to Jan Hubicka from comment #38)
> Created attachment 34803 [details]
> ipp
> 
> OK. Though I do not directly see how it can solve this wrong code issue.

It doesn't work.  propagate_constants_accross_call doesn't call
propagate_alignment_accross_jump_function for all edges.  On some
skipped edges, caller passes arguments with unknown alignment.

[Bug ipa/65028] [5 Regression] 450.soplex in SPEC CPU 2006 is miscompiled

[hjl.tools at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65028

--- Comment #42 from H.J. Lu <hjl.tools at gmail dot com> ---
(In reply to Martin Jambor from comment #41)
> (In reply to H.J. Lu from comment #24)
> > 
> > IPA-CP missed _ZN18eonImageCalculatorC1Ev which calls
> > _ZmlRK10ggSpectrumS1_ with 8-byte aligned rPrimary.
> 
> If I am not mistaken and this call is in between nodes
> __comp_ctor/50391 -> operator*/46520 (the numbers are symtab symbol
> order numbers) then the weird thing is that I do not see this edge
> among jump function listing.  Could we please see
> -fdump-tree-release_ssa dumps of the caller
> _ZN18eonImageCalculatorC1Ev please?

static bool 
propagate_alignment_accross_jump_function (struct cgraph_edge *cs, 
                                           struct ipa_jump_func *jfunc,
                                           struct ipcp_param_lattices
*dest_lat){
  if (alignment_bottom_p (dest_lat))
    return false;

  ipa_alignment cur;
  cur.known = false; 
  if (jfunc->alignment.known)
    cur = jfunc->alignment;

Does it skip a jump func when its argument alignment is unknown?
Shouldn't propagate_alignment_accross_jump_function keeps the
minimum argument alignment, not the maximum argument alignment?

[Bug ipa/65028] [5 Regression] 450.soplex in SPEC CPU 2006 is miscompiled

[jamborm at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65028

--- Comment #43 from Martin Jambor <jamborm at gcc dot gnu.org> ---
(In reply to H.J. Lu from comment #42)
> Does it skip a jump func when its argument alignment is unknown?

No, I don't think it does.  Cur is initialized to unknown alignment
and then only overwrites the whole structure if alignment is known.

> Shouldn't propagate_alignment_accross_jump_function keeps the
> minimum argument alignment, not the maximum argument alignment?

No, it does not calculate minimums or maximums at all.  If it
encounters a different value than one it has seen previously, the
result is unknown alignment (this is what Honza thinks is just way too
pessimistic and fixes with his attached patch).

I really find it very suspicious that even the jump-function printing
code, which is a an iteration over edges as simple as they get, does
not see the wrong call.  If it is originally an indirect call, then
obviously initialization of the lattice is wrong because the
cgraph_local_p information is wrong.

[Bug ipa/65028] [5 Regression] 450.soplex in SPEC CPU 2006 is miscompiled

[hjl.tools at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65028

--- Comment #44 from H.J. Lu <hjl.tools at gmail dot com> ---
Created attachment 34804
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=34804&action=edit
Dumps from -fdump-ipa-cp-details -fdump-tree-release_ssa

[Bug ipa/65028] [5 Regression] 450.soplex in SPEC CPU 2006 is miscompiled

[hjl.tools at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65028

--- Comment #45 from H.J. Lu <hjl.tools at gmail dot com> ---
(In reply to Martin Jambor from comment #43)
>
> I really find it very suspicious that even the jump-function printing
> code, which is a an iteration over edges as simple as they get, does
> not see the wrong call.  If it is originally an indirect call, then
> obviously initialization of the lattice is wrong because the
> cgraph_local_p information is wrong.

Can you see anything wrong with the new dump?

[Bug ipa/65028] [5 Regression] 450.soplex in SPEC CPU 2006 is miscompiled

[jamborm at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65028

--- Comment #46 from Martin Jambor <jamborm at gcc dot gnu.org> ---
(In reply to H.J. Lu from comment #45)
> 
> Can you see anything wrong with the new dump?

release_ssa is an early optimization pass, wpa dump is not helpful.
We need release-ssa dump from the compilation stage (as opposed to the
linking stage) of the source file which contains the body of function
_ZN18eonImageCalculatorC1Ev

[Bug ipa/65028] [5 Regression] 450.soplex in SPEC CPU 2006 is miscompiled

[hjl.tools at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65028

--- Comment #48 from H.J. Lu <hjl.tools at gmail dot com> ---
(In reply to Martin Jambor from comment #46)
> release_ssa is an early optimization pass, wpa dump is not helpful.
> We need release-ssa dump from the compilation stage (as opposed to the
> linking stage) of the source file which contains the body of function
> _ZN18eonImageCalculatorC1Ev

I uploaded the new dump.

[Bug ipa/65028] [5 Regression] 450.soplex in SPEC CPU 2006 is miscompiled

[hjl.tools at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65028

H.J. Lu <hjl.tools at gmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #34793|0                           |1
        is obsolete|                            |
  Attachment #34804|0                           |1
        is obsolete|                            |

--- Comment #47 from H.J. Lu <hjl.tools at gmail dot com> ---
Created attachment 34805
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=34805&action=edit
Dumps from -fdump-ipa-cp-details -fdump-tree-release_ssa

[Bug ipa/65028] [5 Regression] 450.soplex in SPEC CPU 2006 is miscompiled

[trippels at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65028

--- Comment #49 from Markus Trippelsdorf <trippels at gcc dot gnu.org> ---
The same issue also happen when I build Firefox on my amdfam10 testbox.
With "-flto -march=native" Firefox crashes:

Program received signal SIGSEGV, Segmentation fault.
[Switching to Thread 0x7fff9a5fe700 (LWP 4779)]
0x00007ffff567b5cb in js::gc::ArenaCellIterImpl::reset(js::gc::ArenaHeader*) ()
from /var/tmp/moz-build-dir/dist/bin/libxul.so
(gdb) bt
#0  0x00007ffff567b5cb in
js::gc::ArenaCellIterImpl::reset(js::gc::ArenaHeader*) () from
/var/tmp/moz-build-dir/dist/bin/libxul.so
#1  0x00007ffff5ae9eff in JS::Zone::discardJitCode(js::FreeOp*) [clone
.part.593] [clone .lto_priv.66751] () from
/var/tmp/moz-build-dir/dist/bin/libxul.so
#2  0x00007ffff5c2bb1d in js::gc::GCRuntime::updatePointersToRelocatedCells()
() from /var/tmp/moz-build-dir/dist/bin/libxul.so
#3  0x00007ffff5c36a3b in
js::gc::GCRuntime::incrementalCollectSlice(js::SliceBudget&,
JS::gcreason::Reason) () from /var/tmp/moz-build-dir/dist/bin/libxul.so
#4  0x00007ffff5c37bb6 in js::gc::GCRuntime::gcCycle(bool, js::SliceBudget&,
JS::gcreason::Reason) () from /var/tmp/moz-build-dir/dist/bin/libxul.so
#5  0x00007ffff5c381e6 in js::gc::GCRuntime::collect(bool, js::SliceBudget,
JS::gcreason::Reason) () from /var/tmp/moz-build-dir/dist/bin/libxul.so
#6  0x00007ffff5258f60 in (anonymous
namespace)::GarbageCollectRunnable::WorkerRun(JSContext*,
mozilla::dom::workers::WorkerPrivate*) ()
   from /var/tmp/moz-build-dir/dist/bin/libxul.so
#7  0x00007ffff52a4762 in mozilla::dom::workers::WorkerRunnable::Run() () from
/var/tmp/moz-build-dir/dist/bin/libxul.so
#8  0x00007ffff5ead433 in
mozilla::dom::workers::WorkerPrivate::ProcessAllControlRunnablesLocked() [clone
.constprop.26404] () from /var/tmp/moz-build-dir/dist/bin/libxul.so
#9  0x00007ffff5137dfc in (anonymous
namespace)::WorkerThreadPrimaryRunnable::Run() [clone .lto_priv.60874] () from
/var/tmp/moz-build-dir/dist/bin/libxul.so
#10 0x00007ffff42f651a in nsThread::ProcessNextEvent(bool, bool*) () from
/var/tmp/moz-build-dir/dist/bin/libxul.so
#11 0x00007ffff642c0f9 in NS_ProcessNextEvent(nsIThread*, bool) [clone
.constprop.47437] () from /var/tmp/moz-build-dir/dist/bin/libxul.so
#12 0x00007ffff43aeec5 in
mozilla::ipc::MessagePumpForNonMainThreads::Run(base::MessagePump::Delegate*)
() from /var/tmp/moz-build-dir/dist/bin/libxul.so
#13 0x00007ffff4346798 in nsThread::ThreadFunc(void*) () from
/var/tmp/moz-build-dir/dist/bin/libxul.so
#14 0x00007ffff3128b5c in _pt_root () from /usr/lib/libnspr4.so
#15 0x00007ffff7bc53e6 in start_thread () from /lib/libpthread.so.0
#16 0x00007ffff74a06cd in clone () from /lib/libc.so.6

Using "-flto -fno-ipa-cp-alignment -march=native" fixes the problem.

[Bug ipa/65028] [5 Regression] 450.soplex in SPEC CPU 2006 is miscompiled

[jamborm at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65028

Martin Jambor <jamborm at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED

--- Comment #50 from Martin Jambor <jamborm at gcc dot gnu.org> ---
Finally, I have managed to reproduce the problem locally and am looking into
it.

[Bug ipa/65028] [5 Regression] 450.soplex in SPEC CPU 2006 is miscompiled

[jamborm at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65028

--- Comment #51 from Martin Jambor <jamborm at gcc dot gnu.org> ---
So unless I made some mistake, we are looking a the following chain of
calls and aliases

main/48071 -> _ZN18eonImageCalculatorC1Ev/50391 ->alias->
_ZN18eonImageCalculatorC2Ev/50390 ->
->_ZN12ggPhotometerC1ERK10ggSpectrumS2_S2_S2_S2_S2_S2_/49466
->alias->_ZN12ggPhotometerC2ERK10ggSpectrumS2_S2_S2_S2_S2_S2_/49465 ->
-> _ZmlRK10ggSpectrumS1_/46520

with the following jump functions:
    callsite  main/48071 -> __comp_ctor /50391 : 
       param 0: UNKNOWN
         Alignment: 16, misalignment: 0

    callsite  __base_ctor /50390 -> __comp_ctor /49466 : 
       param 0: UNKNOWN
         Alignment: 16, misalignment: 0
->     param 1: ANCESTOR: 0, offset 192
         Unknown alignment
       param 2: ANCESTOR: 0, offset 704
         Unknown alignment
       param 3: ANCESTOR: 0, offset 448
         Unknown alignment
       param 4: UNKNOWN
         Alignment: 16, misalignment: 0
       param 5: UNKNOWN
         Alignment: 16, misalignment: 0
       param 6: UNKNOWN
         Alignment: 16, misalignment: 0
       param 7: UNKNOWN
         Alignment: 16, misalignment: 0

    callsite  __base_ctor /49465 -> operator*/46520 : 
       param 0: ANCESTOR: 0, offset 1088
         Unknown alignment
       param 1: PASS THROUGH: 1 op nop_expr
         Unknown alignment

from poking at things in the debugger, the offset that is marked down as
192 looks much more like 24 in reality.... and 24 times 8 is 192... oh
boy...

[Bug ipa/65028] [5 Regression] 450.soplex in SPEC CPU 2006 is miscompiled

[jamborm at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65028

--- Comment #52 from Martin Jambor <jamborm at gcc dot gnu.org> ---
So, as you might have guessed from the previous comment, this is the
fix.  I should have left the office half an hour ago so I will
properly bootstrap and test and submit it tomorrow, but feel free to
do all of that yourself, if you do not want to wait.

diff --git a/gcc/ipa-cp.c b/gcc/ipa-cp.c
index 440ced4..3bf068a 100644
--- a/gcc/ipa-cp.c
+++ b/gcc/ipa-cp.c
@@ -1451,7 +1451,7 @@ propagate_alignment_accross_jump_function (struct
cgraph_edge *cs,
       else
        {
          src_idx = ipa_get_jf_ancestor_formal_id (jfunc);
-         offset = ipa_get_jf_ancestor_offset (jfunc);
+         offset = ipa_get_jf_ancestor_offset (jfunc) / BITS_PER_UNIT;
        }

       src_lats = ipa_get_parm_lattices (caller_info, src_idx);

[Bug ipa/65028] [5 Regression] 450.soplex in SPEC CPU 2006 is miscompiled

[hjl.tools at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65028

--- Comment #53 from H.J. Lu <hjl.tools at gmail dot com> ---
(In reply to Martin Jambor from comment #52)
> So, as you might have guessed from the previous comment, this is the
> fix.  I should have left the office half an hour ago so I will
> properly bootstrap and test and submit it tomorrow, but feel free to
> do all of that yourself, if you do not want to wait.
> 
> diff --git a/gcc/ipa-cp.c b/gcc/ipa-cp.c
> index 440ced4..3bf068a 100644
> --- a/gcc/ipa-cp.c
> +++ b/gcc/ipa-cp.c
> @@ -1451,7 +1451,7 @@ propagate_alignment_accross_jump_function (struct
> cgraph_edge *cs,
>        else
>         {
>           src_idx = ipa_get_jf_ancestor_formal_id (jfunc);
> -         offset = ipa_get_jf_ancestor_offset (jfunc);
> +         offset = ipa_get_jf_ancestor_offset (jfunc) / BITS_PER_UNIT;
>         }
>  
>        src_lats = ipa_get_parm_lattices (caller_info, src_idx);

I verified that it fixed 252.eon on x32.  Thanks.

[Bug ipa/65028] [5 Regression] 450.soplex in SPEC CPU 2006 is miscompiled

[hubicka at ucw dot cz]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65028

--- Comment #54 from Jan Hubicka <hubicka at ucw dot cz> ---
> 
> diff --git a/gcc/ipa-cp.c b/gcc/ipa-cp.c
> index 440ced4..3bf068a 100644
> --- a/gcc/ipa-cp.c
> +++ b/gcc/ipa-cp.c
> @@ -1451,7 +1451,7 @@ propagate_alignment_accross_jump_function (struct
> cgraph_edge *cs,
>        else
>         {
>           src_idx = ipa_get_jf_ancestor_formal_id (jfunc);
> -         offset = ipa_get_jf_ancestor_offset (jfunc);
> +         offset = ipa_get_jf_ancestor_offset (jfunc) / BITS_PER_UNIT;
>         }
> 
>        src_lats = ipa_get_parm_lattices (caller_info, src_idx);

Hehe, patch is OK
What about the lattice operations patch. If it looks fine to you, i will
regtest/bootstrap and commit.

Honza

[Bug ipa/65028] [5 Regression] 450.soplex in SPEC CPU 2006 is miscompiled

[hjl.tools at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65028

--- Comment #55 from H.J. Lu <hjl.tools at gmail dot com> ---
Another issue is propagate_constants_accross_call has

  for (; (i < args_count) && (i < parms_count); i++)
    {
      struct ipa_jump_func *jump_func = ipa_get_ith_jump_func (args, i);
      struct ipcp_param_lattices *dest_plats;

      dest_plats = ipa_get_parm_lattices (callee_info, i);
      if (availability == AVAIL_INTERPOSABLE)
        ret |= set_all_contains_variable (dest_plats);
      else
        {
          ret |= propagate_scalar_accross_jump_function (cs, jump_func,
                                                         &dest_plats->itself);
          ret |= propagate_context_accross_jump_function (cs, jump_func, i,
                                                          &dest_plats->ctxlat);
          ret |= propagate_alignment_accross_jump_function (cs, jump_func,
                                                            dest_plats);
          ret |= propagate_aggs_accross_jump_function (cs, jump_func,
                                                       dest_plats);
        }
    }

Even if availability == AVAIL_INTERPOSABLE, alignment from jump_func
is used if the function isn't preempted at run-time.  Shouldn't it
call propagate_alignment_accross_jump_function unconditionally?

[Bug ipa/65028] [5 Regression] 450.soplex in SPEC CPU 2006 is miscompiled

[hubicka at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65028

--- Comment #56 from Jan Hubicka <hubicka at gcc dot gnu.org> ---
Author: hubicka
Date: Thu Feb 19 23:31:40 2015
New Revision: 220826

URL: https://gcc.gnu.org/viewcvs?rev=220826&root=gcc&view=rev
Log:
    PR ipa/65028
    * ipa-cp.c (propagate_alignment_accross_jump_function): Fix propagation
    across jump functions.

Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/ipa-cp.c

[Bug ipa/65028] [5 Regression] 450.soplex in SPEC CPU 2006 is miscompiled

[hubicka at ucw dot cz]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65028

--- Comment #57 from Jan Hubicka <hubicka at ucw dot cz> ---
> Another issue is propagate_constants_accross_call has
> 
>   for (; (i < args_count) && (i < parms_count); i++)
>     {
>       struct ipa_jump_func *jump_func = ipa_get_ith_jump_func (args, i);
>       struct ipcp_param_lattices *dest_plats;
> 
>       dest_plats = ipa_get_parm_lattices (callee_info, i);
>       if (availability == AVAIL_INTERPOSABLE)
>         ret |= set_all_contains_variable (dest_plats);
>       else
>         {
>           ret |= propagate_scalar_accross_jump_function (cs, jump_func,
>                                                          &dest_plats->itself);
>           ret |= propagate_context_accross_jump_function (cs, jump_func, i,
>                                                           &dest_plats->ctxlat);
>           ret |= propagate_alignment_accross_jump_function (cs, jump_func,
>                                                             dest_plats);
>           ret |= propagate_aggs_accross_jump_function (cs, jump_func,
>                                                        dest_plats);
>         }
>     }
> 
> Even if availability == AVAIL_INTERPOSABLE, alignment from jump_func
> is used if the function isn't preempted at run-time.  Shouldn't it
> call propagate_alignment_accross_jump_function unconditionally?
Well, set_all_contains_variable will simply make every INTERPOSABLE function to
have
all parameters with unknown alignment.  This is probably OK because the
function is
externally visible anyway and thus called from there.

I commented on this privately to Martin some time ago.  While technically one
can find that the functions if not interposed are used one way or another, but
because we can't clone these functions (we would need to add runtime check
for interposition then), we don't have any use for that anyway.

Martin, I just tested firefox with -march=native -flto -O3 and it still
segfaults
for me at movaps, so it seems that there is still some issue with the code :(
(it happens with or withtout the propagation patch I posted)

Honza

[Bug ipa/65028] [5 Regression] 450.soplex in SPEC CPU 2006 is miscompiled

[trippels at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65028

--- Comment #58 from Markus Trippelsdorf <trippels at gcc dot gnu.org> ---
The Firefox issue from comment #49 is fixed.

[Bug ipa/65028] [5 Regression] 450.soplex in SPEC CPU 2006 is miscompiled

[jamborm at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65028

--- Comment #59 from Martin Jambor <jamborm at gcc dot gnu.org> ---
(In reply to Jan Hubicka from comment #36)
> We end up with algnment unkonwn instead of a. (did not managed to reproduce
> the wrong alignment here).  What about the following:

It is certainly better than what I had there before, except that
unless I read it wrong, I think there are two bugs:

> Index: ipa-cp.c
> ===================================================================
> --- ipa-cp.c	(revision 220789)
> +++ ipa-cp.c	(working copy)
> @@ -1409,6 +1409,60 @@ propagate_context_accross_jump_function
>    return ret;
>  }
>  
> +/* Decrease alignment info DEST to be at most CUR.  */
> +
> +static bool
> +decrease_alignment (ipa_alignment *dest, ipa_alignment cur)
> +{
> +  bool changed = false;
> +
> +  if (!cur.known)
> +    return false;

I really think this should be return set_alignment_to_bottom (dest);

If some known alignment has been already propagated to DEST along a
different edge and now along the current edge an unknown alignment is
coming in, then the result value of the lattice must be BOTTOM and not
the previous alignment this code leaves in place.

> +  if (!dest->known)
> +    {
> +      *dest = cur;
> +      return true;
> +    }
> +  if (dest->align == cur.align
> +      && dest->misalign == cur.misalign)
> +    return false;
> +
> +  if (dest->align > cur.align)
> +    {
> +      dest->align = cur.align;
> +      if (cur.align)
> +	dest->misalign
> +	  = dest->misalign % cur.align;
> +      changed = true;
> +    }
> +  if (dest->align && (dest->misalign != (cur.misalign % dest->align)))
> +    {
> +      int diff = abs (dest->misalign
> +		      - (cur.misalign % dest->align));
> +      dest->align = MIN (dest->align, (unsigned)diff & - diff);
> +      if (dest->align)
> +	dest->misalign
> +	  = dest->misalign % dest->align;
> +      changed = true;
> +    }
> +  return changed;
> +}
> +
> +/* Increase alignment info DEST to be at least CUR.  */
> +
> +static bool
> +increase_alignment (ipa_alignment *dest, ipa_alignment cur)
> +{
> +  if (!dest->known)
> +    return false;

I think this condition always exits.  DEST is caller's CUR which was
read from jfunc->alignment which is always going to be unknown for
PASS_THROUGH and ANCESTOR jump functions at the moment.  Perhaps you
meant if (!cur.known) ?

> +  if (!cur.known || dest->align < cur.align)
> +    {

again here, I think you meant !des->.known.

Apart from that, this is clearly an improvement, thanks.
>From gcc-bugs-return-477947-listarch-gcc-bugs=gcc.gnu.org@gcc.gnu.org Fri Feb 20 13:41:56 2015
Return-Path: <gcc-bugs-return-477947-listarch-gcc-bugs=gcc.gnu.org@gcc.gnu.org>
Delivered-To: listarch-gcc-bugs@gcc.gnu.org
Received: (qmail 12743 invoked by alias); 20 Feb 2015 13:41:56 -0000
Mailing-List: contact gcc-bugs-help@gcc.gnu.org; run by ezmlm
Precedence: bulk
List-Id: <gcc-bugs.gcc.gnu.org>
List-Archive: <http://gcc.gnu.org/ml/gcc-bugs/>
List-Post: <mailto:gcc-bugs@gcc.gnu.org>
List-Help: <mailto:gcc-bugs-help@gcc.gnu.org>
Sender: gcc-bugs-owner@gcc.gnu.org
Delivered-To: mailing list gcc-bugs@gcc.gnu.org
Received: (qmail 12674 invoked by uid 48); 20 Feb 2015 13:41:52 -0000
From: "ysrumyan at gmail dot com" <gcc-bugzilla@gcc.gnu.org>
To: gcc-bugs@gcc.gnu.org
Subject: [Bug rtl-optimization/65135] [5 Regression] Performance regression  in pic mode after r220674.
Date: Fri, 20 Feb 2015 13:49:00 -0000
X-Bugzilla-Reason: CC
X-Bugzilla-Type: changed
X-Bugzilla-Watch-Reason: None
X-Bugzilla-Product: gcc
X-Bugzilla-Component: rtl-optimization
X-Bugzilla-Version: 5.0
X-Bugzilla-Keywords: missed-optimization
X-Bugzilla-Severity: normal
X-Bugzilla-Who: ysrumyan at gmail dot com
X-Bugzilla-Status: WAITING
X-Bugzilla-Priority: P3
X-Bugzilla-Assigned-To: unassigned at gcc dot gnu.org
X-Bugzilla-Target-Milestone: 5.0
X-Bugzilla-Flags:
X-Bugzilla-Changed-Fields:
Message-ID: <bug-65135-4-jS5zTGVtXd@http.gcc.gnu.org/bugzilla/>
In-Reply-To: <bug-65135-4@http.gcc.gnu.org/bugzilla/>
References: <bug-65135-4@http.gcc.gnu.org/bugzilla/>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 7bit
X-Bugzilla-URL: http://gcc.gnu.org/bugzilla/
Auto-Submitted: auto-generated
MIME-Version: 1.0
X-SW-Source: 2015-02/txt/msg02279.txt.bz2
Content-length: 1273

https://gcc.gnu.org/bugzilla/show_bug.cgi?ide135

--- Comment #4 from Yuri Rumyantsev <ysrumyan at gmail dot com> ---
This patch improves performance of almost all benchmarks in pic-mode for 32-bit
target, but we have the only huge degradation on benchmark from eembc1.1 suite.

I mentioned that the innermost loop has one more fill.

before r220674
.L8:
    testl    %esi, %esi
    jne    .L11
    movl    16(%esp), %eax
    movl    (%eax,%edx), %eax
    cmpl    $-1, %eax
    je    .L13
.L43:
    movl    4(%esp), %edi
    leal    0(,%eax,4), %edx
    movl    %ecx, %ebx
    addl    %edx, %edi
    cmpl    $101, %ecx
    je    .L41
    leal    1(%ebx), %ecx
    movl    8(%esp), %ebp
    cmpl    (%edi), %ebp
    jne    .L8

after r220674
.L6:
    movl    4(%esp), %ebx   <--  redundant fill
    testl    %ebx, %ebx
    jne    .L9
    movl    20(%esp), %eax
    movl    (%eax,%edx), %eax
    cmpl    $-1, %eax
    je    .L11
.L42:
    movl    8(%esp), %edi
    leal    0(,%eax,4), %edx
    movl    %ecx, %ebx
    addl    %edx, %edi
    cmpl    $101, %ecx
    je    .L40
    leal    1(%ebx), %ecx
    movl    12(%esp), %esi
    cmpl    (%edi), %esi
    jne    .L6

The issue is that before the fix GOT-register is not considered for allocation
it to physical register.