[Bug c++/77896] Object vtable lookups are not hoisted out of loops

[scovich at gmail dot com]

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

--- Comment #1 from Ryan Johnson  ---
It appears that multiple calls to different virtual functions of the same
object are not optimized, either (each performs the same load-load-jump
operation).

[Bug c++/77896] Object vtable lookups are not hoisted out of loops

[pinskia at gcc dot gnu.org]

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

--- Comment #2 from Andrew Pinski  ---
(In reply to Ryan Johnson from comment #1)
> Given that an object's vtable is fixed over its lifetime,

Yes but functions including virtual functions can deconstruct the object and
then call inplacement new on the same object and change what the type is for
that object.  So saying the object's vtable is fixed is correct but the
lifetime of the object is not fixed.

[Bug c++/77896] Object vtable lookups are not hoisted out of loops

[pinskia at gcc dot gnu.org]

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

--- Comment #3 from Andrew Pinski  ---
Basically what I am trying to say is there is a lot of analysis needed to
determine if the lifetime of object does not end during a call to a virtual
function.

> Altering the test case to trigger speculative devirtualization as follows:

This is a dup of another bug which says once it has been speculative to a
specific function which does not change the lifetime of the object, we can
split the loop.  That is a dup of bug 67886 really.

[Bug c++/77896] Object vtable lookups are not hoisted out of loops

[scovich at gmail dot com]

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

--- Comment #4 from Ryan Johnson  ---
Yikes. That explains it, all right. 

I never would have thought of an object destroying itself and changing its type
with placement new... I guess it must be subject to the same restrictions as
`delete this` [1], because things don't turn out well if the compiler thinks it
knows the type of the object:

 alter.cpp ===
#include 
#include 

struct AlterEgo {
   virtual ~AlterEgo() { }
   virtual void toggle()=0;
};

struct Jekyl : AlterEgo {
   ~Jekyl() { puts("~Jekyl"); }
   void toggle();
};

struct Hyde : AlterEgo {
   ~Hyde() { puts("~Hyde"); }
   void toggle();
};

void Jekyl::toggle()
{ this->~AlterEgo(); new (this) Hyde; }
void Hyde::toggle()
{ this->~AlterEgo(); new (this) Jekyl; }

void whatami(AlterEgo* x)
{
   printf("Jekyl? %p\n", dynamic_cast(x));
   x->toggle();
   printf("Jekyl? %p\n", dynamic_cast(x));
}

int main()
{
   puts("\nWorks ok-ish:");
   Jekyl* x = new Jekyl;
   whatami(x);
   puts("\nJekyl?");
   delete x;

   puts("\nBad idea:");
   Jekyl j;
   j.toggle();
   j.toggle();
   whatami(&j);

   puts("\nJekyl?");
}


$ g++ -Wall alter.cpp && ./a.out

Works ok-ish:
Jekyl? 0x6000104c0
~Jekyl
Jekyl? 0x0

Jekyl?
~Hyde

Bad idea:
~Jekyl
~Hyde
Jekyl? 0x0
~Hyde
Jekyl? 0xcbf0

Jekyl?
~Jekyl

[1] https://isocpp.org/wiki/faq/freestore-mgmt#delete-this

[Bug c++/77896] Object vtable lookups are not hoisted out of loops

[scovich at gmail dot com]

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

--- Comment #5 from Ryan Johnson  ---
In an ideal world, C++ would disallow such behavior by default, with a function
attribute of some kind that flags cases where a type change might occur (kind
of like how c++11 assumes `nothrow()` for destructors unless you specify
otherwise). Not only would it allow better optimizations, it would be safer,
because the compiler could then detect and forbid (or at least warn about)
problematic usage of such a class (like stack-allocating it, or calling a
type-change function when cast as the type that's about to change).

[Bug c++/77896] Object vtable lookups are not hoisted out of loops

[redi at gcc dot gnu.org]

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

--- Comment #6 from Jonathan Wakely  ---
(In reply to Ryan Johnson from comment #4)
>puts("\nWorks ok-ish:");
>Jekyl* x = new Jekyl;
>whatami(x);
>puts("\nJekyl?");
>delete x;

I think this would be OK if it did "delete (AlterEgo*)x" ... as written, I'm
not sure.

>puts("\nBad idea:");
>Jekyl j;
>j.toggle();
>j.toggle();
>whatami(&j);

This is undefined, because ~Jekyl is going to run on the stack object at the
end of the scope, but it's not a Jekyl at that point.

The bound member functions extension can be used to avoid repeated vtable
lookups for the same function:
https://gcc.gnu.org/onlinedocs/gcc/Bound-member-functions.html

[Bug c++/77896] Object vtable lookups are not hoisted out of loops

[rguenth at gcc dot gnu.org]

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

Richard Biener  changed:

   What|Removed |Added

 Status|UNCONFIRMED |RESOLVED
 Resolution|--- |INVALID

--- Comment #7 from Richard Biener  ---
Thus really invalid.