Better comparison of BINFOs in IPA-CP

Hi,

the patch below improves the comparisons of BINFOs in IPA-CP.  The
problem is that we can read different BINFOs for the same type (or a
base type component) from the LTO summaries because BINFOs coming from

On Sun, Sep 4, 2011 at 12:39 AM, Martin Jambor <mjambor@suse.cz> wrote:
> Hi,
>
> the patch below improves the comparisons of BINFOs in IPA-CP.  The
> problem is that we can read different BINFOs for the same type (or a
> base type component) from the LTO summaries because BINFOs coming from
> different compilation units are not unified.  Because we now have the
> BINFO_VTABLE available, we can compare those instead since the VMT
> variables are unified.
>
> Bootstrapped and tested on x86_64-linux, also tested by LTO-building
> Firefox and 483.xalancbmk on the same platform (I lost the actual
> numbers but the new test returned true hundreds of times in both
> these cases).  OK for trunk?

Ok.

Thanks,
Richard.

> Thanks,
>
> Martin
>
>
> 2011-09-02  Martin Jambor  <mjambor@suse.cz>
>
>        * ipa-cp.c (values_equal_for_ipcp_p): When comparing BINFOs, compare
>        their BINFO_VTABLE,
>
> Index: src/gcc/ipa-cp.c
> ===================================================================
> --- src.orig/gcc/ipa-cp.c
> +++ src/gcc/ipa-cp.c
> @@ -800,6 +800,33 @@ values_equal_for_ipcp_p (tree x, tree y)
>   if (x == y)
>     return true;
>
> +  if (TREE_CODE (x) == TREE_BINFO && TREE_CODE (y) == TREE_BINFO)
> +    {
> +      unsigned HOST_WIDE_INT ox, oy;
> +      tree vx = BINFO_VTABLE (x);
> +      tree vy = BINFO_VTABLE (y);
> +
> +      if (!vx || !vy
> +         || TREE_CODE (vx) != POINTER_PLUS_EXPR
> +         || TREE_CODE (vy) != POINTER_PLUS_EXPR)
> +       return false;
> +
> +      ox = tree_low_cst (TREE_OPERAND (vx, 1), 1);
> +      oy = tree_low_cst (TREE_OPERAND (vx, 1), 1);
> +
> +      if (ox != oy)
> +       return false;
> +
> +      vx = TREE_OPERAND (vx, 0);
> +      vy = TREE_OPERAND (vy, 0);
> +      if (TREE_CODE (vx) != ADDR_EXPR
> +         || TREE_CODE (vy) != ADDR_EXPR)
> +       return false;
> +
> +      if (TREE_OPERAND (vx, 0) == TREE_OPERAND (vy, 0))
> +       return true;
> +    }
> +
>   if (TREE_CODE (x) == TREE_BINFO || TREE_CODE (y) == TREE_BINFO)
>     return false;
>
>

> Hi,
> 
> the patch below improves the comparisons of BINFOs in IPA-CP.  The
> problem is that we can read different BINFOs for the same type (or a
> base type component) from the LTO summaries because BINFOs coming from
> different compilation units are not unified.  Because we now have the
> BINFO_VTABLE available, we can compare those instead since the VMT
> variables are unified.

Hmm, I was quite sure this is happening.  Can't you get an extra credit for unifying
the binfos at stream in?

Honza
> 
> Bootstrapped and tested on x86_64-linux, also tested by LTO-building
> Firefox and 483.xalancbmk on the same platform (I lost the actual
> numbers but the new test returned true hundreds of times in both
> these cases).  OK for trunk?
> 
> Thanks,
> 
> Martin
> 
> 
> 2011-09-02  Martin Jambor  <mjambor@suse.cz>
> 
> 	* ipa-cp.c (values_equal_for_ipcp_p): When comparing BINFOs, compare
> 	their BINFO_VTABLE,
> 
> Index: src/gcc/ipa-cp.c
> ===================================================================
> --- src.orig/gcc/ipa-cp.c
> +++ src/gcc/ipa-cp.c
> @@ -800,6 +800,33 @@ values_equal_for_ipcp_p (tree x, tree y)
>    if (x == y)
>      return true;
>  
> +  if (TREE_CODE (x) == TREE_BINFO && TREE_CODE (y) == TREE_BINFO)
> +    {
> +      unsigned HOST_WIDE_INT ox, oy;
> +      tree vx = BINFO_VTABLE (x);
> +      tree vy = BINFO_VTABLE (y);
> +
> +      if (!vx || !vy
> +	  || TREE_CODE (vx) != POINTER_PLUS_EXPR
> +	  || TREE_CODE (vy) != POINTER_PLUS_EXPR)
> +	return false;
> +
> +      ox = tree_low_cst (TREE_OPERAND (vx, 1), 1);
> +      oy = tree_low_cst (TREE_OPERAND (vx, 1), 1);
> +
> +      if (ox != oy)
> +	return false;
> +
> +      vx = TREE_OPERAND (vx, 0);
> +      vy = TREE_OPERAND (vy, 0);
> +      if (TREE_CODE (vx) != ADDR_EXPR
> +	  || TREE_CODE (vy) != ADDR_EXPR)
> +	return false;
> +
> +      if (TREE_OPERAND (vx, 0) == TREE_OPERAND (vy, 0))
> +	return true;
> +    }
> +
>    if (TREE_CODE (x) == TREE_BINFO || TREE_CODE (y) == TREE_BINFO)
>      return false;
>

