This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH] Handle OBJ_TYPE_REF in FRE
- From: Richard Biener <rguenther at suse dot de>
- To: Jan Hubicka <hubicka at ucw dot cz>
- Cc: gcc-patches at gcc dot gnu dot org
- Date: Thu, 03 Dec 2015 21:45:58 +0100
- Subject: Re: [PATCH] Handle OBJ_TYPE_REF in FRE
- Authentication-results: sourceware.org; auth=none
- References: <alpine dot LSU dot 2 dot 11 dot 1512031142360 dot 4884 at t29 dot fhfr dot qr> <20151203174007 dot GA59701 at kam dot mff dot cuni dot cz>
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" } }
>*/