[Bug ipa/66616] [4.9/5/6 regression] fipa-cp-clone ignores thunk

[rguenth at gcc dot gnu.org]

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

--- Comment #24 from Richard Biener  ---
*** Bug 68981 has been marked as a duplicate of this bug. ***

[Bug ipa/66616] [4.9/5/6 regression] fipa-cp-clone ignores thunk

[jamborm at gcc dot gnu.org]

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

--- Comment #21 from Martin Jambor  ---
Author: jamborm
Date: Mon Jan 11 10:03:44 2016
New Revision: 232214

URL: https://gcc.gnu.org/viewcvs?rev=232214=gcc=rev
Log:
[PR ipa/66616] Copy can_change_signature flag to artificial thunks

2016-01-11  Martin Jambor  

PR ipa/66616
* cgraphclones.c (duplicate_thunk_for_node): Copy can_change_signature
flag.


Modified:
trunk/gcc/ChangeLog
trunk/gcc/cgraphclones.c

[Bug ipa/66616] [4.9/5/6 regression] fipa-cp-clone ignores thunk

[jamborm at gcc dot gnu.org]

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

--- Comment #20 from Martin Jambor  ---
Author: jamborm
Date: Mon Jan 11 09:59:48 2016
New Revision: 232213

URL: https://gcc.gnu.org/viewcvs?rev=232213=gcc=rev
Log:
[PR 66616] Check for thunks when adding extra constants to clones

2016-01-11  Martin Jambor  

PR ipa/66616
* ipa-cp.c (propagate_constants_accross_call): Move thuk check...
(call_passes_through_thunk_p): ...here.
(find_more_scalar_values_for_callers_subset): Perform thunk checks
like propagate_constants_accross_call does.
* cgraphclones.c (duplicate_thunk_for_node): Copy can_change_signature
flag from the node to the new flag.

testsuite/
* g++.dg/ipa/pr66616.C: New test.


Added:
branches/gcc-4_9-branch/gcc/testsuite/g++.dg/ipa/pr66616.C
Modified:
branches/gcc-4_9-branch/gcc/ChangeLog
branches/gcc-4_9-branch/gcc/cgraphclones.c
branches/gcc-4_9-branch/gcc/ipa-cp.c
branches/gcc-4_9-branch/gcc/testsuite/ChangeLog

[Bug ipa/66616] [4.9/5/6 regression] fipa-cp-clone ignores thunk

[jamborm at gcc dot gnu.org]

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

--- Comment #22 from Martin Jambor  ---
Author: jamborm
Date: Mon Jan 11 12:44:53 2016
New Revision: 232226

URL: https://gcc.gnu.org/viewcvs?rev=232226=gcc=rev
Log:
[PR ipa/66616] Copy can_change_signature flag to artificial thunks

2016-01-11  Martin Jambor  

PR ipa/66616
* cgraphclones.c (duplicate_thunk_for_node): Copy can_change_signature
flag.


Modified:
branches/gcc-5-branch/gcc/ChangeLog
branches/gcc-5-branch/gcc/cgraphclones.c

[Bug ipa/66616] [4.9/5/6 regression] fipa-cp-clone ignores thunk

[jamborm at gcc dot gnu.org]

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

Martin Jambor  changed:

   What|Removed |Added

 Status|NEW |RESOLVED
 Resolution|--- |FIXED

--- Comment #23 from Martin Jambor  ---
Hopefully finally fixed

[Bug ipa/66616] [4.9/5/6 regression] fipa-cp-clone ignores thunk

[jamborm at gcc dot gnu.org]

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

--- Comment #19 from Martin Jambor  ---
I have posted the patch to address the i386 -m32 issue to the mailing
list:

https://gcc.gnu.org/ml/gcc-patches/2016-01/msg00427.html

[Bug ipa/66616] [4.9/5/6 regression] fipa-cp-clone ignores thunk

[jamborm at gcc dot gnu.org]

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

--- Comment #17 from Martin Jambor  ---
Yesterday Honza told me on IRC to check that the new artificial thunk
has local flag set.  However, it appears that it does (in
set_new_clone_decl_and_node_flags), so I am going to investigate
further.

[Bug ipa/66616] [4.9/5/6 regression] fipa-cp-clone ignores thunk

[jamborm at gcc dot gnu.org]

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

--- Comment #18 from Martin Jambor  ---
So it was a near miss, the problem appears to be the
can_change_signature flag instead.  The following seems to fix the
-m32 failure.  I'm going to bootstrap it and if it passes, send it to
the mailing list.

diff --git a/gcc/cgraphclones.c b/gcc/cgraphclones.c
index f8a7d37..8759ce4 100644
--- a/gcc/cgraphclones.c
+++ b/gcc/cgraphclones.c
@@ -328,6 +328,7 @@ duplicate_thunk_for_node (cgraph_node *thunk, cgraph_node
*node)
   new_thunk = cgraph_node::create (new_decl);
   set_new_clone_decl_and_node_flags (new_thunk);
   new_thunk->definition = true;
