diff mbox

PR middle-end/35535 part I

Message ID 20131217180028.GA21945@kam.mff.cuni.cz
State New
Headers show

Commit Message

Jan Hubicka Dec. 17, 2013, 6 p.m. UTC 
Hi,
PR 35545 has trivial testcase of feedback directed devirtualization:
int main()
{
 int i;
  A* ap = 0;

  for (i = 0; i < 10000; i++)
  {
     if (i%7==0)
        ap = new A();
     else
        ap = new B();
    ap->foo();
    ....

Here we should devirtualize since B's foo is dominating target and we correctly do so.
We however do more, we trace the code into:
  for (i = 0; i < 10000; i++)
  {
     if (i%7==0)
	{
          ap = new A();
          ap->foo();
	}
     else
	{
          ap = new B();
          ap->foo();
	}
that should allow us to devirtualize completely.  Instead of doing that we get stuck
on stupid
  <bb 5>:
  ap_8 = operator new (16);
  ap_8->i = 0;
  ap_8->_vptr.A = &MEM[(void *)&_ZTV1A + 16B];
  _19 = foo;
  PROF_26 = [obj_type_ref] OBJ_TYPE_REF(_19;(struct A)ap_8->0);
  if (PROF_26 == foo)
    goto <bb 8>;
  else
    goto <bb 7>;

  <bb 6>:
  ap_13 = operator new (16);
  MEM[(struct B *)ap_13].D.2237.i = 0;
  MEM[(struct B *)ap_13].b = 0;
  MEM[(struct B *)ap_13].D.2237._vptr.A = &MEM[(void *)&_ZTV1B + 16B];
  _1 = foo;
  PROF_30 = [obj_type_ref] OBJ_TYPE_REF(_1;(struct A)ap_13->0);
  if (PROF_30 == foo)
    goto <bb 8>;
  else
    goto <bb 7>;

There are several reasons for it
 1) most of our passes do not expect OBJ_TYPE_REF in arguments and cowardly
    consider it volatile
 2) tracer happens too late and there is no VRP pass to cleanup afterwards
 3) folding machinery expect OBJ_TYPE_REF to be only in argument.

After some consideration I decided to not update gimple_ic to remove OBJ_TYPE_REF,
since this is a perfect example where OBJ_TYPE_REF may be useful after inlining:
in a more complex cases a type based devirt may kick in and save a day even
late after unrolling/tracing and other code specialization.

This is first trivial patch to make VRP behaving correctly.
Bootstrapped/regtested x86_64-linux OK?

	* tree-vrp.c (extract_range_from_unary_expr_1): Add OBJ_TYPE_REF

Comments

Jeff Law Dec. 17, 2013, 8:56 p.m. UTC | #1 
On 12/17/13 11:00, Jan Hubicka wrote:

>
> Here we should devirtualize since B's foo is dominating target and we correctly do so.
> We however do more, we trace the code into:
>    for (i = 0; i < 10000; i++)
>    {
>       if (i%7==0)
> 	{
>            ap = new A();
>            ap->foo();
> 	}
>       else
> 	{
>            ap = new B();
>            ap->foo();
> 	}
> that should allow us to devirtualize completely.  Instead of doing that we get stuck
> on stupid
>    <bb 5>:
>    ap_8 = operator new (16);
>    ap_8->i = 0;
>    ap_8->_vptr.A = &MEM[(void *)&_ZTV1A + 16B];
>    _19 = foo;
>    PROF_26 = [obj_type_ref] OBJ_TYPE_REF(_19;(struct A)ap_8->0);
>    if (PROF_26 == foo)
>      goto <bb 8>;
>    else
>      goto <bb 7>;
>
>    <bb 6>:
>    ap_13 = operator new (16);
>    MEM[(struct B *)ap_13].D.2237.i = 0;
>    MEM[(struct B *)ap_13].b = 0;
>    MEM[(struct B *)ap_13].D.2237._vptr.A = &MEM[(void *)&_ZTV1B + 16B];
>    _1 = foo;
>    PROF_30 = [obj_type_ref] OBJ_TYPE_REF(_1;(struct A)ap_13->0);
>    if (PROF_30 == foo)
>      goto <bb 8>;
>    else
>      goto <bb 7>;
>
> There are several reasons for it
>   1) most of our passes do not expect OBJ_TYPE_REF in arguments and cowardly
>      consider it volatile
>   2) tracer happens too late and there is no VRP pass to cleanup afterwards
>   3) folding machinery expect OBJ_TYPE_REF to be only in argument.
>
> After some consideration I decided to not update gimple_ic to remove OBJ_TYPE_REF,
> since this is a perfect example where OBJ_TYPE_REF may be useful after inlining:
> in a more complex cases a type based devirt may kick in and save a day even
> late after unrolling/tracing and other code specialization.
>
> This is first trivial patch to make VRP behaving correctly.
> Bootstrapped/regtested x86_64-linux OK?
>
> 	* tree-vrp.c (extract_range_from_unary_expr_1): Add OBJ_TYPE_REF
s/Add/Handle.  Please add the PR marker as well.

OK with that trivial nit.

jeff
Tobias Burnus Dec. 18, 2013, 6:53 a.m. UTC | #2 
Am 17.12.2013 21:56, schrieb Jeff Law:
>>     * tree-vrp.c (extract_range_from_unary_expr_1): Add OBJ_TYPE_REF
> s/Add/Handle.  Please add the PR marker as well.
>
> OK with that trivial nit.

And the proper PR. I don't think that INVALID C++ PR is the PR you want 
to refer to.

Tobias
Jeff Law Dec. 18, 2013, 6:53 a.m. UTC | #3 
On 12/17/13 23:53, Tobias Burnus wrote:
> Am 17.12.2013 21:56, schrieb Jeff Law:
>>>     * tree-vrp.c (extract_range_from_unary_expr_1): Add OBJ_TYPE_REF
>> s/Add/Handle.  Please add the PR marker as well.
>>
>> OK with that trivial nit.
>
> And the proper PR. I don't think that INVALID C++ PR is the PR you want
> to refer to.
Yea, I mentioned that for the part II patch.  The right number is 35545 
I think.

jeff
Jan Hubicka Dec. 18, 2013, 9:12 a.m. UTC | #4 
> On 12/17/13 23:53, Tobias Burnus wrote:
> >Am 17.12.2013 21:56, schrieb Jeff Law:
> >>>    * tree-vrp.c (extract_range_from_unary_expr_1): Add OBJ_TYPE_REF
> >>s/Add/Handle.  Please add the PR marker as well.
> >>
> >>OK with that trivial nit.
> >
> >And the proper PR. I don't think that INVALID C++ PR is the PR you want
> >to refer to.
> Yea, I mentioned that for the part II patch.  The right number is
> 35545 I think.

I see, now I understand the comment.  I fixed the changelog.

Honza
> 
> jeff
diff mbox

Patch

Index: tree-vrp.c
===================================================================
--- tree-vrp.c	(revision 206040)
+++ tree-vrp.c	(working copy)
@@ -3202,9 +3202,9 @@  extract_range_from_unary_expr_1 (value_r
     }
 
   /* Handle operations that we express in terms of others.  */
-  if (code == PAREN_EXPR)
+  if (code == PAREN_EXPR || code == OBJ_TYPE_REF)
     {
-      /* PAREN_EXPR is a simple copy.  */
+      /* PAREN_EXPR and OBJ_TYPE_REF are simple copies.  */
       copy_value_range (vr, &vr0);
       return;
     }