[Bug middle-end/35560] New: Missing CSE/PRE for memory operations involved in virtual call.

[Bug middle-end/35560] New: Missing CSE/PRE for memory operations involved in virtual call.

[xinliangli at gmail dot com]

In the following example, three loads to the vtable pointer value can be CSEed,
so are two accesses to the same vtable entry (for foo), but final assembly
dumping indicate otherwise.  It is true that the object pointed to by ap may be
modified by the calls, but this does not apply to the vptr field.

class A {

public:

virtual int foo(int i);
virtual int bar(int i);

private:
int a;
};

int test(A* ap, int i)
{

   int r1 = ap->foo(i);
   int r2 = ap->bar(i);

   return r1+r2 + ap->foo(i);
}
     ...
.LCFI4:
        movq    (%rdi), %rax
        movl    %esi, %r12d
        call    *(%rax)
        movl    %eax, %r13d
        movq    (%rbx), %rax   <----- redundant
        movl    %r12d, %esi
        movq    %rbx, %rdi
        call    *8(%rax)
        movl    %eax, %ebp
        movq    (%rbx), %rax   <---- redundant
        movl    %r12d, %esi
        movq    %rbx, %rdi
        call    *(%rax)             <-- *(%rax) is redundant
        leal    (%rbp,%r13), %edx
        movq    8(%rsp), %rbx
        movq    16(%rsp), %rbp
        movq    24(%rsp), %r12
        movq    32(%rsp), %r13
        addq    $40, %rsp
        leal    (%rdx,%rax), %eax
        ret


-- 
           Summary: Missing CSE/PRE for memory operations involved in
                    virtual call.
           Product: gcc
           Version: 4.4.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: middle-end
        AssignedTo: unassigned at gcc dot gnu dot org
        ReportedBy: xinliangli at gmail dot com


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=35560

[Bug middle-end/35560] Missing CSE/PRE for memory operations involved in virtual call.

[pinskia at gcc dot gnu dot org]

-- 

pinskia at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Severity|normal                      |enhancement


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=35560

[Bug middle-end/35560] Missing CSE/PRE for memory operations involved in virtual call.

[steven at gcc dot gnu dot org]

------- Comment #1 from steven at gcc dot gnu dot org  2008-11-29 22:42 -------
Looks like something alias related - so CCing Richi.


-- 

steven at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |rguenth at gcc dot gnu dot
                   |                            |org
             Status|UNCONFIRMED                 |NEW
     Ever Confirmed|0                           |1
   Last reconfirmed|0000-00-00 00:00:00         |2008-11-29 22:42:58
               date|                            |


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=35560

[Bug middle-end/35560] Missing CSE/PRE for memory operations involved in virtual call.

[rguenth at gcc dot gnu dot org]

------- Comment #2 from rguenth at gcc dot gnu dot org  2008-11-30 11:56 -------
Well, the middle-end thinks that *ap is clobbered by the calls to foo/bar
(which it obviously is).  The middle-end has no idea that the _vptr member
of *ap is special and not clobbered (is it?).  We do not have a suitable way
to represent this either.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=35560

[Bug middle-end/35560] Missing CSE/PRE for memory operations involved in virtual call.

[rguenth at gcc dot gnu dot org]

------- Comment #3 from rguenth at gcc dot gnu dot org  2008-11-30 12:09 -------
Note that we also do not CSE OBJ_TYPE_REF trees which seem to be valid gimple
in place of the call in GIMPLE_CALLs so they do not get a separate
value-number.
(not that we would do anything useful with them if they were standalone,
testcase for that welcome)


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=35560

Re: [Bug middle-end/35560] Missing CSE/PRE for memory operations involved in virtual call.

[Andrew Thomas Pinski]

I think this bug is invalid since the type can change via a placement  
new which will change the vtable.

Sent from my iPhone

On Nov 30, 2008, at 4:09 AM, "rguenth at gcc dot gnu dot org" <gcc-bugzilla@gcc.gnu.org 
 > wrote:

>
>
> ------- Comment #3 from rguenth at gcc dot gnu dot org  2008-11-30  
> 12:09 -------
> Note that we also do not CSE OBJ_TYPE_REF trees which seem to be  
> valid gimple
> in place of the call in GIMPLE_CALLs so they do not get a separate
> value-number.
> (not that we would do anything useful with them if they were  
> standalone,
> testcase for that welcome)
>
>
> -- 
>
>
> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=35560
>

