[PATCH] Handle OBJ_TYPE_REF in FRE

[PATCH] Handle OBJ_TYPE_REF in FRE

[Richard Biener]

The following patch handles CSEing OBJ_TYPE_REF which was omitted
because it is a GENERIC expression even on GIMPLE (for whatever
reason...).  Rather than changing this now the following patch
simply treats it properly as such.

Bootstrap & regtest running on x86_64-unknown-linux-gnu.

Note that this does not (yet) substitute OBJ_TYPE_REFs in calls
with SSA names that have the same value - not sure if that would
be desired generally (does the devirt machinery cope with that?).

Thanks,
Richard.

2015-12-03  Richard Biener  <rguenther@suse.de>

	PR tree-optimization/64812
	* tree-ssa-sccvn.c (vn_get_stmt_kind): Handle OBJ_TYPE_REF.
	(vn_nary_length_from_stmt): Likewise.
	(init_vn_nary_op_from_stmt): Likewise.
	* gimple-match-head.c (maybe_build_generic_op): Likewise.
	* gimple-pretty-print.c (dump_unary_rhs): Likewise.

	* g++.dg/tree-ssa/ssa-fre-1.C: New testcase.

Index: gcc/tree-ssa-sccvn.c
===================================================================
*** gcc/tree-ssa-sccvn.c	(revision 231221)
--- gcc/tree-ssa-sccvn.c	(working copy)
*************** vn_get_stmt_kind (gimple *stmt)
*** 460,465 ****
--- 460,467 ----
  			  ? VN_CONSTANT : VN_REFERENCE);
  		else if (code == CONSTRUCTOR)
  		  return VN_NARY;
+ 		else if (code == OBJ_TYPE_REF)
+ 		  return VN_NARY;
  		return VN_NONE;
  	      }
  	  default:
*************** vn_nary_length_from_stmt (gimple *stmt)
*** 2479,2484 ****
--- 2481,2487 ----
        return 1;
  
      case BIT_FIELD_REF:
+     case OBJ_TYPE_REF:
        return 3;
  
      case CONSTRUCTOR:
