From: Richard Biener <rguenther@suse.de>
To: Jan Hubicka <hubicka@ucw.cz>
Cc: gcc-patches@gcc.gnu.org
Subject: Re: [PATCH] Handle OBJ_TYPE_REF in FRE
Date: Thu, 03 Dec 2015 20:46:00 -0000 [thread overview]
Message-ID: <133CDD55-7EFC-41F9-81C1-BEC5717A9984@suse.de> (raw)
In-Reply-To: <20151203174007.GA59701@kam.mff.cuni.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" } }
>*/
next prev parent reply other threads:[~2015-12-03 20:46 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-12-03 10:46 Richard Biener
2015-12-03 17:40 ` Jan Hubicka
2015-12-03 20:46 ` Richard Biener [this message]
2015-12-03 22:52 ` Jan Hubicka
2015-12-04 9:02 ` Richard Biener
2015-12-04 9:31 ` Jan Hubicka
2015-12-04 9:40 ` Richard Biener
2015-12-04 16:59 ` Jan Hubicka
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=133CDD55-7EFC-41F9-81C1-BEC5717A9984@suse.de \
--to=rguenther@suse.de \
--cc=gcc-patches@gcc.gnu.org \
--cc=hubicka@ucw.cz \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).