+  new_thunk->local.can_change_signature = node->local.can_change_signature;
   new_thunk->thunk = thunk->thunk;
   new_thunk->unique_name = in_lto_p;
   new_thunk->former_clone_of = thunk->decl;

[Bug ipa/66616] [4.9/5/6 regression] fipa-cp-clone ignores thunk

[jamborm at gcc dot gnu.org]

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

--- Comment #16 from Martin Jambor  ---
(In reply to H.J. Lu from comment #13)
> I got
> 
> FAIL: g++.dg/ipa/pr66616.C  -std=gnu++11 execution test
> FAIL: g++.dg/ipa/pr66616.C  -std=gnu++14 execution test
> FAIL: g++.dg/ipa/pr66616.C  -std=gnu++98 execution test
> 
> on trunk/x86-64.

Ugh, it seems that with -m32, the clone and its artificial thunks use
a different calling convention than their caller.

With -fno-ipa-cp -fno-inline, the generated assembly for the original
thunk is:

_ZThn12_N1B3fooEi:
.LFB20:
.cfi_startproc
subl$12, 4(%esp)
jmp .LTHUNK0
.cfi_endproc

and the method that IPA-CP would choose to clone also accesses
arguments on the stack.

Whereas with only -fno-inline, the generated assembly for the method
and its artificial thunk is:

_ZN1B3fooEi.constprop.1:
.LFB20:
.cfi_startproc
movl16(%eax), %eax
movl%eax, _ZL2go
ret

_ZThn12_N1B3fooEi.artificial_thunk.2:
.LFB23:
.cfi_startproc
subl$12, %eax
jmp _ZN1B3fooEi.constprop.1

But in both cases, the caller of the thunk (main), attempts to pass
the argument on the stack.

Honza, do you know how is the calling convention determined at the
caller side?  Do we need to change something in gimple for expansion
to take a hint?

[Bug ipa/66616] [4.9/5/6 regression] fipa-cp-clone ignores thunk

[hjl.tools at gmail dot com]

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

--- Comment #15 from H.J. Lu  ---
(In reply to H.J. Lu from comment #14)
> (In reply to H.J. Lu from comment #13)
> > I got
> > 
> > FAIL: g++.dg/ipa/pr66616.C  -std=gnu++11 execution test
> > FAIL: g++.dg/ipa/pr66616.C  -std=gnu++14 execution test
> > FAIL: g++.dg/ipa/pr66616.C  -std=gnu++98 execution test
> > 
> > on trunk/x86-64.
> 
> It fails with -m32 on x86-64 for trunk and gcc-5-branch:
> 

It also fails on i686.

[Bug ipa/66616] [4.9/5/6 regression] fipa-cp-clone ignores thunk

[hjl.tools at gmail dot com]

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

--- Comment #14 from H.J. Lu  ---
(In reply to H.J. Lu from comment #13)
> I got
> 
> FAIL: g++.dg/ipa/pr66616.C  -std=gnu++11 execution test
> FAIL: g++.dg/ipa/pr66616.C  -std=gnu++14 execution test
> FAIL: g++.dg/ipa/pr66616.C  -std=gnu++98 execution test
> 
> on trunk/x86-64.

It fails with -m32 on x86-64 for trunk and gcc-5-branch:

[hjl@gnu-6 gcc]$ 
/export/build/gnu/gcc-x32-5/build-x86_64-linux/gcc/testsuite/g++/../../xg++
-B/export/build/gnu/gcc-x32-5/build-x86_64-linux/gcc/testsuite/g++/../../
/export/gnu/import/git/sources/gcc-release/gcc/testsuite/g++.dg/ipa/pr66616.C
-m32 -fno-diagnostics-show-caret -fdiagnostics-color=never -nostdinc++
-I/export/build/gnu/gcc-x32-5/build-x86_64-linux/x86_64-unknown-linux-gnu/32/libstdc++-v3/include/x86_64-unknown-linux-gnu
-I/export/build/gnu/gcc-x32-5/build-x86_64-linux/x86_64-unknown-linux-gnu/32/libstdc++-v3/include
-I/export/gnu/import/git/sources/gcc-release/libstdc++-v3/libsupc++
-I/export/gnu/import/git/sources/gcc-release/libstdc++-v3/include/backward
-I/export/gnu/import/git/sources/gcc-release/libstdc++-v3/testsuite/util
-fmessage-length=0 -std=gnu++14 -O2 -fipa-cp-clone
-L/export/build/gnu/gcc-x32-5/build-x86_64-linux/x86_64-unknown-linux-gnu/32/libstdc++-v3/src/.libs
-B/export/build/gnu/gcc-x32-5/build-x86_64-linux/x86_64-unknown-linux-gnu/32/libstdc++-v3/src/.libs
-L/export/build/gnu/gcc-x32-5/build-x86_64-linux/x86_64-unknown-linux-gnu/32/libstdc++-v3/src/.libs
-lm -o ./pr66616.exe
[hjl@gnu-6 gcc]$ ./pr66616.exe
Aborted
[hjl@gnu-6 gcc]$

[Bug ipa/66616] [4.9/5/6 regression] fipa-cp-clone ignores thunk

[jamborm at gcc dot gnu.org]

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

--- Comment #12 from Martin Jambor  ---
No, I'm still in the process of testing a slightly modified patch for 4.9.

[Bug ipa/66616] [4.9/5/6 regression] fipa-cp-clone ignores thunk

[hjl.tools at gmail dot com]

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

H.J. Lu  changed:

   What|Removed |Added

 CC||hjl.tools at gmail dot com

--- Comment #13 from H.J. Lu  ---
I got

FAIL: g++.dg/ipa/pr66616.C  -std=gnu++11 execution test
FAIL: g++.dg/ipa/pr66616.C  -std=gnu++14 execution test
FAIL: g++.dg/ipa/pr66616.C  -std=gnu++98 execution test

on trunk/x86-64.

[Bug ipa/66616] [4.9/5/6 regression] fipa-cp-clone ignores thunk

[jamborm at gcc dot gnu.org]

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

--- Comment #9 from Martin Jambor  ---
Author: jamborm
Date: Mon Dec 14 09:33:12 2015
New Revision: 231607

URL: https://gcc.gnu.org/viewcvs?rev=231607=gcc=rev
Log:
[PR 66616] Check for thunks when adding extra constants to clones

2015-12-14  Martin Jambor  

PR ipa/66616
* ipa-cp.c (propagate_constants_accross_call): Move thuk check...
(call_passes_through_thunk_p): ...here.
(find_more_scalar_values_for_callers_subset): Perform thunk checks
like propagate_constants_accross_call does.

testsuite/
* g++.dg/ipa/pr66616.C: New test.


Added:
trunk/gcc/testsuite/g++.dg/ipa/pr66616.C
Modified:
trunk/gcc/ChangeLog
trunk/gcc/ipa-cp.c
trunk/gcc/testsuite/ChangeLog

[Bug ipa/66616] [4.9/5/6 regression] fipa-cp-clone ignores thunk

[jamborm at gcc dot gnu.org]

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

--- Comment #10 from Martin Jambor  ---
Author: jamborm
Date: Mon Dec 14 16:31:26 2015
New Revision: 231622

URL: https://gcc.gnu.org/viewcvs?rev=231622=gcc=rev
Log:
[PR 66616] Check for thunks when adding extra constants to clones

2015-12-14  Martin Jambor  

PR ipa/66616
* ipa-cp.c (propagate_constants_accross_call): Move thuk check...
(call_passes_through_thunk_p): ...here.
(find_more_scalar_values_for_callers_subset): Perform thunk checks
like propagate_constants_accross_call does.

testsuite/
* g++.dg/ipa/pr66616.C: New test.


Added:
branches/gcc-5-branch/gcc/testsuite/g++.dg/ipa/pr66616.C
Modified:
branches/gcc-5-branch/gcc/ChangeLog
branches/gcc-5-branch/gcc/ipa-cp.c
branches/gcc-5-branch/gcc/testsuite/ChangeLog

[Bug ipa/66616] [4.9/5/6 regression] fipa-cp-clone ignores thunk

[hubicka at gcc dot gnu.org]

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

--- Comment #11 from Jan Hubicka  ---
Fixed now?

[Bug ipa/66616] [4.9/5/6 regression] fipa-cp-clone ignores thunk

[jamborm at gcc dot gnu.org]

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

--- Comment #8 from Martin Jambor  ---
After looking at all the wrong places I finally found the correct
one.  I have proposed a fix on the mailing list:

https://gcc.gnu.org/ml/gcc-patches/2015-12/msg01271.html

[Bug ipa/66616] [4.9/5/6 regression] fipa-cp-clone ignores thunk

[jamborm at gcc dot gnu.org]

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

Martin Jambor  changed:

   What|Removed |Added

   Assignee|unassigned at gcc dot gnu.org  |jamborm at gcc dot 
gnu.org

--- Comment #7 from Martin Jambor  ---
I will.

[Bug ipa/66616] [4.9/5/6 regression] fipa-cp-clone ignores thunk

[hubicka at gcc dot gnu.org]

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

--- Comment #6 from Jan Hubicka  ---
Martin, this is your area, can you take a look?

[Bug ipa/66616] [4.9/5/6 regression] fipa-cp-clone ignores thunk

[rguenth at gcc dot gnu.org]

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

Richard Biener  changed:

   What|Removed |Added

   Priority|P3  |P2
 Status|UNCONFIRMED |NEW
   Last reconfirmed||2015-11-18
 CC||jamborm at gcc dot gnu.org
  Known to work||4.8.5
 Ever confirmed|0   |1
  Known to fail||4.9.3, 5.2.0, 6.0

--- Comment #5 from Richard Biener  ---
Still broken.

[Bug ipa/66616] [4.9/5/6 regression] fipa-cp-clone ignores thunk

[rguenth at gcc dot gnu.org]

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

Richard Biener  changed:

   What|Removed |Added

   Target Milestone|--- |4.9.4