*************** init_vn_nary_op_from_stmt (vn_nary_op_t
*** 2508,2513 ****
--- 2511,2517 ----
        break;
  
      case BIT_FIELD_REF:
+     case OBJ_TYPE_REF:
        vno->length = 3;
        vno->op[0] = TREE_OPERAND (gimple_assign_rhs1 (stmt), 0);
        vno->op[1] = TREE_OPERAND (gimple_assign_rhs1 (stmt), 1);
Index: gcc/gimple-match-head.c
===================================================================
*** gcc/gimple-match-head.c	(revision 231221)
--- gcc/gimple-match-head.c	(working copy)
*************** maybe_build_generic_op (enum tree_code c
*** 243,248 ****
--- 243,249 ----
        *op0 = build1 (code, type, *op0);
        break;
      case BIT_FIELD_REF:
+     case OBJ_TYPE_REF:
        *op0 = build3 (code, type, *op0, op1, op2);
        break;
      default:;
Index: gcc/gimple-pretty-print.c
===================================================================
*** gcc/gimple-pretty-print.c	(revision 231221)
--- gcc/gimple-pretty-print.c	(working copy)
*************** dump_unary_rhs (pretty_printer *buffer,
*** 302,308 ****
  	  || TREE_CODE_CLASS (rhs_code) == tcc_reference
  	  || rhs_code == SSA_NAME
  	  || rhs_code == ADDR_EXPR
! 	  || rhs_code == CONSTRUCTOR)
  	{
  	  dump_generic_node (buffer, rhs, spc, flags, false);
  	  break;
--- 302,309 ----
  	  || TREE_CODE_CLASS (rhs_code) == tcc_reference
  	  || rhs_code == SSA_NAME
  	  || rhs_code == ADDR_EXPR
! 	  || rhs_code == CONSTRUCTOR
! 	  || rhs_code == OBJ_TYPE_REF)
  	{
  	  dump_generic_node (buffer, rhs, spc, flags, false);
  	  break;
Index: gcc/testsuite/g++.dg/tree-ssa/ssa-fre-1.C
===================================================================
*** gcc/testsuite/g++.dg/tree-ssa/ssa-fre-1.C	(revision 0)
--- gcc/testsuite/g++.dg/tree-ssa/ssa-fre-1.C	(working copy)
***************
*** 0 ****
--- 1,44 ----
+ /* { dg-do compile } */
+ /* { dg-options "-O2 -fdump-tree-fre2" } */
+ 
+ template <class T> class A
+ {
+   T *p;
+ 
+ public:
+   A (T *p1) : p (p1) { p->acquire (); }
+ };
+ 
+ class B
+ {
+ public:
+     virtual void acquire ();
+ };
+ class D : public B
+ {
+ };
+ class F : B
+ {
+   int mrContext;
+ };
+ class WindowListenerMultiplexer : F, public D
+ {
+   void acquire () { acquire (); }
+ };
+ class C
+ {
+   void createPeer () throw ();
+   WindowListenerMultiplexer maWindowListeners;
+ };
+ class FmXGridPeer
+ {
+ public:
+     void addWindowListener (A<D>);
+ } a;
+ void
+ C::createPeer () throw ()
+ {
+   a.addWindowListener (&maWindowListeners);
+ }
+ 
+ /* { dg-final { scan-tree-dump-times "= OBJ_TYPE_REF" 1 "fre2" } } */

Re: [PATCH] Handle OBJ_TYPE_REF in FRE

[Jan Hubicka]

> 
> The following patch handles CSEing OBJ_TYPE_REF which was omitted
> because it is a GENERIC expression even on GIMPLE (for whatever

Why it is generic? It is part of gimple grammar :)

> reason...).  Rather than changing this now the following patch
> simply treats it properly as such.

Thanks for working on this! Will this do code motion, too?
I think you may want to compare the ODR type of obj_type_ref_class
otherwise two otherwise equivalent OBJ_TYPE_REFs may lead to different
optimizations later.  I suppose we can have code of form

if (test)
  OBJ_TYPE_REF1
  ...
else
  OBJ_TYPE_REF2
  ..
where each invoke method of different class type but would otherwise
match as equivalent for tree-ssa-sccvn becuase we ignore pointed-to types.

so doing

OBJ_TYPE_REF1
if (test)
  ...
else
  ...

may lead to wrong code.

Or do you just substitute the operands of OBJ_TYPE_REF? 
> 
> Bootstrap & regtest running on x86_64-unknown-linux-gnu.
> 
> Note that this does not (yet) substitute OBJ_TYPE_REFs in calls
> with SSA names that have the same value - not sure if that would
> be desired generally (does the devirt machinery cope with that?).

This should work fine.
> 
> Thanks,
> Richard.
> 
> 2015-12-03  Richard Biener  <rguenther@suse.de>
> 
> 	PR tree-optimization/64812
> 	* tree-ssa-sccvn.c (vn_get_stmt_kind): Handle OBJ_TYPE_REF.
> 	(vn_nary_length_from_stmt): Likewise.
> 	(init_vn_nary_op_from_stmt): Likewise.
> 	* gimple-match-head.c (maybe_build_generic_op): Likewise.
> 	* gimple-pretty-print.c (dump_unary_rhs): Likewise.
> 
> 	* g++.dg/tree-ssa/ssa-fre-1.C: New testcase.
> 
> Index: gcc/tree-ssa-sccvn.c
> ===================================================================
> *** gcc/tree-ssa-sccvn.c	(revision 231221)
> --- gcc/tree-ssa-sccvn.c	(working copy)
> *************** vn_get_stmt_kind (gimple *stmt)
> *** 460,465 ****
> --- 460,467 ----
>   			  ? VN_CONSTANT : VN_REFERENCE);
>   		else if (code == CONSTRUCTOR)
>   		  return VN_NARY;
> + 		else if (code == OBJ_TYPE_REF)
> + 		  return VN_NARY;
>   		return VN_NONE;
>   	      }
>   	  default:
> *************** vn_nary_length_from_stmt (gimple *stmt)
> *** 2479,2484 ****
> --- 2481,2487 ----
>         return 1;
>   
>       case BIT_FIELD_REF:
> +     case OBJ_TYPE_REF:
>         return 3;
>   
>       case CONSTRUCTOR:
> *************** init_vn_nary_op_from_stmt (vn_nary_op_t
> *** 2508,2513 ****
> --- 2511,2517 ----
>         break;
>   
>       case BIT_FIELD_REF:
> +     case OBJ_TYPE_REF:
>         vno->length = 3;
>         vno->op[0] = TREE_OPERAND (gimple_assign_rhs1 (stmt), 0);
>         vno->op[1] = TREE_OPERAND (gimple_assign_rhs1 (stmt), 1);
> Index: gcc/gimple-match-head.c
> ===================================================================
> *** gcc/gimple-match-head.c	(revision 231221)
> --- gcc/gimple-match-head.c	(working copy)
> *************** maybe_build_generic_op (enum tree_code c
> *** 243,248 ****
> --- 243,249 ----
>         *op0 = build1 (code, type, *op0);
>         break;
>       case BIT_FIELD_REF:
> +     case OBJ_TYPE_REF:
>         *op0 = build3 (code, type, *op0, op1, op2);
>         break;
>       default:;
> Index: gcc/gimple-pretty-print.c
> ===================================================================
> *** gcc/gimple-pretty-print.c	(revision 231221)
> --- gcc/gimple-pretty-print.c	(working copy)
> *************** dump_unary_rhs (pretty_printer *buffer,
> *** 302,308 ****
>   	  || TREE_CODE_CLASS (rhs_code) == tcc_reference
>   	  || rhs_code == SSA_NAME
>   	  || rhs_code == ADDR_EXPR
> ! 	  || rhs_code == CONSTRUCTOR)
>   	{
>   	  dump_generic_node (buffer, rhs, spc, flags, false);
>   	  break;
> --- 302,309 ----
>   	  || TREE_CODE_CLASS (rhs_code) == tcc_reference
>   	  || rhs_code == SSA_NAME
>   	  || rhs_code == ADDR_EXPR
> ! 	  || rhs_code == CONSTRUCTOR
> ! 	  || rhs_code == OBJ_TYPE_REF)
>   	{
>   	  dump_generic_node (buffer, rhs, spc, flags, false);
>   	  break;
> Index: gcc/testsuite/g++.dg/tree-ssa/ssa-fre-1.C
> ===================================================================
> *** gcc/testsuite/g++.dg/tree-ssa/ssa-fre-1.C	(revision 0)
> --- gcc/testsuite/g++.dg/tree-ssa/ssa-fre-1.C	(working copy)
> ***************
> *** 0 ****
> --- 1,44 ----
> + /* { dg-do compile } */
> + /* { dg-options "-O2 -fdump-tree-fre2" } */
> + 
> + template <class T> class A
> + {
> +   T *p;
> + 
> + public:
> +   A (T *p1) : p (p1) { p->acquire (); }
> + };
> + 
> + class B
> + {
> + public:
> +     virtual void acquire ();
> + };
> + class D : public B
> + {
> + };
> + class F : B
> + {
> +   int mrContext;
> + };
> + class WindowListenerMultiplexer : F, public D
> + {
> +   void acquire () { acquire (); }
> + };
> + class C
> + {
> +   void createPeer () throw ();
> +   WindowListenerMultiplexer maWindowListeners;
> + };
> + class FmXGridPeer
> + {
> + public:
> +     void addWindowListener (A<D>);
> + } a;
> + void
> + C::createPeer () throw ()
> + {
> +   a.addWindowListener (&maWindowListeners);
> + }
> + 
> + /* { dg-final { scan-tree-dump-times "= OBJ_TYPE_REF" 1 "fre2" } } */

Re: [PATCH] Handle OBJ_TYPE_REF in FRE

[Richard Biener]

On December 3, 2015 6:40:07 PM GMT+01:00, Jan Hubicka <hubicka@ucw.cz> wrote:
>> 
>> The following patch handles CSEing OBJ_TYPE_REF which was omitted
>> because it is a GENERIC expression even on GIMPLE (for whatever
>
>Why it is generic? It is part of gimple grammar :)
>
>> reason...).  Rather than changing this now the following patch
>> simply treats it properly as such.
>
>Thanks for working on this! Will this do code motion, too?

It will do PRE, so "yes".

>I think you may want to compare the ODR type of obj_type_ref_class
>otherwise two otherwise equivalent OBJ_TYPE_REFs may lead to different
>optimizations later.  I suppose we can have code of form
>
>if (test)
>  OBJ_TYPE_REF1
>  ...
>else
>  OBJ_TYPE_REF2
>  ..
>where each invoke method of different class type but would otherwise
>match as equivalent for tree-ssa-sccvn becuase we ignore pointed-to
>types.
>so doing
>
>OBJ_TYPE_REF1
>if (test)
>  ...
>else
>  ...
>
>may lead to wrong code.

Can you try generating a testcase?  Because with equal vptr and voffset I can't see how that can happen unless some pass extracts information from the pointer types without sanity checking with the pointers and offsets.

>Or do you just substitute the operands of OBJ_TYPE_REF? 

No, I value number them.  But yes, the type issue also crossed my mind.  Meanwhile testing revealed that I need to adjust gimple_expr_type to preserve the type of the obj-type-ref, otherwise the devirt machinery ICEs (receiving void *). That's also a reason we can't make obj-type-ref a ternary RHS.

>> Bootstrap & regtest running on x86_64-unknown-linux-gnu.
>> 
>> Note that this does not (yet) substitute OBJ_TYPE_REFs in calls
>> with SSA names that have the same value - not sure if that would
>> be desired generally (does the devirt machinery cope with that?).
>
>This should work fine.

OK. So with that substituting the direct call later should work as well.

Richard.

>> 
>> Thanks,
>> Richard.
>> 
>> 2015-12-03  Richard Biener  <rguenther@suse.de>
>> 
>> 	PR tree-optimization/64812
>> 	* tree-ssa-sccvn.c (vn_get_stmt_kind): Handle OBJ_TYPE_REF.
>> 	(vn_nary_length_from_stmt): Likewise.
>> 	(init_vn_nary_op_from_stmt): Likewise.
>> 	* gimple-match-head.c (maybe_build_generic_op): Likewise.
>> 	* gimple-pretty-print.c (dump_unary_rhs): Likewise.
>> 
>> 	* g++.dg/tree-ssa/ssa-fre-1.C: New testcase.
>> 
>> Index: gcc/tree-ssa-sccvn.c
>> ===================================================================
>> *** gcc/tree-ssa-sccvn.c	(revision 231221)
>> --- gcc/tree-ssa-sccvn.c	(working copy)
>> *************** vn_get_stmt_kind (gimple *stmt)
>> *** 460,465 ****
>> --- 460,467 ----
>>   			  ? VN_CONSTANT : VN_REFERENCE);
>>   		else if (code == CONSTRUCTOR)
>>   		  return VN_NARY;
>> + 		else if (code == OBJ_TYPE_REF)
>> + 		  return VN_NARY;
>>   		return VN_NONE;
>>   	      }
>>   	  default:
>> *************** vn_nary_length_from_stmt (gimple *stmt)
>> *** 2479,2484 ****
>> --- 2481,2487 ----
>>         return 1;
>>   
>>       case BIT_FIELD_REF:
>> +     case OBJ_TYPE_REF:
>>         return 3;
>>   
>>       case CONSTRUCTOR:
>> *************** init_vn_nary_op_from_stmt (vn_nary_op_t
>> *** 2508,2513 ****
>> --- 2511,2517 ----
>>         break;
>>   
>>       case BIT_FIELD_REF:
>> +     case OBJ_TYPE_REF:
>>         vno->length = 3;
>>         vno->op[0] = TREE_OPERAND (gimple_assign_rhs1 (stmt), 0);
>>         vno->op[1] = TREE_OPERAND (gimple_assign_rhs1 (stmt), 1);
>> Index: gcc/gimple-match-head.c
>> ===================================================================
>> *** gcc/gimple-match-head.c	(revision 231221)
>> --- gcc/gimple-match-head.c	(working copy)
>> *************** maybe_build_generic_op (enum tree_code c
>> *** 243,248 ****
>> --- 243,249 ----
>>         *op0 = build1 (code, type, *op0);
>>         break;
>>       case BIT_FIELD_REF:
>> +     case OBJ_TYPE_REF:
>>         *op0 = build3 (code, type, *op0, op1, op2);
>>         break;
>>       default:;
>> Index: gcc/gimple-pretty-print.c
>> ===================================================================
>> *** gcc/gimple-pretty-print.c	(revision 231221)
>> --- gcc/gimple-pretty-print.c	(working copy)
>> *************** dump_unary_rhs (pretty_printer *buffer,
>> *** 302,308 ****
>>   	  || TREE_CODE_CLASS (rhs_code) == tcc_reference
>>   	  || rhs_code == SSA_NAME
>>   	  || rhs_code == ADDR_EXPR
>> ! 	  || rhs_code == CONSTRUCTOR)
>>   	{
>>   	  dump_generic_node (buffer, rhs, spc, flags, false);
>>   	  break;
>> --- 302,309 ----
>>   	  || TREE_CODE_CLASS (rhs_code) == tcc_reference
>>   	  || rhs_code == SSA_NAME
>>   	  || rhs_code == ADDR_EXPR
>> ! 	  || rhs_code == CONSTRUCTOR
>> ! 	  || rhs_code == OBJ_TYPE_REF)
>>   	{
>>   	  dump_generic_node (buffer, rhs, spc, flags, false);
>>   	  break;
>> Index: gcc/testsuite/g++.dg/tree-ssa/ssa-fre-1.C
>> ===================================================================
>> *** gcc/testsuite/g++.dg/tree-ssa/ssa-fre-1.C	(revision 0)
>> --- gcc/testsuite/g++.dg/tree-ssa/ssa-fre-1.C	(working copy)
>> ***************
>> *** 0 ****
>> --- 1,44 ----
>> + /* { dg-do compile } */
>> + /* { dg-options "-O2 -fdump-tree-fre2" } */
>> + 
>> + template <class T> class A
>> + {
>> +   T *p;
>> + 
>> + public:
>> +   A (T *p1) : p (p1) { p->acquire (); }
>> + };
>> + 
>> + class B
>> + {
>> + public:
>> +     virtual void acquire ();
>> + };
>> + class D : public B
>> + {
>> + };
>> + class F : B
>> + {
>> +   int mrContext;
>> + };
>> + class WindowListenerMultiplexer : F, public D
>> + {
>> +   void acquire () { acquire (); }
>> + };
>> + class C
>> + {
>> +   void createPeer () throw ();
>> +   WindowListenerMultiplexer maWindowListeners;
>> + };
>> + class FmXGridPeer
>> + {
>> + public:
>> +     void addWindowListener (A<D>);
>> + } a;
>> + void
>> + C::createPeer () throw ()
>> + {
>> +   a.addWindowListener (&maWindowListeners);
>> + }
>> + 
>> + /* { dg-final { scan-tree-dump-times "= OBJ_TYPE_REF" 1 "fre2" } }
>*/

Re: [PATCH] Handle OBJ_TYPE_REF in FRE

[Jan Hubicka]

> >may lead to wrong code.
> 
> Can you try generating a testcase?
>  Because with equal vptr and voffset I can't see how that can happen unless some pass extracts information from the pointer types without sanity checking with the pointers and offsets.

I am not sure I can get a wrong code with current mainline, because for now you
only substitute for the lookup done for speculative devirt and if we wrongly
predict the thing to be __builtin_unreachable, we dispatch to usual virtual
call.  Once you get movement on calls it will be easier to do.

OBJ_TYPE_REF is a wrapper around OBJ_TYPE_EXPR adding three extra parameters:
 - OBJ_TYPE_REF_OBJECT
 - OBJ_TYPE_REF_TOKEN
 - obj_type_ref_class which is computed from TREE_TYPE (obj_type_ref) itself.

While two OBJ_TYPE_REFS with equivalent OBJ_TYPE_EXPR are kind of same
expressions, they are optimized differently (just as if they was in different
alias set).  For that reason you need to match the type of obj_type_ref_class
because that one is not matched by usless_type_conversion (it is a pointer to
method of corresponding class type we are looking up)

The following testcase:
struct foo {virtual void bar(void) __attribute__ ((const));};
struct foobar {virtual void bar(void) __attribute__ ((const));};
void
dojob(void *ptr, int t)
{
  if (t)
   ((struct foo*)ptr)->bar();
  else
   ((struct foobar*)ptr)->bar();
}

produces
void dojob(void*, int) (void * ptr, int t)
{
  int (*__vtbl_ptr_type) () * _5;
  int (*__vtbl_ptr_type) () _6;
  int (*__vtbl_ptr_type) () * _8;
  int (*__vtbl_ptr_type) () _9;

  <bb 2>:
  if (t_2(D) != 0)
    goto <bb 3>;
  else
    goto <bb 4>;

  <bb 3>:
  _5 = MEM[(struct foo *)ptr_4(D)]._vptr.foo;
  _6 = *_5;
  OBJ_TYPE_REF(_6;(struct foo)ptr_4(D)->0) (ptr_4(D));
  goto <bb 5>;

  <bb 4>:
  _8 = MEM[(struct foobar *)ptr_4(D)]._vptr.foobar;
  _9 = *_8;
  OBJ_TYPE_REF(_9;(struct foobar)ptr_4(D)->0) (ptr_4(D));

  <bb 5>:
  return;

}

Now I would need to get some code movement done to get _5 and _6
moved and unified with _8 and _9 that we currently don't do.  
Still would feel safer if the equivalence predicate also checked
that the type is the same.
> >Or do you just substitute the operands of OBJ_TYPE_REF? 
> 
> No, I value number them.  But yes, the type issue also crossed my mind.  Meanwhile testing revealed that I need to adjust gimple_expr_type to preserve the type of the obj-type-ref, otherwise the devirt machinery ICEs (receiving void *). That's also a reason we can't make obj-type-ref a ternary RHS.

Yep, type of OBJ_TYPE_REF matters...
> 
> >> Bootstrap & regtest running on x86_64-unknown-linux-gnu.
> >> 
> >> Note that this does not (yet) substitute OBJ_TYPE_REFs in calls
> >> with SSA names that have the same value - not sure if that would
> >> be desired generally (does the devirt machinery cope with that?).
> >
> >This should work fine.
> 
> OK. So with that substituting the direct call later should work as well.
Great!
> 
> Richard.

Honza
> 
> >> 
> >> Thanks,
> >> Richard.
> >> 
> >> 2015-12-03  Richard Biener  <rguenther@suse.de>
> >> 
> >> 	PR tree-optimization/64812
> >> 	* tree-ssa-sccvn.c (vn_get_stmt_kind): Handle OBJ_TYPE_REF.
> >> 	(vn_nary_length_from_stmt): Likewise.
> >> 	(init_vn_nary_op_from_stmt): Likewise.
> >> 	* gimple-match-head.c (maybe_build_generic_op): Likewise.
> >> 	* gimple-pretty-print.c (dump_unary_rhs): Likewise.
> >> 
> >> 	* g++.dg/tree-ssa/ssa-fre-1.C: New testcase.
> >> 
> >> Index: gcc/tree-ssa-sccvn.c
> >> ===================================================================
> >> *** gcc/tree-ssa-sccvn.c	(revision 231221)
> >> --- gcc/tree-ssa-sccvn.c	(working copy)
> >> *************** vn_get_stmt_kind (gimple *stmt)
> >> *** 460,465 ****
> >> --- 460,467 ----
> >>   			  ? VN_CONSTANT : VN_REFERENCE);
> >>   		else if (code == CONSTRUCTOR)
> >>   		  return VN_NARY;
> >> + 		else if (code == OBJ_TYPE_REF)
> >> + 		  return VN_NARY;
> >>   		return VN_NONE;
> >>   	      }
> >>   	  default:
> >> *************** vn_nary_length_from_stmt (gimple *stmt)
> >> *** 2479,2484 ****
> >> --- 2481,2487 ----
> >>         return 1;
> >>   
> >>       case BIT_FIELD_REF:
> >> +     case OBJ_TYPE_REF:
> >>         return 3;
> >>   
> >>       case CONSTRUCTOR:
> >> *************** init_vn_nary_op_from_stmt (vn_nary_op_t
> >> *** 2508,2513 ****
> >> --- 2511,2517 ----
> >>         break;
> >>   
> >>       case BIT_FIELD_REF:
> >> +     case OBJ_TYPE_REF:
> >>         vno->length = 3;
> >>         vno->op[0] = TREE_OPERAND (gimple_assign_rhs1 (stmt), 0);
> >>         vno->op[1] = TREE_OPERAND (gimple_assign_rhs1 (stmt), 1);
> >> Index: gcc/gimple-match-head.c
> >> ===================================================================
> >> *** gcc/gimple-match-head.c	(revision 231221)
> >> --- gcc/gimple-match-head.c	(working copy)
> >> *************** maybe_build_generic_op (enum tree_code c
> >> *** 243,248 ****
> >> --- 243,249 ----
> >>         *op0 = build1 (code, type, *op0);
> >>         break;
> >>       case BIT_FIELD_REF:
> >> +     case OBJ_TYPE_REF:
> >>         *op0 = build3 (code, type, *op0, op1, op2);
> >>         break;
> >>       default:;
> >> Index: gcc/gimple-pretty-print.c
> >> ===================================================================
> >> *** gcc/gimple-pretty-print.c	(revision 231221)
> >> --- gcc/gimple-pretty-print.c	(working copy)
> >> *************** dump_unary_rhs (pretty_printer *buffer,
> >> *** 302,308 ****
> >>   	  || TREE_CODE_CLASS (rhs_code) == tcc_reference
> >>   	  || rhs_code == SSA_NAME
> >>   	  || rhs_code == ADDR_EXPR
> >> ! 	  || rhs_code == CONSTRUCTOR)
> >>   	{
> >>   	  dump_generic_node (buffer, rhs, spc, flags, false);
> >>   	  break;
> >> --- 302,309 ----
> >>   	  || TREE_CODE_CLASS (rhs_code) == tcc_reference
> >>   	  || rhs_code == SSA_NAME
> >>   	  || rhs_code == ADDR_EXPR
> >> ! 	  || rhs_code == CONSTRUCTOR
> >> ! 	  || rhs_code == OBJ_TYPE_REF)
> >>   	{
> >>   	  dump_generic_node (buffer, rhs, spc, flags, false);
> >>   	  break;
> >> Index: gcc/testsuite/g++.dg/tree-ssa/ssa-fre-1.C
> >> ===================================================================
> >> *** gcc/testsuite/g++.dg/tree-ssa/ssa-fre-1.C	(revision 0)
> >> --- gcc/testsuite/g++.dg/tree-ssa/ssa-fre-1.C	(working copy)
> >> ***************
> >> *** 0 ****
> >> --- 1,44 ----
> >> + /* { dg-do compile } */
> >> + /* { dg-options "-O2 -fdump-tree-fre2" } */
> >> + 
> >> + template <class T> class A
> >> + {
> >> +   T *p;
> >> + 
> >> + public:
> >> +   A (T *p1) : p (p1) { p->acquire (); }
> >> + };
> >> + 
> >> + class B
> >> + {
> >> + public:
> >> +     virtual void acquire ();
> >> + };
> >> + class D : public B
> >> + {
> >> + };
> >> + class F : B
> >> + {
> >> +   int mrContext;
> >> + };
> >> + class WindowListenerMultiplexer : F, public D
> >> + {
> >> +   void acquire () { acquire (); }
> >> + };
> >> + class C
> >> + {
> >> +   void createPeer () throw ();
> >> +   WindowListenerMultiplexer maWindowListeners;
> >> + };
> >> + class FmXGridPeer
> >> + {
> >> + public:
> >> +     void addWindowListener (A<D>);
> >> + } a;
> >> + void
> >> + C::createPeer () throw ()
> >> + {
> >> +   a.addWindowListener (&maWindowListeners);
> >> + }
> >> + 
> >> + /* { dg-final { scan-tree-dump-times "= OBJ_TYPE_REF" 1 "fre2" } }
> >*/
> 

Re: [PATCH] Handle OBJ_TYPE_REF in FRE

[Richard Biener]

On Thu, 3 Dec 2015, Jan Hubicka wrote:

> > >may lead to wrong code.
> > 
> > Can you try generating a testcase?
> >  Because with equal vptr and voffset I can't see how that can happen 
> > unless some pass extracts information from the pointer types without 
> > sanity checking with the pointers and offsets.
> 
> I am not sure I can get a wrong code with current mainline, because for now you
> only substitute for the lookup done for speculative devirt and if we wrongly
> predict the thing to be __builtin_unreachable, we dispatch to usual virtual
> call.  Once you get movement on calls it will be easier to do.
> 
> OBJ_TYPE_REF is a wrapper around OBJ_TYPE_EXPR adding three extra parameters:
>  - OBJ_TYPE_REF_OBJECT
>  - OBJ_TYPE_REF_TOKEN
>  - obj_type_ref_class which is computed from TREE_TYPE (obj_type_ref) itself.
> 
> While two OBJ_TYPE_REFS with equivalent OBJ_TYPE_EXPR are kind of same
> expressions, they are optimized differently (just as if they was in different
> alias set).  For that reason you need to match the type of obj_type_ref_class
> because that one is not matched by usless_type_conversion (it is a pointer to
> method of corresponding class type we are looking up)
> 
> The following testcase:
> struct foo {virtual void bar(void) __attribute__ ((const));};
> struct foobar {virtual void bar(void) __attribute__ ((const));};
> void
> dojob(void *ptr, int t)
> {
>   if (t)
>    ((struct foo*)ptr)->bar();
>   else
>    ((struct foobar*)ptr)->bar();
> }
> 
> produces
> void dojob(void*, int) (void * ptr, int t)
> {
>   int (*__vtbl_ptr_type) () * _5;
>   int (*__vtbl_ptr_type) () _6;
>   int (*__vtbl_ptr_type) () * _8;
>   int (*__vtbl_ptr_type) () _9;
> 
>   <bb 2>:
>   if (t_2(D) != 0)
>     goto <bb 3>;
>   else
>     goto <bb 4>;
> 
>   <bb 3>:
>   _5 = MEM[(struct foo *)ptr_4(D)]._vptr.foo;
>   _6 = *_5;
>   OBJ_TYPE_REF(_6;(struct foo)ptr_4(D)->0) (ptr_4(D));
>   goto <bb 5>;
> 
>   <bb 4>:
>   _8 = MEM[(struct foobar *)ptr_4(D)]._vptr.foobar;
>   _9 = *_8;
>   OBJ_TYPE_REF(_9;(struct foobar)ptr_4(D)->0) (ptr_4(D));
> 
>   <bb 5>:
>   return;
> 
> }
> 
> Now I would need to get some code movement done to get _5 and _6
> moved and unified with _8 and _9 that we currently don't do.  
> Still would feel safer if the equivalence predicate also checked
> that the type is the same.

Indeed we don't do code hoisting yet.  Maybe one could trick PPRE
into doing it.

Note that for OBJ_TYPE_REFs in calls you probably should better use
gimple_call_fntype instead of the type of the OBJ_TYPE_REF anyway
(well, fntype will be the method-type, not pointer-to-method-type).

Not sure if you need OBJ_TYPE_REFs type in non-call contexts?

> > >Or do you just substitute the operands of OBJ_TYPE_REF? 
> > 
> > No, I value number them.  But yes, the type issue also crossed my 
> > mind.  Meanwhile testing revealed that I need to adjust 
> > gimple_expr_type to preserve the type of the obj-type-ref, otherwise 
> > the devirt machinery ICEs (receiving void *). That's also a reason we 
> > can't make obj-type-ref a ternary RHS.
> 
> Yep, type of OBJ_TYPE_REF matters...

See above.

> > 
> > >> Bootstrap & regtest running on x86_64-unknown-linux-gnu.
> > >> 
> > >> Note that this does not (yet) substitute OBJ_TYPE_REFs in calls
> > >> with SSA names that have the same value - not sure if that would
> > >> be desired generally (does the devirt machinery cope with that?).
> > >
> > >This should work fine.
> > 
> > OK. So with that substituting the direct call later should work as well.
> Great!

For the above reasons I'm defering all this to stage1.

Below is the patch that actually passed bootstrap & regtest on 
x86_64-unknown-linux-gnu, just in case you want to play with it.
It doesn't do the propagation into calls yet though, the following
does (untested)

Index: gcc/tree-ssa-pre.c
===================================================================
--- gcc/tree-ssa-pre.c  (revision 231244)
+++ gcc/tree-ssa-pre.c  (working copy)
@@ -4334,6 +4334,22 @@ eliminate_dom_walker::before_dom_childre
                  maybe_remove_unused_call_args (cfun, call_stmt);
                  gimple_set_modified (stmt, true);
                }
+
+             else
+               {
+                 /* Lookup the OBJ_TYPE_REF.  */
+                 tree sprime
+                   = vn_nary_op_lookup_pieces (3, OBJ_TYPE_REF,
+                                               TREE_TYPE (fn),
+                                               &TREE_OPERAND (fn, 0), 
NULL);
+                 if (sprime)
+                   sprime = eliminate_avail (sprime);
+                 if (sprime)
+                   {
+                     gimple_call_set_fn (call_stmt, sprime);
+                     gimple_set_modified (stmt, true);
+                   }
+               }
            }
        }
 
but it ICEs because we decided (tree-cfg.c, verify_gimple_call):

  if (fn
      && (!POINTER_TYPE_P (TREE_TYPE (fn))
          || (TREE_CODE (TREE_TYPE (TREE_TYPE (fn))) != FUNCTION_TYPE
              && TREE_CODE (TREE_TYPE (TREE_TYPE (fn))) != METHOD_TYPE)))
    {
      error ("non-function in gimple call");
      return true;
    }

and in useless_type_conversion_p:

      /* Do not lose casts to function pointer types.  */
      if ((TREE_CODE (TREE_TYPE (outer_type)) == FUNCTION_TYPE
           || TREE_CODE (TREE_TYPE (outer_type)) == METHOD_TYPE)
          && !(TREE_CODE (TREE_TYPE (inner_type)) == FUNCTION_TYPE
               || TREE_CODE (TREE_TYPE (inner_type)) == METHOD_TYPE))
        return false;

probably from the times we didn't have gimple_call_fntype.  So if I
paper over the ICE (in the verifier) then the libreoffice testcase
gets optimized to

  <bb 2>:
  _3 = this_2(D)->D.2399.D.2325._vptr.B;
  _4 = *_3;
  PROF_6 = OBJ_TYPE_REF(_4;(struct 
WindowListenerMultiplexer)this_2(D)->0);
  if (PROF_6 == acquire)
    goto <bb 3>;
  else
    goto <bb 4>;

  <bb 3>:
  PROF_6 (this_2(D));
  goto <bb 5>;

  <bb 4>:
  PROF_6 (this_2(D));

by FRE2 and either VRP or DOM will propagate the equivalency to

  <bb 2>:
  _3 = this_2(D)->D.2399.D.2325._vptr.B;
  _4 = *_3;
  PROF_6 = OBJ_TYPE_REF(_4;(struct 
WindowListenerMultiplexer)this_2(D)->0);
  if (PROF_6 == acquire)
    goto <bb 3>;
  else
    goto <bb 4>;

  <bb 3>:
  WindowListenerMultiplexer::acquire (this_2(D));
  goto <bb 5>;

  <bb 4>:
  PROF_6 (this_2(D));

Richard.

2015-12-03  Richard Biener  <rguenther@suse.de>

	PR tree-optimization/64812
	* tree-ssa-sccvn.c (vn_get_stmt_kind): Handle OBJ_TYPE_REF.
	(vn_nary_length_from_stmt): Likewise.
	(init_vn_nary_op_from_stmt): Likewise.
	* gimple-match-head.c (maybe_build_generic_op): Likewise.
	* gimple-pretty-print.c (dump_unary_rhs): Likewise.
	* gimple-fold.c (gimple_build): Likewise.
	* gimple.h (gimple_expr_type): Likewise.

	* g++.dg/tree-ssa/ssa-fre-1.C: New testcase.

Index: gcc/tree-ssa-sccvn.c
===================================================================
*** gcc/tree-ssa-sccvn.c	(revision 231221)
--- gcc/tree-ssa-sccvn.c	(working copy)
*************** vn_get_stmt_kind (gimple *stmt)
*** 460,465 ****
--- 460,467 ----
  			  ? VN_CONSTANT : VN_REFERENCE);
  		else if (code == CONSTRUCTOR)
  		  return VN_NARY;
