[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