* [C++ Patch] for c++/11750
@ 2012-08-13 20:49 Fabien Chêne
2012-11-06 19:59 ` Fabien Chêne
2012-11-08 22:16 ` Jason Merrill
0 siblings, 2 replies; 3+ messages in thread
From: Fabien Chêne @ 2012-08-13 20:49 UTC (permalink / raw)
To: Jason Merrill; +Cc: GCC Patches
[-- Attachment #1: Type: text/plain, Size: 1160 bytes --]
Hi,
Here, we were setting the LOOKUP_NONVIRTUAL flag wrongly. Actually, we
need to check if the function context is the same than the instance
type -- yes that might happen that they be different in presence of
using-declarations.
It happens that it was working if the call was invoked through a
pointer, that's because we were failing to determine the dynamic type
(in resolved_fixed_type_p). On the contrary, it wasn't working if the
call was done through a reference because we manage to determine the
dynamic_type thanks to a special case in fixed_type_or_null. There is
probably room for improvement here, though I'm not sure the C++ front
end is the better place to de-virtualize.
Tested x84_64-unknown-linux-gnu without regressions. OK to commit ?
gcc/testsuite/ChangeLog
2012-08-12 Fabien Chêne <fabien@gcc.gnu.org>
PR c++/11750
* g++.dg/inherit/vitual9.C: New.
gcc/cp/ChangeLog
2012-08-12 Fabien Chêne <fabien@gcc.gnu.org>
PR c++/11750
* call.c (build_new_method_call_1): Check that the instance type
and the function context are the same before setting the flag
LOOKUP_NONVIRTUAL.
--
Fabien
[-- Attachment #2: pr11750.patch --]
[-- Type: application/octet-stream, Size: 1699 bytes --]
Index: cp/call.c
===================================================================
--- cp/call.c (revision 188293)
+++ cp/call.c (working copy)
@@ -7449,9 +7449,14 @@
}
else
{
- /* Optimize away vtable lookup if we know that this function
- can't be overridden. */
+ /* Optimize away vtable lookup if we know that this
+ function can't be overridden. We need to check if
+ the context and the instance type are the same,
+ actually FN migth be defined in a different class
+ type because of a using-declaration. In this case, we
+ do not want to perform a non-virtual call. */
if (DECL_VINDEX (fn) && ! (flags & LOOKUP_NONVIRTUAL)
+ && same_type_p (DECL_CONTEXT (fn), TREE_TYPE (instance))
&& resolves_to_fixed_type_p (instance, 0))
flags |= LOOKUP_NONVIRTUAL;
if (explicit_targs)
Index: testsuite/g++.dg/inherit/virtual9.C
===================================================================
--- testsuite/g++.dg/inherit/virtual9.C (revision 0)
+++ testsuite/g++.dg/inherit/virtual9.C (revision 0)
@@ -0,0 +1,44 @@
+// { dg-do run }
+// PR c++/11750
+
+struct A
+{
+ virtual void f() const { __builtin_abort(); }
+ virtual void g() {}
+};
+
+struct B : virtual A
+{
+ virtual void f() const {}
+ virtual void g() { __builtin_abort(); }
+};
+
+struct C : B, virtual A
+{
+ using A::f;
+ using A::g;
+};
+
+int main()
+{
+ C c;
+ c.f(); // call B::f
+
+ C c2;
+ c2.C::g(); // call A::g
+
+ C* c3 = &c;
+ c3->f(); // call B::f
+
+ C& c4 = c;
+ c4.f(); // call B::f
+
+ C const* c5 = &c;
+ c5->f(); // call B::f
+
+ C** c6 = &c3;
+ (*c6)->f(); // call B::f
+
+ C const& c7 = c;
+ c7.f(); // call B::f
+}
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [C++ Patch] for c++/11750
2012-08-13 20:49 [C++ Patch] for c++/11750 Fabien Chêne
@ 2012-11-06 19:59 ` Fabien Chêne
2012-11-08 22:16 ` Jason Merrill
1 sibling, 0 replies; 3+ messages in thread
From: Fabien Chêne @ 2012-11-06 19:59 UTC (permalink / raw)
To: Jason Merrill; +Cc: GCC Patches
Jason, could you please have a look at this (rather old) one ?
Thanks.
2012/8/13 Fabien Chêne <fabien.chene@gmail.com>:
> Hi,
>
> Here, we were setting the LOOKUP_NONVIRTUAL flag wrongly. Actually, we
> need to check if the function context is the same than the instance
> type -- yes that might happen that they be different in presence of
> using-declarations.
>
> It happens that it was working if the call was invoked through a
> pointer, that's because we were failing to determine the dynamic type
> (in resolved_fixed_type_p). On the contrary, it wasn't working if the
> call was done through a reference because we manage to determine the
> dynamic_type thanks to a special case in fixed_type_or_null. There is
> probably room for improvement here, though I'm not sure the C++ front
> end is the better place to de-virtualize.
>
> Tested x84_64-unknown-linux-gnu without regressions. OK to commit ?
>
> gcc/testsuite/ChangeLog
>
> 2012-08-12 Fabien Chêne <fabien@gcc.gnu.org>
>
> PR c++/11750
> * g++.dg/inherit/vitual9.C: New.
>
> gcc/cp/ChangeLog
>
> 2012-08-12 Fabien Chêne <fabien@gcc.gnu.org>
>
> PR c++/11750
> * call.c (build_new_method_call_1): Check that the instance type
> and the function context are the same before setting the flag
> LOOKUP_NONVIRTUAL.
>
>
> --
> Fabien
--
Fabien
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [C++ Patch] for c++/11750
2012-08-13 20:49 [C++ Patch] for c++/11750 Fabien Chêne
2012-11-06 19:59 ` Fabien Chêne
@ 2012-11-08 22:16 ` Jason Merrill
1 sibling, 0 replies; 3+ messages in thread
From: Jason Merrill @ 2012-11-08 22:16 UTC (permalink / raw)
To: Fabien Chêne; +Cc: GCC Patches
On 08/13/2012 04:49 PM, Fabien Chêne wrote:
> + /* Optimize away vtable lookup if we know that this
> + function can't be overridden. We need to check if
> + the context and the instance type are the same,
> + actually FN migth be defined in a different class
Typo: "might"
> + type because of a using-declaration. In this case, we
> + do not want to perform a non-virtual call. */
> if (DECL_VINDEX (fn) && ! (flags & LOOKUP_NONVIRTUAL)
> + && same_type_p (DECL_CONTEXT (fn), TREE_TYPE (instance))
> && resolves_to_fixed_type_p (instance, 0))
This should be same_type_ignoring_top_level_qualifiers_p.
OK with that change.
Jason
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2012-11-08 22:16 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-08-13 20:49 [C++ Patch] for c++/11750 Fabien Chêne
2012-11-06 19:59 ` Fabien Chêne
2012-11-08 22:16 ` Jason Merrill
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).