[Bug middle-end/35560] Missing CSE/PRE for memory operations involved in virtual call.

[pinskia at gmail dot com]

------- Comment #4 from pinskia at gmail dot com  2008-11-30 19:29 -------
Subject: Re:  Missing CSE/PRE for memory operations involved in virtual call.

I think this bug is invalid since the type can change via a placement  
new which will change the vtable.

Sent from my iPhone

On Nov 30, 2008, at 4:09 AM, "rguenth at gcc dot gnu dot org"
<gcc-bugzilla@gcc.gnu.org 
 > wrote:

>
>
> ------- Comment #3 from rguenth at gcc dot gnu dot org  2008-11-30  
> 12:09 -------
> Note that we also do not CSE OBJ_TYPE_REF trees which seem to be  
> valid gimple
> in place of the call in GIMPLE_CALLs so they do not get a separate
> value-number.
> (not that we would do anything useful with them if they were  
> standalone,
> testcase for that welcome)
>
>
> -- 
>
>
> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=35560
>


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=35560

[Bug middle-end/35560] Missing CSE/PRE for memory operations involved in virtual call.

[rguenth at gcc dot gnu dot org]

------- Comment #5 from rguenth at gcc dot gnu dot org  2008-11-30 19:38 -------
Yeah, that definitely complicates matters.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=35560

[Bug middle-end/35560] Missing CSE/PRE for memory operations involved in virtual call.

[davidxl at gcc dot gnu dot org]

------- Comment #6 from davidxl at gcc dot gnu dot org  2010-02-03 18:30 -------
See discussions in http://gcc.gnu.org/ml/gcc-patches/2010-02/msg00138.html
about changing dynamic types using placement new -- it is basically not allowed
-- so the optimization is valid.


David


-- 

davidxl at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         AssignedTo|unassigned at gcc dot gnu   |davidxl at gcc dot gnu dot
                   |dot org                     |org
             Status|NEW                         |ASSIGNED
   Last reconfirmed|2008-11-29 22:42:58         |2010-02-03 18:30:00
               date|                            |


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=35560

[Bug middle-end/35560] Missing CSE/PRE for memory operations involved in virtual call.

[rguenth at gcc dot gnu dot org]

------- Comment #7 from rguenth at gcc dot gnu dot org  2010-02-03 21:24 -------
It is valid to use placement new to construct a more or less derived type
which would change the vtable pointer.

Thus I think this bug is still invalid.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=35560

[Bug middle-end/35560] Missing CSE/PRE for memory operations involved in virtual call.

[davidxl at gcc dot gnu dot org]

------- Comment #8 from davidxl at gcc dot gnu dot org  2010-02-03 21:44 -------
(In reply to comment #7)
> It is valid to use placement new to construct a more or less derived type
> which would change the vtable pointer.
> 
> Thus I think this bug is still invalid.
> 

How did you reach this conclusion from reading p7 of 3.8 in the standard?
"The original object was a most derived object of type T and the new object is
a most derived object of type T...."

The following is allowed:
class B {
 virtual ...
};

class D : public B {
  ...
};

B* bp = new D ();
...

new (bp) D();

but vptr does not change.


Set aside the standard -- this optimization is useful regardless. Some of the
develpoers are so desperate that they manually do LICM of vptr and vtbl access
for vcalls in the loop.  The worst case is to use a option to guard it (which I
think the default should be on).

David


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=35560

[Bug middle-end/35560] Missing CSE/PRE for memory operations involved in virtual call.

[steven at gcc dot gnu dot org]

------- Comment #9 from steven at gcc dot gnu dot org  2010-02-03 21:49 -------
Ah, "Set aside the standard". Another user who wants to make up his own
semantics for a standardized language. No, no, and damn no.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=35560

[Bug middle-end/35560] Missing CSE/PRE for memory operations involved in virtual call.

[rguenth at gcc dot gnu dot org]

------- Comment #10 from rguenth at gcc dot gnu dot org  2010-02-03 21:54 -------
Oh, I wonder anyway how you are going to implement this "vtable pointers are
special" thing in the alias machinery.  Good luck - and please keep LTO
in mind ;)


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=35560

[Bug middle-end/35560] Missing CSE/PRE for memory operations involved in virtual call.

[davidxl at gcc dot gnu dot org]