+ 		else if (code == OBJ_TYPE_REF)
+ 		  return VN_NARY;
  		return VN_NONE;
  	      }
  	  default:
*************** vn_nary_length_from_stmt (gimple *stmt)
*** 2479,2484 ****
--- 2481,2487 ----
        return 1;
  
      case BIT_FIELD_REF:
+     case OBJ_TYPE_REF:
        return 3;
  
      case CONSTRUCTOR:
*************** init_vn_nary_op_from_stmt (vn_nary_op_t
*** 2508,2513 ****
--- 2511,2517 ----
        break;
  
      case BIT_FIELD_REF:
+     case OBJ_TYPE_REF:
        vno->length = 3;
        vno->op[0] = TREE_OPERAND (gimple_assign_rhs1 (stmt), 0);
        vno->op[1] = TREE_OPERAND (gimple_assign_rhs1 (stmt), 1);
Index: gcc/gimple-match-head.c
===================================================================
*** gcc/gimple-match-head.c	(revision 231221)
--- gcc/gimple-match-head.c	(working copy)
*************** maybe_build_generic_op (enum tree_code c
*** 243,248 ****
--- 243,249 ----
        *op0 = build1 (code, type, *op0);
        break;
      case BIT_FIELD_REF:
+     case OBJ_TYPE_REF:
        *op0 = build3 (code, type, *op0, op1, op2);
        break;
      default:;
Index: gcc/gimple-pretty-print.c
===================================================================
*** gcc/gimple-pretty-print.c	(revision 231221)
--- gcc/gimple-pretty-print.c	(working copy)
*************** dump_unary_rhs (pretty_printer *buffer,
*** 302,308 ****
  	  || TREE_CODE_CLASS (rhs_code) == tcc_reference
  	  || rhs_code == SSA_NAME
  	  || rhs_code == ADDR_EXPR
! 	  || rhs_code == CONSTRUCTOR)
  	{
  	  dump_generic_node (buffer, rhs, spc, flags, false);
  	  break;
--- 302,309 ----
  	  || TREE_CODE_CLASS (rhs_code) == tcc_reference
  	  || rhs_code == SSA_NAME
  	  || rhs_code == ADDR_EXPR
! 	  || rhs_code == CONSTRUCTOR
! 	  || rhs_code == OBJ_TYPE_REF)
  	{
  	  dump_generic_node (buffer, rhs, spc, flags, false);
  	  break;
Index: gcc/testsuite/g++.dg/tree-ssa/ssa-fre-1.C
===================================================================
*** gcc/testsuite/g++.dg/tree-ssa/ssa-fre-1.C	(revision 0)
--- gcc/testsuite/g++.dg/tree-ssa/ssa-fre-1.C	(working copy)
***************
*** 0 ****
--- 1,44 ----
+ /* { dg-do compile } */
+ /* { dg-options "-O2 -fdump-tree-fre2" } */
+ 
+ template <class T> class A
+ {
+   T *p;
+ 
+ public:
+   A (T *p1) : p (p1) { p->acquire (); }
+ };
+ 
+ class B
+ {
+ public:
+     virtual void acquire ();
+ };
+ class D : public B
+ {
+ };
+ class F : B
+ {
+   int mrContext;
+ };
+ class WindowListenerMultiplexer : F, public D
+ {
+   void acquire () { acquire (); }
+ };
+ class C
+ {
+   void createPeer () throw ();
+   WindowListenerMultiplexer maWindowListeners;
+ };
+ class FmXGridPeer
+ {
+ public:
+     void addWindowListener (A<D>);
+ } a;
+ void
+ C::createPeer () throw ()
+ {
+   a.addWindowListener (&maWindowListeners);
+ }
+ 
+ /* { dg-final { scan-tree-dump-times "= OBJ_TYPE_REF" 1 "fre2" } } */
Index: gcc/gimple-fold.c
===================================================================
--- gcc/gimple-fold.c	(revision 231221)
+++ gcc/gimple-fold.c	(working copy)
@@ -6038,7 +6038,8 @@ gimple_build (gimple_seq *seq, location_
       else
 	res = create_tmp_reg (type);
       gimple *stmt;
-      if (code == BIT_FIELD_REF)
+      if (code == BIT_FIELD_REF
+	  || code == OBJ_TYPE_REF)
 	stmt = gimple_build_assign (res, code,
 				    build3 (code, type, op0, op1, op2));
       else
Index: gcc/gimple.h
===================================================================
--- gcc/gimple.h	(revision 231221)
+++ gcc/gimple.h	(working copy)
@@ -6079,7 +6079,9 @@ gimple_expr_type (const gimple *stmt)
     }
   else if (code == GIMPLE_ASSIGN)
     {
-      if (gimple_assign_rhs_code (stmt) == POINTER_PLUS_EXPR)
+      enum tree_code rcode = gimple_assign_rhs_code (stmt);
+      if (rcode == POINTER_PLUS_EXPR
+	  || rcode == OBJ_TYPE_REF)
         return TREE_TYPE (gimple_assign_rhs1 (stmt));
       else
         /* As fallback use the type of the LHS.  */

Re: [PATCH] Handle OBJ_TYPE_REF in FRE

[Jan Hubicka]

> Indeed we don't do code hoisting yet.  Maybe one could trick PPRE
> into doing it.
> 
> Note that for OBJ_TYPE_REFs in calls you probably should better use
> gimple_call_fntype instead of the type of the OBJ_TYPE_REF anyway
> (well, fntype will be the method-type, not pointer-to-method-type).
> 
> Not sure if you need OBJ_TYPE_REFs type in non-call contexts?

Well, to optimize speculative call sequences

if (funptr == thismethod)
  inlined this method body
else
  funptr ();

Here you want to devirtualize the conditional, not the call in order
to get the inlined method unconditonally.

In general I think OBJ_TYPE_REF is misplaced - it should be on vtable load
instead of the call/conditional. It is a property of the vtable lookup.
Then it would work for method pointers too.
> 
>   if (fn
>       && (!POINTER_TYPE_P (TREE_TYPE (fn))
>           || (TREE_CODE (TREE_TYPE (TREE_TYPE (fn))) != FUNCTION_TYPE
>               && TREE_CODE (TREE_TYPE (TREE_TYPE (fn))) != METHOD_TYPE)))
>     {
>       error ("non-function in gimple call");
>       return true;
>     }
> 
> and in useless_type_conversion_p:
> 
>       /* Do not lose casts to function pointer types.  */
>       if ((TREE_CODE (TREE_TYPE (outer_type)) == FUNCTION_TYPE
>            || TREE_CODE (TREE_TYPE (outer_type)) == METHOD_TYPE)
>           && !(TREE_CODE (TREE_TYPE (inner_type)) == FUNCTION_TYPE
>                || TREE_CODE (TREE_TYPE (inner_type)) == METHOD_TYPE))
>         return false;

Yeah, this does not make much sense to me anymore.  Something to track next
stage1.
> 
> probably from the times we didn't have gimple_call_fntype.  So if I
> paper over the ICE (in the verifier) then the libreoffice testcase
> gets optimized to
> 
>   <bb 2>:
>   _3 = this_2(D)->D.2399.D.2325._vptr.B;
>   _4 = *_3;
>   PROF_6 = OBJ_TYPE_REF(_4;(struct 
> WindowListenerMultiplexer)this_2(D)->0);
>   if (PROF_6 == acquire)
>     goto <bb 3>;
>   else
>     goto <bb 4>;
> 
>   <bb 3>:
>   PROF_6 (this_2(D));
>   goto <bb 5>;
> 
>   <bb 4>:
>   PROF_6 (this_2(D));
> 
> by FRE2 and either VRP or DOM will propagate the equivalency to
> 
>   <bb 2>:
>   _3 = this_2(D)->D.2399.D.2325._vptr.B;
>   _4 = *_3;
>   PROF_6 = OBJ_TYPE_REF(_4;(struct 
> WindowListenerMultiplexer)this_2(D)->0);
>   if (PROF_6 == acquire)
>     goto <bb 3>;
>   else
>     goto <bb 4>;
> 
>   <bb 3>:
>   WindowListenerMultiplexer::acquire (this_2(D));
>   goto <bb 5>;
> 
>   <bb 4>:
>   PROF_6 (this_2(D));
> 
> Richard.

LGTM.
Honza
> 
> 2015-12-03  Richard Biener  <rguenther@suse.de>
> 
> 	PR tree-optimization/64812
> 	* tree-ssa-sccvn.c (vn_get_stmt_kind): Handle OBJ_TYPE_REF.
> 	(vn_nary_length_from_stmt): Likewise.
> 	(init_vn_nary_op_from_stmt): Likewise.
> 	* gimple-match-head.c (maybe_build_generic_op): Likewise.
> 	* gimple-pretty-print.c (dump_unary_rhs): Likewise.
> 	* gimple-fold.c (gimple_build): Likewise.
> 	* gimple.h (gimple_expr_type): Likewise.
> 
> 	* g++.dg/tree-ssa/ssa-fre-1.C: New testcase.
> 
> Index: gcc/tree-ssa-sccvn.c
> ===================================================================
> *** gcc/tree-ssa-sccvn.c	(revision 231221)
> --- gcc/tree-ssa-sccvn.c	(working copy)
> *************** vn_get_stmt_kind (gimple *stmt)
> *** 460,465 ****
> --- 460,467 ----
>   			  ? VN_CONSTANT : VN_REFERENCE);
>   		else if (code == CONSTRUCTOR)
>   		  return VN_NARY;
> + 		else if (code == OBJ_TYPE_REF)
> + 		  return VN_NARY;
>   		return VN_NONE;
>   	      }
>   	  default:
> *************** vn_nary_length_from_stmt (gimple *stmt)
> *** 2479,2484 ****
> --- 2481,2487 ----
>         return 1;
>   
>       case BIT_FIELD_REF:
> +     case OBJ_TYPE_REF:
>         return 3;
>   
>       case CONSTRUCTOR:
> *************** init_vn_nary_op_from_stmt (vn_nary_op_t
> *** 2508,2513 ****
> --- 2511,2517 ----
>         break;
>   
>       case BIT_FIELD_REF:
> +     case OBJ_TYPE_REF:
>         vno->length = 3;
>         vno->op[0] = TREE_OPERAND (gimple_assign_rhs1 (stmt), 0);
>         vno->op[1] = TREE_OPERAND (gimple_assign_rhs1 (stmt), 1);
> Index: gcc/gimple-match-head.c
> ===================================================================
> *** gcc/gimple-match-head.c	(revision 231221)
> --- gcc/gimple-match-head.c	(working copy)
> *************** maybe_build_generic_op (enum tree_code c
> *** 243,248 ****
> --- 243,249 ----
>         *op0 = build1 (code, type, *op0);
>         break;
>       case BIT_FIELD_REF:
> +     case OBJ_TYPE_REF:
>         *op0 = build3 (code, type, *op0, op1, op2);
>         break;
>       default:;
> Index: gcc/gimple-pretty-print.c
> ===================================================================
> *** gcc/gimple-pretty-print.c	(revision 231221)
> --- gcc/gimple-pretty-print.c	(working copy)
> *************** dump_unary_rhs (pretty_printer *buffer,
> *** 302,308 ****
>   	  || TREE_CODE_CLASS (rhs_code) == tcc_reference
>   	  || rhs_code == SSA_NAME
>   	  || rhs_code == ADDR_EXPR
> ! 	  || rhs_code == CONSTRUCTOR)
>   	{
>   	  dump_generic_node (buffer, rhs, spc, flags, false);
>   	  break;
> --- 302,309 ----
>   	  || TREE_CODE_CLASS (rhs_code) == tcc_reference
>   	  || rhs_code == SSA_NAME
>   	  || rhs_code == ADDR_EXPR
> ! 	  || rhs_code == CONSTRUCTOR
> ! 	  || rhs_code == OBJ_TYPE_REF)
>   	{
>   	  dump_generic_node (buffer, rhs, spc, flags, false);
>   	  break;
> Index: gcc/testsuite/g++.dg/tree-ssa/ssa-fre-1.C
> ===================================================================
> *** gcc/testsuite/g++.dg/tree-ssa/ssa-fre-1.C	(revision 0)
> --- gcc/testsuite/g++.dg/tree-ssa/ssa-fre-1.C	(working copy)
> ***************
> *** 0 ****
> --- 1,44 ----
> + /* { dg-do compile } */
> + /* { dg-options "-O2 -fdump-tree-fre2" } */
> + 
> + template <class T> class A
> + {
> +   T *p;
> + 
> + public:
> +   A (T *p1) : p (p1) { p->acquire (); }
> + };
> + 
> + class B
> + {
> + public:
> +     virtual void acquire ();
> + };
> + class D : public B
> + {
> + };
> + class F : B
> + {
> +   int mrContext;
> + };
> + class WindowListenerMultiplexer : F, public D
> + {
> +   void acquire () { acquire (); }
> + };
> + class C
> + {
> +   void createPeer () throw ();
> +   WindowListenerMultiplexer maWindowListeners;
> + };
> + class FmXGridPeer
> + {
> + public:
> +     void addWindowListener (A<D>);
> + } a;
> + void
> + C::createPeer () throw ()
> + {
> +   a.addWindowListener (&maWindowListeners);
> + }
> + 
> + /* { dg-final { scan-tree-dump-times "= OBJ_TYPE_REF" 1 "fre2" } } */
> Index: gcc/gimple-fold.c
> ===================================================================
> --- gcc/gimple-fold.c	(revision 231221)
> +++ gcc/gimple-fold.c	(working copy)
> @@ -6038,7 +6038,8 @@ gimple_build (gimple_seq *seq, location_
>        else
>  	res = create_tmp_reg (type);
>        gimple *stmt;
> -      if (code == BIT_FIELD_REF)
> +      if (code == BIT_FIELD_REF
> +	  || code == OBJ_TYPE_REF)
>  	stmt = gimple_build_assign (res, code,
>  				    build3 (code, type, op0, op1, op2));
>        else
> Index: gcc/gimple.h
> ===================================================================
> --- gcc/gimple.h	(revision 231221)
> +++ gcc/gimple.h	(working copy)
> @@ -6079,7 +6079,9 @@ gimple_expr_type (const gimple *stmt)
>      }
>    else if (code == GIMPLE_ASSIGN)
>      {
> -      if (gimple_assign_rhs_code (stmt) == POINTER_PLUS_EXPR)
> +      enum tree_code rcode = gimple_assign_rhs_code (stmt);
> +      if (rcode == POINTER_PLUS_EXPR
> +	  || rcode == OBJ_TYPE_REF)
>          return TREE_TYPE (gimple_assign_rhs1 (stmt));
>        else
>          /* As fallback use the type of the LHS.  */

Re: [PATCH] Handle OBJ_TYPE_REF in FRE

[Richard Biener]

On Fri, 4 Dec 2015, Jan Hubicka wrote:

> > Indeed we don't do code hoisting yet.  Maybe one could trick PPRE
> > into doing it.
> > 
> > Note that for OBJ_TYPE_REFs in calls you probably should better use
> > gimple_call_fntype instead of the type of the OBJ_TYPE_REF anyway
> > (well, fntype will be the method-type, not pointer-to-method-type).
> > 
> > Not sure if you need OBJ_TYPE_REFs type in non-call contexts?
> 
> Well, to optimize speculative call sequences
> 
> if (funptr == thismethod)
>   inlined this method body
> else
>   funptr ();
> 
> Here you want to devirtualize the conditional, not the call in order
> to get the inlined method unconditonally.
> 
> In general I think OBJ_TYPE_REF is misplaced - it should be on vtable load
> instead of the call/conditional. It is a property of the vtable lookup.
> Then it would work for method pointers too.

Even better.  Make it a tcc_reference tree then.

> > 
> >   if (fn
> >       && (!POINTER_TYPE_P (TREE_TYPE (fn))
> >           || (TREE_CODE (TREE_TYPE (TREE_TYPE (fn))) != FUNCTION_TYPE
> >               && TREE_CODE (TREE_TYPE (TREE_TYPE (fn))) != METHOD_TYPE)))
> >     {
> >       error ("non-function in gimple call");
> >       return true;
> >     }
> > 
> > and in useless_type_conversion_p:
> > 
> >       /* Do not lose casts to function pointer types.  */
> >       if ((TREE_CODE (TREE_TYPE (outer_type)) == FUNCTION_TYPE
> >            || TREE_CODE (TREE_TYPE (outer_type)) == METHOD_TYPE)
> >           && !(TREE_CODE (TREE_TYPE (inner_type)) == FUNCTION_TYPE
> >                || TREE_CODE (TREE_TYPE (inner_type)) == METHOD_TYPE))
> >         return false;
> 
> Yeah, this does not make much sense to me anymore.  Something to track next
> stage1.

Btw, might be necessary for targets with function descriptors - not sure
though.

Richard.

> > 
> > probably from the times we didn't have gimple_call_fntype.  So if I
> > paper over the ICE (in the verifier) then the libreoffice testcase
> > gets optimized to
> > 
> >   <bb 2>:
> >   _3 = this_2(D)->D.2399.D.2325._vptr.B;
> >   _4 = *_3;
> >   PROF_6 = OBJ_TYPE_REF(_4;(struct 
> > WindowListenerMultiplexer)this_2(D)->0);
> >   if (PROF_6 == acquire)
> >     goto <bb 3>;
> >   else
> >     goto <bb 4>;
> > 
> >   <bb 3>:
> >   PROF_6 (this_2(D));
> >   goto <bb 5>;
> > 
> >   <bb 4>:
> >   PROF_6 (this_2(D));
> > 
> > by FRE2 and either VRP or DOM will propagate the equivalency to
> > 
> >   <bb 2>:
> >   _3 = this_2(D)->D.2399.D.2325._vptr.B;
> >   _4 = *_3;
> >   PROF_6 = OBJ_TYPE_REF(_4;(struct 
> > WindowListenerMultiplexer)this_2(D)->0);
> >   if (PROF_6 == acquire)
> >     goto <bb 3>;
> >   else
> >     goto <bb 4>;
> > 
> >   <bb 3>:
> >   WindowListenerMultiplexer::acquire (this_2(D));
> >   goto <bb 5>;
> > 
> >   <bb 4>:
> >   PROF_6 (this_2(D));
> > 
> > Richard.
> 
> LGTM.
> Honza
> > 
> > 2015-12-03  Richard Biener  <rguenther@suse.de>
> > 
> > 	PR tree-optimization/64812
> > 	* tree-ssa-sccvn.c (vn_get_stmt_kind): Handle OBJ_TYPE_REF.
> > 	(vn_nary_length_from_stmt): Likewise.
> > 	(init_vn_nary_op_from_stmt): Likewise.
> > 	* gimple-match-head.c (maybe_build_generic_op): Likewise.
> > 	* gimple-pretty-print.c (dump_unary_rhs): Likewise.
> > 	* gimple-fold.c (gimple_build): Likewise.
> > 	* gimple.h (gimple_expr_type): Likewise.
> > 
> > 	* g++.dg/tree-ssa/ssa-fre-1.C: New testcase.
> > 
> > Index: gcc/tree-ssa-sccvn.c
> > ===================================================================
> > *** gcc/tree-ssa-sccvn.c	(revision 231221)
> > --- gcc/tree-ssa-sccvn.c	(working copy)
> > *************** vn_get_stmt_kind (gimple *stmt)
> > *** 460,465 ****
> > --- 460,467 ----
> >   			  ? VN_CONSTANT : VN_REFERENCE);
> >   		else if (code == CONSTRUCTOR)
> >   		  return VN_NARY;
> > + 		else if (code == OBJ_TYPE_REF)
> > + 		  return VN_NARY;
> >   		return VN_NONE;
> >   	      }
> >   	  default:
> > *************** vn_nary_length_from_stmt (gimple *stmt)
> > *** 2479,2484 ****
> > --- 2481,2487 ----
> >         return 1;
> >   
> >       case BIT_FIELD_REF:
> > +     case OBJ_TYPE_REF:
> >         return 3;
> >   
> >       case CONSTRUCTOR:
> > *************** init_vn_nary_op_from_stmt (vn_nary_op_t
> > *** 2508,2513 ****
> > --- 2511,2517 ----
> >         break;
> >   
> >       case BIT_FIELD_REF:
> > +     case OBJ_TYPE_REF:
> >         vno->length = 3;
> >         vno->op[0] = TREE_OPERAND (gimple_assign_rhs1 (stmt), 0);
> >         vno->op[1] = TREE_OPERAND (gimple_assign_rhs1 (stmt), 1);
> > Index: gcc/gimple-match-head.c
> > ===================================================================
> > *** gcc/gimple-match-head.c	(revision 231221)
> > --- gcc/gimple-match-head.c	(working copy)
> > *************** maybe_build_generic_op (enum tree_code c
> > *** 243,248 ****
> > --- 243,249 ----
> >         *op0 = build1 (code, type, *op0);
> >         break;
> >       case BIT_FIELD_REF:
> > +     case OBJ_TYPE_REF:
> >         *op0 = build3 (code, type, *op0, op1, op2);
> >         break;
> >       default:;
> > Index: gcc/gimple-pretty-print.c
> > ===================================================================
> > *** gcc/gimple-pretty-print.c	(revision 231221)
> > --- gcc/gimple-pretty-print.c	(working copy)
> > *************** dump_unary_rhs (pretty_printer *buffer,
> > *** 302,308 ****
> >   	  || TREE_CODE_CLASS (rhs_code) == tcc_reference
> >   	  || rhs_code == SSA_NAME
> >   	  || rhs_code == ADDR_EXPR
> > ! 	  || rhs_code == CONSTRUCTOR)
> >   	{
> >   	  dump_generic_node (buffer, rhs, spc, flags, false);
> >   	  break;
> > --- 302,309 ----
> >   	  || TREE_CODE_CLASS (rhs_code) == tcc_reference
> >   	  || rhs_code == SSA_NAME
> >   	  || rhs_code == ADDR_EXPR
> > ! 	  || rhs_code == CONSTRUCTOR
> > ! 	  || rhs_code == OBJ_TYPE_REF)
> >   	{
> >   	  dump_generic_node (buffer, rhs, spc, flags, false);
> >   	  break;
> > Index: gcc/testsuite/g++.dg/tree-ssa/ssa-fre-1.C
> > ===================================================================
> > *** gcc/testsuite/g++.dg/tree-ssa/ssa-fre-1.C	(revision 0)
> > --- gcc/testsuite/g++.dg/tree-ssa/ssa-fre-1.C	(working copy)
> > ***************
> > *** 0 ****
> > --- 1,44 ----
> > + /* { dg-do compile } */
> > + /* { dg-options "-O2 -fdump-tree-fre2" } */
> > + 
> > + template <class T> class A
> > + {
> > +   T *p;
> > + 
> > + public:
> > +   A (T *p1) : p (p1) { p->acquire (); }
> > + };
> > + 
> > + class B
> > + {
> > + public:
> > +     virtual void acquire ();
> > + };
> > + class D : public B
> > + {
> > + };
> > + class F : B
> > + {
> > +   int mrContext;
> > + };
> > + class WindowListenerMultiplexer : F, public D
> > + {
> > +   void acquire () { acquire (); }
> > + };
> > + class C
> > + {
> > +   void createPeer () throw ();
> > +   WindowListenerMultiplexer maWindowListeners;
> > + };
> > + class FmXGridPeer
> > + {
> > + public:
> > +     void addWindowListener (A<D>);
> > + } a;
> > + void
> > + C::createPeer () throw ()
> > + {
> > +   a.addWindowListener (&maWindowListeners);
> > + }
> > + 
> > + /* { dg-final { scan-tree-dump-times "= OBJ_TYPE_REF" 1 "fre2" } } */
> > Index: gcc/gimple-fold.c
> > ===================================================================
> > --- gcc/gimple-fold.c	(revision 231221)
> > +++ gcc/gimple-fold.c	(working copy)
> > @@ -6038,7 +6038,8 @@ gimple_build (gimple_seq *seq, location_
> >        else
> >  	res = create_tmp_reg (type);
> >        gimple *stmt;
> > -      if (code == BIT_FIELD_REF)
> > +      if (code == BIT_FIELD_REF
> > +	  || code == OBJ_TYPE_REF)
> >  	stmt = gimple_build_assign (res, code,
> >  				    build3 (code, type, op0, op1, op2));
> >        else
> > Index: gcc/gimple.h
> > ===================================================================
> > --- gcc/gimple.h	(revision 231221)
> > +++ gcc/gimple.h	(working copy)
> > @@ -6079,7 +6079,9 @@ gimple_expr_type (const gimple *stmt)
> >      }
> >    else if (code == GIMPLE_ASSIGN)
> >      {
> > -      if (gimple_assign_rhs_code (stmt) == POINTER_PLUS_EXPR)
> > +      enum tree_code rcode = gimple_assign_rhs_code (stmt);
> > +      if (rcode == POINTER_PLUS_EXPR
> > +	  || rcode == OBJ_TYPE_REF)
> >          return TREE_TYPE (gimple_assign_rhs1 (stmt));
> >        else
> >          /* As fallback use the type of the LHS.  */
> 
> 

-- 
Richard Biener <rguenther@suse.de>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg)

Re: [PATCH] Handle OBJ_TYPE_REF in FRE

[Jan Hubicka]

> On Fri, 4 Dec 2015, Jan Hubicka wrote:
> 
> > > Indeed we don't do code hoisting yet.  Maybe one could trick PPRE
> > > into doing it.
> > > 
> > > Note that for OBJ_TYPE_REFs in calls you probably should better use
> > > gimple_call_fntype instead of the type of the OBJ_TYPE_REF anyway
> > > (well, fntype will be the method-type, not pointer-to-method-type).
> > > 
> > > Not sure if you need OBJ_TYPE_REFs type in non-call contexts?
> > 
> > Well, to optimize speculative call sequences
> > 
> > if (funptr == thismethod)
> >   inlined this method body
> > else
> >   funptr ();
> > 
> > Here you want to devirtualize the conditional, not the call in order
> > to get the inlined method unconditonally.
> > 
> > In general I think OBJ_TYPE_REF is misplaced - it should be on vtable load
> > instead of the call/conditional. It is a property of the vtable lookup.
> > Then it would work for method pointers too.
> 
> Even better.  Make it a tcc_reference tree then.

makes sense to me - it is indeed more similar to MEM_REF than to an expression.
I will look into that next stage1.

Honza