On Fri, 20 May 2016, Jakub Jelinek wrote:
> Hi!
>
> This is another case in the never ending story of dropping lhs of noreturn
> calls when we shouldn't.
>
> Though, in this case, while we can optimize a call to a direct call to
> normal [[noreturn]] method, we can also optimize into __cxa_pure_virtual
> or __builtin_unreachable. And in those cases IMHO it is desirable to
> not have the lhs, but we should also adjust gimple_call_set_fntype,
> because we are now calling something different, we've just reused the
> same call stmt for that.
Hmm, I think for devirt we can unconditionally adjust fntype. It should
be impossible to create a wrongly typed virtual call in source
(maybe with the help of LTO and some ODR violations).
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
Ok with doing the set_fntype unconditionally. If you think it's safer
the way you did it that's also ok.
Richard.
> 2016-05-20 Jakub Jelinek <ja...@redhat.com>
>
> PR c++/71210
> * gimple-fold.c (gimple_fold_call): Do not remove lhs of noreturn
> calls if the LHS is variable length or has addressable type.
> If targets[0]->decl is a noreturn call with void return type and
> zero arguments, adjust fntype and remove lhs in that case.
>
> * g++.dg/opt/pr71210-1.C: New test.
> * g++.dg/opt/pr71210-2.C: New test.
>
> --- gcc/gimple-fold.c.jj 2016-05-03 14:12:19.000000000 +0200
> +++ gcc/gimple-fold.c 2016-05-20 10:18:22.818728240 +0200
> @@ -3039,10 +3039,25 @@ gimple_fold_call (gimple_stmt_iterator *
> }
> if (targets.length () == 1)
> {
> - gimple_call_set_fndecl (stmt, targets[0]->decl);
> + tree fndecl = targets[0]->decl;
> + gimple_call_set_fndecl (stmt, fndecl);
> changed = true;
> + /* If changing the call to __cxa_pure_virtual
> + or similar noreturn function, adjust gimple_call_fntype
> + too. */
> + if ((gimple_call_flags (stmt) & ECF_NORETURN)
> + && VOID_TYPE_P (TREE_TYPE (TREE_TYPE (fndecl)))
> + && TYPE_ARG_TYPES (TREE_TYPE (fndecl))
> + && (TREE_VALUE (TYPE_ARG_TYPES (TREE_TYPE (fndecl)))
> + == void_type_node))
> + gimple_call_set_fntype (stmt, TREE_TYPE (fndecl));
> /* If the call becomes noreturn, remove the lhs. */
> - if (lhs && (gimple_call_flags (stmt) & ECF_NORETURN))
> + if (lhs
> + && (gimple_call_flags (stmt) & ECF_NORETURN)
> + && (VOID_TYPE_P (TREE_TYPE (gimple_call_fntype (stmt)))
> + || ((TREE_CODE (TYPE_SIZE_UNIT (TREE_TYPE (lhs)))
> + == INTEGER_CST)
> + && !TREE_ADDRESSABLE (TREE_TYPE (lhs)))))
> {
> if (TREE_CODE (lhs) == SSA_NAME)
> {
> --- gcc/testsuite/g++.dg/opt/pr71210-1.C.jj 2016-05-20 09:30:13.119724805
> +0200
> +++ gcc/testsuite/g++.dg/opt/pr71210-1.C 2016-05-20 09:29:42.000000000
> +0200
> @@ -0,0 +1,14 @@
> +// PR c++/71210
> +// { dg-do compile }
> +// { dg-options "-O2" }
> +
> +#include <typeinfo>
> +
> +void f1 (const std::type_info&) __attribute__((noreturn));
> +struct S1 { ~S1 (); };
> +struct S2
> +{
> + virtual S1 f2 () const { f1 (typeid (*this)); }
> + S1 f3 () const { return f2 (); }
> +};
> +void f4 () { S2 a; a.f3 (); }
> --- gcc/testsuite/g++.dg/opt/pr71210-2.C.jj 2016-05-20 10:20:00.918402232
> +0200
> +++ gcc/testsuite/g++.dg/opt/pr71210-2.C 2016-05-20 10:19:48.000000000
> +0200
> @@ -0,0 +1,23 @@
> +// PR c++/71210
> +// { dg-do compile }
> +// { dg-options "-O2" }
> +
> +struct C { int a; int b; C (); ~C (); };
> +
> +namespace
> +{
> + struct A
> + {
> + A () {}
> + virtual C bar (int) = 0;
> + C baz (int x) { return bar (x); }
> + };
> +}
> +
> +A *a;
> +
> +void
> +foo ()
> +{
> + C c = a->baz (0);
> +}
>
> Jakub
>
>
--
Richard Biener <rguent...@suse.de>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB
21284 (AG Nuernberg)
