Bug 3713

Summary: Pointers to functions or member functions are not folded or inlined
Product: gcc Reporter: philippeb
Component: tree-optimizationAssignee: Richard Biener <rguenth>
Status: RESOLVED FIXED    
Severity: enhancement CC: aloysius.wild, bangerth, cesarb, gcc-bugs, hubicka, ian, kirr, lerdsuwa, nathan, paolo.carlini, smelkov
Priority: P3 Keywords: missed-optimization
Version: 3.0   
Target Milestone: ---   
See Also: http://bugs.debian.org/173513
Host: Target:
Build: Known to work: 4.9.0
Known to fail: Last reconfirmed: 2008-04-16 13:27:21
Bug Depends on: 15838, 26282    
Bug Blocks: 55996    
Attachments: inlineptf.cpp
Implements folding of "(&function & 1)"
testcase for member function pointer that isn't inlined
patch

Description philippeb 2001-07-17 10:16:00 UTC
Pointers to functions or member functions are not compile-time constants, meaning that the functions won't be inlined, even if the function is not virtual. It seems also that 'const' modifiers applied to pointers to member are discarded (tested by calling a template function).

The attached program demontrates the problem by confirming that the compiler needs references to those constants:

/tmp/ccQihCjs.o: In function `main':
/home/philippeb/devel/tests/inlineptf.cpp:17: undefined reference to `A::p'
/home/philippeb/devel/tests/inlineptf.cpp:17: undefined reference to `A::p'

Release:
20010319 (Debian prerelease)

Environment:
Debian Linux on i386
Comment 1 philippeb 2001-07-17 10:16:00 UTC
Fix:
Don't use a pointer to function if you want your code to be inlined.
Comment 2 Kriang Lerdsuwanakij 2001-08-10 08:37:51 UTC
State-Changed-From-To: open->closed
State-Changed-Why: Not a bug.  A::p must be defined somewhere outside the class
    A.  The following is the correct version of the code:
    
    #include <iostream>
    using namespace std;
    
    struct A
    {
            void foo()
            {
                    cout << __PRETTY_FUNCTION__ << endl;
            }
    			// This must be defined later
            static void (A::* const p)() = & A::foo;
    };
    void (A::* const A::p)();	// Definition
    
    int main()
    {
            A a;
            (a.*A::p)();
    }
Comment 3 Kriang Lerdsuwanakij 2001-09-17 07:22:53 UTC
State-Changed-From-To: closed->open
State-Changed-Why: Test case reopened.  After the fix, and compile with
    optimization, function is still not inlined.  Dump of
    assembly in followup shown in PR4277.
Comment 4 Nathan Sidwell 2001-12-15 10:07:25 UTC
State-Changed-From-To: open->analyzed
State-Changed-Why: still a pessimization
Comment 5 Andrew Pinski 2003-05-25 00:05:33 UTC
*** Bug 4277 has been marked as a duplicate of this bug. ***
Comment 6 Andrew Pinski 2003-05-27 22:33:05 UTC
The code is no longer compiles with the mainline because it is invalid C++, here is the 
new source (also removing iostream and removing all the junk):
    struct A
    {
            void foo()
            {
            }
                        // This must be defined later
            static void (A::* const p)();
    };
    void (A::* const A::p)() = & A::foo;        // Definition

    int main()
    {
            A a;
            (a.*A::p)();
    }
Comment 7 Andrew Pinski 2003-07-08 00:53:24 UTC
Changing this to optimization since this is good thing to do for all languages.
Comment 8 Andrew Pinski 2003-07-08 00:53:53 UTC
*** Bug 9079 has been marked as a duplicate of this bug. ***
Comment 9 Andrew Pinski 2004-06-06 01:26:26 UTC
I filed PR15838 to track part of this bug which needs to be done to help fix this.
Comment 10 Andrew Pinski 2004-07-03 19:40:53 UTC
*** Bug 16352 has been marked as a duplicate of this bug. ***
Comment 11 Steven Bosscher 2005-01-23 15:14:53 UTC
*** Bug 9079 has been marked as a duplicate of this bug. ***
Comment 12 Andrew Pinski 2005-07-04 21:54:25 UTC
Actually the testcase above is not really valid.
The below testcase is more correct as A::p cannot be inlined as it has its address taken (stupid bug in 
the C++ front-end really).
But here is a better testcase:
    struct A
    {
            void foo()
            {
            }
    };

    int main()
    {
     void (A::* const p)() = & A::foo;
            A a;
            (a.*p)();
    }


---- cut ---
  D.1749 = (int) foo;
  if ((D.1749 & 1) != 0) goto <L0>; else goto <L5>;

hmm, we need to know the alignment of a function, which is weird.  Why is the C++ front-end 
producing code like this, oh well.
Comment 13 Paolo Carlini 2005-07-04 23:42:13 UTC
... alternately, one we really care about, performance-wise, could be distilled
from libstdc++-v3/testsuite/performance/27_io/fmtflags_manipulators.cc...
Comment 14 Guillaume Melquiond 2007-09-02 11:56:07 UTC
Created attachment 14150 [details]
Implements folding of "(&function & 1)"

I encountered the same issue with some heavy template code: GCC was generating some really awful code, while it should have been optimized to almost anything. It basically comes down to this kind of C++ code:

  struct A { void a(); };
  template < void (A::*f)() >
  void g(A *t) { (t->*f)(); }
  void h(A *t) { g<&A::a>(t); }

After instantiation, it is somehow transformed to:

  struct A { void a(); };
  void h(A *t) { (t->*(&A::a))(); }

And GCC is unable to optimize it. So I played a bit with the attached patch, which does fix the issue for my template code.

Note that GCC does inline the member function for my code (just add an empty body to A::a to test it), but it doesn't inline it in Andrew Pinski's last testcase. I have no idea why; perhaps because the address goes through a variable. But at least the generated code no longer contain any crap and it directly calls the correct function, even if it doesn't inline it.

(As for why the C++ front-end generates this code in the first place, it is because the lower bit of the pointer indicates whether an extra indirection is needed when calling the function through the pointer to member. The C++ front-end actually assumes that functions have an alignment of 2 at least, so that this trick can work.)
Comment 15 Andrew Pinski 2007-09-24 19:45:54 UTC
The testcase in comment #12 was fixed via http://gcc.gnu.org/ml/gcc-cvs/2007-09/msg00696.html .
Comment 16 Martin Jambor 2008-04-16 13:27:21 UTC
I'm now working on inlining of indirect calls (PR 9079) and intend to allow for inlining of calls through member pointers too.
Comment 17 Martin Jambor 2009-08-06 12:22:04 UTC
Note to self: PR 40874 is related.
Comment 18 Jan Hubicka 2010-11-10 22:12:42 UTC
Martin,
is this solved now?
Comment 19 owner 2010-11-10 22:24:15 UTC
Thank you for the additional information you have supplied regarding
this Bug report.

This is an automatically generated reply to let you know your message
has been received.

Your message is being forwarded to the package maintainers and other
interested parties for their attention; they will reply in due course.

Your message has been sent to the package maintainer(s):
 Debian GCC Maintainers <debian-gcc@lists.debian.org>

If you wish to submit further information on this problem, please
send it to 173513@bugs.debian.org.

Please do not send mail to owner@bugs.debian.org unless you wish
to report a problem with the Bug-tracking system.
Comment 20 owner 2010-11-10 22:27:14 UTC
Thank you for the additional information you have supplied regarding
this Bug report.

This is an automatically generated reply to let you know your message
has been received.

Your message is being forwarded to the package maintainers and other
interested parties for their attention; they will reply in due course.

Your message has been sent to the package maintainer(s):
 Debian GCC Maintainers <debian-gcc@lists.debian.org>

If you wish to submit further information on this problem, please
send it to 173513@bugs.debian.org.

Please do not send mail to owner@bugs.debian.org unless you wish
to report a problem with the Bug-tracking system.
Comment 21 owner 2010-11-10 22:30:23 UTC
Thank you for the additional information you have supplied regarding
this Bug report.

This is an automatically generated reply to let you know your message
has been received.

Your message is being forwarded to the package maintainers and other
interested parties for their attention; they will reply in due course.

Your message has been sent to the package maintainer(s):
 Debian GCC Maintainers <debian-gcc@lists.debian.org>

If you wish to submit further information on this problem, please
send it to 173513@bugs.debian.org.

Please do not send mail to owner@bugs.debian.org unless you wish
to report a problem with the Bug-tracking system.
Comment 22 owner 2010-11-10 22:33:17 UTC
Thank you for the additional information you have supplied regarding
this Bug report.

This is an automatically generated reply to let you know your message
has been received.

Your message is being forwarded to the package maintainers and other
interested parties for their attention; they will reply in due course.

Your message has been sent to the package maintainer(s):
 Debian GCC Maintainers <debian-gcc@lists.debian.org>

If you wish to submit further information on this problem, please
send it to 173513@bugs.debian.org.

Please do not send mail to owner@bugs.debian.org unless you wish
to report a problem with the Bug-tracking system.
Comment 23 owner 2010-11-10 22:36:14 UTC
Thank you for the additional information you have supplied regarding
this Bug report.

This is an automatically generated reply to let you know your message
has been received.

Your message is being forwarded to the package maintainers and other
interested parties for their attention; they will reply in due course.

Your message has been sent to the package maintainer(s):
 Debian GCC Maintainers <debian-gcc@lists.debian.org>

If you wish to submit further information on this problem, please
send it to 173513@bugs.debian.org.

Please do not send mail to owner@bugs.debian.org unless you wish
to report a problem with the Bug-tracking system.
Comment 24 owner 2010-11-10 22:39:13 UTC
Thank you for the additional information you have supplied regarding
this Bug report.

This is an automatically generated reply to let you know your message
has been received.

Your message is being forwarded to the package maintainers and other
interested parties for their attention; they will reply in due course.

Your message has been sent to the package maintainer(s):
 Debian GCC Maintainers <debian-gcc@lists.debian.org>

If you wish to submit further information on this problem, please
send it to 173513@bugs.debian.org.

Please do not send mail to owner@bugs.debian.org unless you wish
to report a problem with the Bug-tracking system.
Comment 25 owner 2010-11-10 22:39:14 UTC
Thank you for the additional information you have supplied regarding
this Bug report.

This is an automatically generated reply to let you know your message
has been received.

Your message is being forwarded to the package maintainers and other
interested parties for their attention; they will reply in due course.

Your message has been sent to the package maintainer(s):
 Debian GCC Maintainers <debian-gcc@lists.debian.org>

If you wish to submit further information on this problem, please
send it to 173513@bugs.debian.org.

Please do not send mail to owner@bugs.debian.org unless you wish
to report a problem with the Bug-tracking system.
Comment 26 Ian Lance Taylor 2010-11-10 22:49:18 UTC
See also http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=173513 .
Comment 27 Martin Jambor 2010-11-11 09:45:02 UTC
(In reply to comment #18)
> Martin,
> is this solved now?

The last time I checked it wasn't, mainly because this is not really
an IPA problem (if it was, indirect inlining would handle it just
fine).

I believe the current plan is to make early FRE fix this, see the
discussion in PR 40874.
Comment 28 Paolo Carlini 2010-11-14 17:11:21 UTC
For the record, my favorite example in the C++ library, testsuite/performance/27_io/fmtflags_manipulators.cc, pretty simple, still probably noticeable by many of our users, is by now perfectly optimized at -O2. Many thanks.
Comment 29 Michael van der Kolff 2013-01-16 12:38:41 UTC
Created attachment 29180 [details]
testcase for member function pointer that isn't inlined
Comment 30 Michael van der Kolff 2013-01-16 12:39:24 UTC
The following is currently (g++ 4.7.2) inlined:
#include <iostream>

using namespace std;

class Foo
{
public:
    void Bar() const
    {
        cout << "Howdy!" << endl;
    }
};

int main()
{
    Foo x;
    auto y = [] (const Foo& z) { z.Bar(); };
    y(x);
    return 0;
}
objdump -D:
--snip --
  400700:       48 83 ec 08             sub    $0x8,%rsp
  400704:       be 0c 09 40 00          mov    $0x40090c,%esi
  400709:       bf a0 0c 60 00          mov    $0x600ca0,%edi
  40070e:       e8 cd ff ff ff          callq  4006e0 <_ZStlsISt11char_traitsIcEERSt13basic_ostreamIcT_ES5_PKc@plt>
  400713:       48 89 c7                mov    %rax,%rdi
  400716:       e8 d5 ff ff ff          callq  4006f0 <_ZSt4endlIcSt11char_traitsIcEERSt13basic_ostreamIT_T0_ES6_@plt>
  40071b:       31 c0                   xor    %eax,%eax
  40071d:       48 83 c4 08             add    $0x8,%rsp
  400721:       c3                      retq   
  400722:       66 66 66 66 66 2e 0f    data32 data32 data32 data32 nopw %cs:0x0(%rax,%rax,1)
--snip --
whereas any version with ptr-to-mem fn is not:
...
int main()
{
    Foo x;
    auto y = &Foo::Bar;
    (x.*y)();
    return 0;
}
objdump -D:
--snip --
0000000000400830 <main>:
  400830:       48 83 ec 18             sub    $0x18,%rsp
  400834:       48 8d 7c 24 0f          lea    0xf(%rsp),%rdi
  400839:       e8 42 01 00 00          callq  400980 <_ZNK3Foo3BarEv>
  40083e:       31 c0                   xor    %eax,%eax
  400840:       48 83 c4 18             add    $0x18,%rsp
  400844:       c3                      retq   
  400845:       66 66 2e 0f 1f 84 00    data32 nopw %cs:0x0(%rax,%rax,1)
--snip --
Comment 31 Jan Hubicka 2013-01-16 14:20:46 UTC
Well, after early optimizations we get:
int main() ()
{
  struct Foo x;
  void Foo::<T392> (const struct Foo *) * iftmp.0;
  long int _3;
  long int _4;
  int (*__vtbl_ptr_type) () * _5;
  long int _6;
  sizetype _7;
  int (*__vtbl_ptr_type) () * _8;

  <bb 2>:
  _3 = (long int) Bar;
  _4 = _3 & 1;
  if (_4 == 0)
    goto <bb 4>;
  else
    goto <bb 3>;

  <bb 3>:
  _5 = MEM[(int (*__vtbl_ptr_type) () * *)&x];

that is only later, at ccp2 times turned int
int main() ()
{
  struct Foo x;
  long int _3;

  <bb 2>:
  _3 = (long int) Bar;
  Foo::Bar (&x);
  x ={v} {CLOBBER};
  return 0;

}

ccp1 s not able to do this because it still sees:
int main() ()
{
  struct
  {
    void Foo::<T392> (const struct Foo *) * __pfn;
    long int __delta;
  } y;
  struct Foo x;
  void Foo::<T392> (const struct Foo *) * iftmp.0;
  void Foo::<T392> (const struct Foo *) * _5;
  long int _6;
  long int _7;
  long int _9;
  sizetype _10;
  struct Foo * _11;
  int (*__vtbl_ptr_type) () * _12;
  void Foo::<T392> (const struct Foo *) * _13;
  long int _14;
  long int _15;
  sizetype _16;
  int (*__vtbl_ptr_type) () * _17;
  long int _19;
  sizetype _20;
  const struct Foo * _21;

  <bb 2>:
  y.__pfn = Bar;
  y.__delta = 0;
  _5 = y.__pfn;
  _6 = (long int) _5;
  _7 = _6 & 1;
  if (_7 == 0)
    goto <bb 3>;
  else
    goto <bb 4>;

This is fixed by SRA to:
  <bb 2>:
  y$__pfn_4 = Bar;
  y$__delta_3 = 0;
  _5 = y$__pfn_4;
  _6 = (long int) _5;
  _7 = _6 & 1;
  if (_7 == 0)
    goto <bb 3>;
  else
    goto <bb 4>;

  <bb 3>:
  iftmp.0_8 = y$__pfn_4;

Richi, is there chance for subsequent FRE to catch this?
Comment 32 Richard Biener 2013-01-16 14:49:31 UTC
Created attachment 29181 [details]
patch
Comment 33 Richard Biener 2013-03-18 08:53:51 UTC
Author: rguenth
Date: Mon Mar 18 08:53:42 2013
New Revision: 196771

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=196771
Log:
2013-03-18  Richard Biener  <rguenther@suse.de>

	PR tree-optimization/3713
	* tree-ssa-sccvn.c (visit_copy): Simplify.  Always propagate
	has_constants and expr.
	(stmt_has_constants): Properly valueize SSA names when deciding
	whether the stmt has constants.

	* g++.dg/ipa/devirt-12.C: New testcase.

Added:
    trunk/gcc/testsuite/g++.dg/ipa/devirt-12.C
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/testsuite/ChangeLog
    trunk/gcc/tree-ssa-sccvn.c
Comment 34 Richard Biener 2013-03-18 08:55:09 UTC
Fixed for 4.9.