Hi,

On Mon, Sep 05, 2011 at 09:24:36AM +0200, Jan Hubicka wrote:
> > Hi,
> > 
> > the patch below improves the comparisons of BINFOs in IPA-CP.  The
> > problem is that we can read different BINFOs for the same type (or a
> > base type component) from the LTO summaries because BINFOs coming from
> > different compilation units are not unified.  Because we now have the
> > BINFO_VTABLE available, we can compare those instead since the VMT
> > variables are unified.
> 
> Hmm, I was quite sure this is happening.  Can't you get an extra credit for unifying
> the binfos at stream in?
> 

Unifying them (especially BASE_BINFOs) in a way that we end up with an
appropriate BINFO attached to the prevailing type is not trivial and I
really would like to avoid writing that.

On the other hand, we basically use BINFOs as a mapping from the
encapsulating type and offset into virtual table expressions and so
what we can do is stream the encapsulating type and offset in jump
functions and at the WPA stage use the two to get the BINFO (now
unique because of type unification) and work with that.

So I decided to implement the above and not to commit this patch
(I'd only consider it if I did not manage to have the proper solution
ready by the end of stage1).

Thanks,

Martin



> Honza
> > 
> > Bootstrapped and tested on x86_64-linux, also tested by LTO-building
> > Firefox and 483.xalancbmk on the same platform (I lost the actual
> > numbers but the new test returned true hundreds of times in both
> > these cases).  OK for trunk?
> > 
> > Thanks,
> > 
> > Martin
> > 
> > 
> > 2011-09-02  Martin Jambor  <mjambor@suse.cz>
> > 
> > 	* ipa-cp.c (values_equal_for_ipcp_p): When comparing BINFOs, compare
> > 	their BINFO_VTABLE,
> > 
> > Index: src/gcc/ipa-cp.c
> > ===================================================================
> > --- src.orig/gcc/ipa-cp.c
> > +++ src/gcc/ipa-cp.c
> > @@ -800,6 +800,33 @@ values_equal_for_ipcp_p (tree x, tree y)
> >    if (x == y)
> >      return true;
> >  
> > +  if (TREE_CODE (x) == TREE_BINFO && TREE_CODE (y) == TREE_BINFO)
> > +    {
> > +      unsigned HOST_WIDE_INT ox, oy;
> > +      tree vx = BINFO_VTABLE (x);
> > +      tree vy = BINFO_VTABLE (y);
> > +
> > +      if (!vx || !vy
> > +	  || TREE_CODE (vx) != POINTER_PLUS_EXPR
> > +	  || TREE_CODE (vy) != POINTER_PLUS_EXPR)
> > +	return false;
> > +
> > +      ox = tree_low_cst (TREE_OPERAND (vx, 1), 1);
> > +      oy = tree_low_cst (TREE_OPERAND (vx, 1), 1);
> > +
> > +      if (ox != oy)
> > +	return false;
> > +
> > +      vx = TREE_OPERAND (vx, 0);
> > +      vy = TREE_OPERAND (vy, 0);
> > +      if (TREE_CODE (vx) != ADDR_EXPR
> > +	  || TREE_CODE (vy) != ADDR_EXPR)
> > +	return false;
> > +
> > +      if (TREE_OPERAND (vx, 0) == TREE_OPERAND (vy, 0))
> > +	return true;
> > +    }
> > +
> >    if (TREE_CODE (x) == TREE_BINFO || TREE_CODE (y) == TREE_BINFO)
> >      return false;
> >