Hi,
On Sat, Dec 14, 2013 at 11:01:53PM +0100, Jan Hubicka wrote:
> Hi,
> this patch makes stmt_may_be_vtbl_ptr_store to skip clobbers as discussed
> previously.
This is the first time I hear about this but the change is obviously
OK, thanks.
> Martin, I do not fully follow the logic of this function. Can't we just
> ignore all stores that are not of pointer type that looks like vptr
> type? All those tores are compiler generated and we probably can just
> annotate them, right?
Richard wanted me to be quite conservative here and to a big extent I
can see his point. The AGGREGATE_TYPE_P is there because copies of
the whole object could change the vptr in an unpredictable ways. I
doubt that this is what copy-constructors get expanded to but gimple
does not provide any necessary guarantees. Perhaps we could come up
with an elaborate explanation why it cannot happen in valid C++ just
as I did for the call statement (and that is written in the comment
above the function) and then we could remove it.
Of course, having gimple defined and required annotation for vptr
changes would be much better but then of course all transformations
would have to make sure they preserve it.
IIRC, flag_strict_aliasing test was explicitely Richard's request,
perhaps we could restrict the pointer type test further.
Does that answer your question?
Martin
>
> Bootstrapped/regtested x86_64-linux, comitted.
>
> Honza
>
> * ipa-prop.c (stmt_may_be_vtbl_ptr_store): Skip clobbers.
> * g++.dg/ipa/devirt-19.C: New testcase.
> Index: ipa-prop.c
> ===================================================================
> --- ipa-prop.c (revision 205987)
> +++ ipa-prop.c (working copy)
> @@ -560,6 +560,8 @@ stmt_may_be_vtbl_ptr_store (gimple stmt)
> {
> if (is_gimple_call (stmt))
> return false;
> + else if (gimple_clobber_p (stmt))
> + return false;
> else if (is_gimple_assign (stmt))
> {
> tree lhs = gimple_assign_lhs (stmt);
> Index: testsuite/g++.dg/ipa/devirt-19.C
> ===================================================================
> --- testsuite/g++.dg/ipa/devirt-19.C (revision 0)
> +++ testsuite/g++.dg/ipa/devirt-19.C (revision 0)
> @@ -0,0 +1,32 @@
> +/* We should specialize for &b and devirtualize the call.
> + Previously we were failing by considering CLOBBER statement to be
> + a type change. */
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -fdump-ipa-cp" } */
> +struct A {
> + void operator==(const A &);
> +};
> +class B {
> +public:
> + A m_fn1();
> + A m_fn2();
> +};
> +template <typename T, typename M> class C {
> +public:
> + T Key;
> + const M &m_fn2(const T &);
> + virtual void m_fn1() {}
> + B _map;
> +};
> +
> +C<int, int> b;
> +template <typename T, typename M> const M &C<T, M>::m_fn2(const T &) {
> + A a = _map.m_fn2();
> + a == _map.m_fn1();
> + m_fn1();
> +}
> +
> +void fn1() { b.m_fn2(0); }
> +/* { dg-final { scan-ipa-dump-times "Discovered a virtual call to a known
> target" 1 "cp" } } */
> +/* { dg-final { cleanup-ipa-dump "cp" } } */
> +