------- Comment #11 from davidxl at gcc dot gnu dot org  2010-02-03 21:55 -------
(In reply to comment #9)
> Ah, "Set aside the standard". Another user who wants to make up his own
> semantics for a standardized language. No, no, and damn no.
> 


Of course, things like this can be brought up to the language committee as long
as it is 1) not ambiguous 2) and generally useful.

(In terms of optimization related semantics (type aliasing, restrict etc), I am
not sure how standard it actually is given the ambiguity here and there.)

David


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=35560

[Bug middle-end/35560] Missing CSE/PRE for memory operations involved in virtual call.

[rguenth at gcc dot gnu dot org]

------- Comment #12 from rguenth at gcc dot gnu dot org  2010-02-03 21:55 -------
Btw, a destructor call also changes the vtbl pointer.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=35560

[Bug middle-end/35560] Missing CSE/PRE for memory operations involved in virtual call.

[davidxl at gcc dot gnu dot org]

------- Comment #13 from davidxl at gcc dot gnu dot org  2010-02-03 22:05 -------
(In reply to comment #12)
> Btw, a destructor call also changes the vtbl pointer.
> 

ctors, dtors, wrapper function calls etc are all handled. Detailed write up
will be available at some point. To put it a simple way, it is done via live
across analyis: if an poly object is referenced before and after a call
(accesses to any field of it) both available and anticipated from a a call --
it is live across the call -- vptr field won't be modified by the call. 
Partially anticipated case is also handled.  Once vptr is handled, vtbl access
follows automatically -- at vtbls are RO. vptr assignment is treated
conservatively.

I implemented this thing in 4.4 line using special shadow symbols and
VUSE/VDEFS. It works as expected except that SCCVN time went to hell. Simple
fix to collapse varying defs in DFS walk help a lot but still slow. Need to do
this using alias oracle.

David


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=35560

[Bug middle-end/35560] Missing CSE/PRE for memory operations involved in virtual call.

[witold.baryluk+gcc at gmail dot com]

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

Witold Baryluk <witold.baryluk+gcc at gmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |witold.baryluk+gcc at gmail dot co
                   |                            |m

--- Comment #15 from Witold Baryluk <witold.baryluk+gcc at gmail dot com> ---
I know this is a pretty old bug, but I was exploring some assembly of gcc and
clang on godbolt, and also stumbled into same issue.

https://godbolt.org/z/qPzMhWse1

class A {
public:
    virtual int f7(int x) const;
};

int g(const A * const a, int x) {
    int r = 0;
    for (int i = 0; i < 10000; i++)
        r += a->f7(x);
    return r;
}

(same happens without loop, when just calling a->f7 multiple times)



g(A const*, int):
        push    r13
        mov     r13d, esi
        push    r12
        xor     r12d, r12d
        push    rbp
        mov     rbp, rdi
        push    rbx
        mov     ebx, 10000
        sub     rsp, 8
.L2:
        mov     rax, QWORD PTR [rbp+0]       # a vtable deref
        mov     esi, r13d
        mov     rdi, rbp
        call    [QWORD PTR [rax]]            # f7 indirect call
        add     r12d, eax
        dec     ebx
        jne     .L2

        add     rsp, 8
        pop     rbx
        pop     rbp
        mov     eax, r12d
        pop     r12
        pop     r13
        ret


I was expecting  mov     rax, QWORD PTR [rbp+0] and call    [QWORD PTR [rax]],
to be hoisted out of the loop (call converted to lea, and call register).


A bit sad.

Is there some recent work done on this optimization?

Are there at least some cases where it is valid to do CSE, or change code so it
is moved out of the loop?

[Bug middle-end/35560] Missing CSE/PRE for memory operations involved in virtual call.

[rguenth at gcc dot gnu.org]

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

--- Comment #16 from Richard Biener <rguenth at gcc dot gnu.org> ---
(In reply to Witold Baryluk from comment #15)
> I know this is a pretty old bug, but I was exploring some assembly of gcc
> and clang on godbolt, and also stumbled into same issue.
> 
> https://godbolt.org/z/qPzMhWse1
> 
> class A {
> public:
>     virtual int f7(int x) const;
> };
> 
> int g(const A * const a, int x) {
>     int r = 0;
>     for (int i = 0; i < 10000; i++)
>         r += a->f7(x);
>     return r;
> }
> 
> (same happens without loop, when just calling a->f7 multiple times)
> 
> 
> 
> g(A const*, int):
>         push    r13
>         mov     r13d, esi
>         push    r12
>         xor     r12d, r12d
>         push    rbp
>         mov     rbp, rdi
>         push    rbx
>         mov     ebx, 10000
>         sub     rsp, 8
> .L2:
>         mov     rax, QWORD PTR [rbp+0]       # a vtable deref
>         mov     esi, r13d
>         mov     rdi, rbp
>         call    [QWORD PTR [rax]]            # f7 indirect call
>         add     r12d, eax
>         dec     ebx
>         jne     .L2
> 
>         add     rsp, 8
>         pop     rbx
>         pop     rbp
>         mov     eax, r12d
>         pop     r12
>         pop     r13
>         ret
> 
> 
> I was expecting  mov     rax, QWORD PTR [rbp+0] and call    [QWORD PTR
> [rax]], to be hoisted out of the loop (call converted to lea, and call
> register).
> 
> 
> A bit sad.
> 
> Is there some recent work done on this optimization?
> 
> Are there at least some cases where it is valid to do CSE, or change code so
> it is moved out of the loop?

GCC sees a->f() as possibly altering the virtual table [pointer] since
the function gets passed 'a' and thus a pointer to it (and *a is global
memory anyway, so GCC has to assume f() has access to it).

In C++ probably there's probably no virtual function that could do this.
A virtual DTOR would leave an uninitialized object.  Not sure if

class A
{
  virtual void f() { }
}
class B : A
{
  virtual void f() { new A (this); }
}

would be valid (maybe with first calling the DTOR on the existing object).

[Bug middle-end/35560] Missing CSE/PRE for memory operations involved in virtual call.

[m.cencora at gmail dot com]

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

m.cencora at gmail dot com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |m.cencora at gmail dot com

--- Comment #17 from m.cencora at gmail dot com ---
It is (In reply to Richard Biener from comment #16)
> (In reply to Witold Baryluk from comment #15)
> > I know this is a pretty old bug, but I was exploring some assembly of gcc
> > and clang on godbolt, and also stumbled into same issue.
> > 
> > https://godbolt.org/z/qPzMhWse1
> > 
> > class A {
> > public:
> >     virtual int f7(int x) const;
> > };
> > 
> > int g(const A * const a, int x) {
> >     int r = 0;
> >     for (int i = 0; i < 10000; i++)
> >         r += a->f7(x);
> >     return r;
> > }
> > 
> > (same happens without loop, when just calling a->f7 multiple times)
> > 
> > 
> > 
> > g(A const*, int):
> >         push    r13
> >         mov     r13d, esi
> >         push    r12
> >         xor     r12d, r12d
> >         push    rbp
> >         mov     rbp, rdi
> >         push    rbx
> >         mov     ebx, 10000
> >         sub     rsp, 8
> > .L2:
> >         mov     rax, QWORD PTR [rbp+0]       # a vtable deref
> >         mov     esi, r13d
> >         mov     rdi, rbp
> >         call    [QWORD PTR [rax]]            # f7 indirect call
> >         add     r12d, eax
> >         dec     ebx
> >         jne     .L2
> > 
> >         add     rsp, 8
> >         pop     rbx
> >         pop     rbp
> >         mov     eax, r12d
> >         pop     r12
> >         pop     r13
> >         ret
> > 
> > 
> > I was expecting  mov     rax, QWORD PTR [rbp+0] and call    [QWORD PTR
> > [rax]], to be hoisted out of the loop (call converted to lea, and call
> > register).
> > 
> > 
> > A bit sad.
> > 
> > Is there some recent work done on this optimization?
> > 
> > Are there at least some cases where it is valid to do CSE, or change code so
> > it is moved out of the loop?
> 
> GCC sees a->f() as possibly altering the virtual table [pointer] since
> the function gets passed 'a' and thus a pointer to it (and *a is global
> memory anyway, so GCC has to assume f() has access to it).
> 
> In C++ probably there's probably no virtual function that could do this.
> A virtual DTOR would leave an uninitialized object.  Not sure if
> 
> class A
> {
>   virtual void f() { }
> }
> class B : A
> {
>   virtual void f() { new A (this); }
> }
> 
> would be valid (maybe with first calling the DTOR on the existing object).

It is valid, but to be able to use B object after B::f was invoked (and a new
object is placed there instead), a user need to launder the pointer.
https://en.cppreference.com/w/cpp/utility/launder