This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] Handle OBJ_TYPE_REF in FRE


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" } }
>*/



Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]