* [C++ Patch] PR 90909 ("[10 Regression] call devirtualized to pure virtual")
@ 2019-06-20 4:24 Paolo Carlini
2019-06-21 18:50 ` Jason Merrill
0 siblings, 1 reply; 13+ messages in thread
From: Paolo Carlini @ 2019-06-20 4:24 UTC (permalink / raw)
To: gcc-patches; +Cc: Jason Merrill
[-- Attachment #1: Type: text/plain, Size: 1123 bytes --]
Hi,
this bug notices that the more aggressive de-virtualization check that
we have now in place (fixed c++/67184) doesn't work correctly for the
below reproducer, which involves a pure virtual: we de-virtualize and
the build fails at link-time. To cure this I believe we simply want an
additional DECL_PURE_VIRTUAL_P in the condition. I also checked that the
other compilers I have at hand appear to do the same, that is, they
compile the reproducer both as-is and without the final specifier to the
same assembly.
Note, in principle we have the option of not doing the additional
DECL_PURE_VIRTUAL_P check when the final overrider comes from the class
itself, not from a base, that is in the cases that we were already
de-virtualizing pre-67184. That is, for something like:
struct A final
{
 virtual void foo () = 0;
};
void fun(A* a, B* b)
{
 a->foo();
}
devirtualize anyway (which then doesn't link). We could add back an '||
CLASSTYPE_FINAL (TYPE_METHOD_BASETYPE (TREE_TYPE (fn)))' for that. ICC
appears to behave this way.
Tested x86_64-linux.
Thanks, Paolo.
////////////////////////
[-- Attachment #2: CL_90909 --]
[-- Type: text/plain, Size: 278 bytes --]
/cp
2019-06-20 Paolo Carlini <paolo.carlini@oracle.com>
PR c++/90909
* call.c (build_over_call): Do not try to devirtualize when
then function is pure virtual.
/testsuite
2019-06-20 Paolo Carlini <paolo.carlini@oracle.com>
PR c++/90909
* g++.dg/other/final6.C: New.
[-- Attachment #3: patch_90909 --]
[-- Type: text/plain, Size: 1083 bytes --]
Index: testsuite/g++.dg/other/final6.C
===================================================================
--- testsuite/g++.dg/other/final6.C (nonexistent)
+++ testsuite/g++.dg/other/final6.C (working copy)
@@ -0,0 +1,9 @@
+// PR c++/90909
+// { dg-do link { target c++11 } }
+
+struct S1 { virtual void f() = 0; };
+struct S2: S1 { virtual void f() {} };
+struct S3: S2 { using S1::f; };
+struct S4 final: S3 { void g(); };
+void S4::g() { f(); }
+int main() { S4().g(); }
Index: cp/call.c
===================================================================
--- cp/call.c (revision 272410)
+++ cp/call.c (working copy)
@@ -8244,7 +8244,8 @@ build_over_call (struct z_candidate *cand, int fla
/* See if the function member or the whole class type is declared
final and the call can be devirtualized. */
if (DECL_FINAL_P (fn)
- || CLASSTYPE_FINAL (TREE_TYPE (argtype)))
+ || (CLASSTYPE_FINAL (TREE_TYPE (argtype))
+ && !DECL_PURE_VIRTUAL_P (fn)))
flags |= LOOKUP_NONVIRTUAL;
/* [class.mfct.nonstatic]: If a nonstatic member function of a class
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [C++ Patch] PR 90909 ("[10 Regression] call devirtualized to pure virtual")
2019-06-20 4:24 [C++ Patch] PR 90909 ("[10 Regression] call devirtualized to pure virtual") Paolo Carlini
@ 2019-06-21 18:50 ` Jason Merrill
2019-06-21 19:29 ` Paolo Carlini
0 siblings, 1 reply; 13+ messages in thread
From: Jason Merrill @ 2019-06-21 18:50 UTC (permalink / raw)
To: Paolo Carlini, gcc-patches
On 6/20/19 12:24 AM, Paolo Carlini wrote:
> Hi,
>
> this bug notices that the more aggressive de-virtualization check that
> we have now in place (fixed c++/67184) doesn't work correctly for the
> below reproducer, which involves a pure virtual: we de-virtualize and
> the build fails at link-time. To cure this I believe we simply want an
> additional DECL_PURE_VIRTUAL_P in the condition.
I don't see why this bug would be specific to pure virtual functions, so
the fix also should not be. If S1::f is not pure virtual, I'd expect
that we wrongly devirtualize to it in the same way.
Devirtualizing the call in S4 is a good optimization, we're just
selecting the wrong function.
It seems like the issue here is that the using-declaration hides the
final overrider from lookup. So we need to work harder to find the
actual final overrider, perhaps by looking into the argtype vtable.
Jason
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [C++ Patch] PR 90909 ("[10 Regression] call devirtualized to pure virtual")
2019-06-21 18:50 ` Jason Merrill
@ 2019-06-21 19:29 ` Paolo Carlini
2019-06-21 19:41 ` Paolo Carlini
0 siblings, 1 reply; 13+ messages in thread
From: Paolo Carlini @ 2019-06-21 19:29 UTC (permalink / raw)
To: Jason Merrill, gcc-patches
Hi,
On 21/06/19 20:50, Jason Merrill wrote:
> On 6/20/19 12:24 AM, Paolo Carlini wrote:
>> Hi,
>>
>> this bug notices that the more aggressive de-virtualization check
>> that we have now in place (fixed c++/67184) doesn't work correctly
>> for the below reproducer, which involves a pure virtual: we
>> de-virtualize and the build fails at link-time. To cure this I
>> believe we simply want an additional DECL_PURE_VIRTUAL_P in the
>> condition.
>
> I don't see why this bug would be specific to pure virtual functions,
> so the fix also should not be. If S1::f is not pure virtual, I'd
> expect that we wrongly devirtualize to it in the same way.
>
> Devirtualizing the call in S4 is a good optimization, we're just
> selecting the wrong function.
>
> It seems like the issue here is that the using-declaration hides the
> final overrider from lookup. So we need to work harder to find the
> actual final overrider, perhaps by looking into the argtype vtable.
I see, thanks for the suggestion.
The issue seems rather tricky, then. For the time being I'm going to
revert the recent improvements. Probably I'm also going to unassign
myself, because I don't want to prevent somebody else more knowledgeable
than me in the area from contributing a good solution.
Thanks, Paolo.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [C++ Patch] PR 90909 ("[10 Regression] call devirtualized to pure virtual")
2019-06-21 19:29 ` Paolo Carlini
@ 2019-06-21 19:41 ` Paolo Carlini
2019-06-21 19:56 ` Paolo Carlini
0 siblings, 1 reply; 13+ messages in thread
From: Paolo Carlini @ 2019-06-21 19:41 UTC (permalink / raw)
To: Jason Merrill, gcc-patches
Hi,
On 21/06/19 21:27, Paolo Carlini wrote:
> Hi,
>
> On 21/06/19 20:50, Jason Merrill wrote:
>> On 6/20/19 12:24 AM, Paolo Carlini wrote:
>>> Hi,
>>>
>>> this bug notices that the more aggressive de-virtualization check
>>> that we have now in place (fixed c++/67184) doesn't work correctly
>>> for the below reproducer, which involves a pure virtual: we
>>> de-virtualize and the build fails at link-time. To cure this I
>>> believe we simply want an additional DECL_PURE_VIRTUAL_P in the
>>> condition.
>>
>> I don't see why this bug would be specific to pure virtual functions,
>> so the fix also should not be. If S1::f is not pure virtual, I'd
>> expect that we wrongly devirtualize to it in the same way.
By the way, if S1:.f is not pure virtual, just a virtual declaration -
all the rest being the same - the code doesn't link: undefined
references to vtable and typeinfo for S1. The same happens with current
clang and icc. I don't know if this detail helps us in the short term....
Paolo.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [C++ Patch] PR 90909 ("[10 Regression] call devirtualized to pure virtual")
2019-06-21 19:41 ` Paolo Carlini
@ 2019-06-21 19:56 ` Paolo Carlini
2019-06-21 21:53 ` Paolo Carlini
0 siblings, 1 reply; 13+ messages in thread
From: Paolo Carlini @ 2019-06-21 19:56 UTC (permalink / raw)
To: Jason Merrill, gcc-patches
and...
> By the way, if S1:.f is not pure virtual, just a virtual declaration -
> all the rest being the same - the code doesn't link: undefined
> references to vtable and typeinfo for S1. The same happens with
> current clang and icc. I don't know if this detail helps us in the
> short term....
... the same happens without final too.
Paolo.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [C++ Patch] PR 90909 ("[10 Regression] call devirtualized to pure virtual")
2019-06-21 19:56 ` Paolo Carlini
@ 2019-06-21 21:53 ` Paolo Carlini
2019-06-23 11:53 ` Paolo Carlini
0 siblings, 1 reply; 13+ messages in thread
From: Paolo Carlini @ 2019-06-21 21:53 UTC (permalink / raw)
To: Jason Merrill, gcc-patches
[-- Attachment #1: Type: text/plain, Size: 135 bytes --]
... so, now that I see the issue more clearly, I'm adding to the
testsuite the below too.
Thanks, Paolo.
////////////////////////
[-- Attachment #2: p --]
[-- Type: text/plain, Size: 472 bytes --]
Index: final7.C
===================================================================
--- final7.C (nonexistent)
+++ final7.C (working copy)
@@ -0,0 +1,11 @@
+// PR c++/90909
+// { dg-do run { target c++11 } }
+
+#include <cassert>
+
+struct S1 { virtual bool f() { return false; } };
+struct S2: S1 { virtual bool f() { return true; } };
+struct S3: S2 { using S1::f; };
+struct S4 final: S3 { void g(); };
+void S4::g() { assert (f() == true); }
+int main() { S4().g(); }
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [C++ Patch] PR 90909 ("[10 Regression] call devirtualized to pure virtual")
2019-06-21 21:53 ` Paolo Carlini
@ 2019-06-23 11:53 ` Paolo Carlini
2019-06-23 17:45 ` Jason Merrill
0 siblings, 1 reply; 13+ messages in thread
From: Paolo Carlini @ 2019-06-23 11:53 UTC (permalink / raw)
To: Jason Merrill, gcc-patches
[-- Attachment #1: Type: text/plain, Size: 805 bytes --]
... hi again ;)
The other day I was having a look at using declarations for this issue
and noticed that only a few lines below the de-virtualization check we
have to handle functions found by a using declaration, for various
reasons. In particular, we know whether we found a function fn where has
been declared or in a derived class. Thus the idea: for the purpose of
making some progress, in particular all the cases in c++/67184 & co,
would it make sense for the time being to simply add a check to the
de-virtualization condition restricting it to non-using declarations?
See the below (it also moves the conditional a few lines below only for
clarity and consistency with the code handling using declarations, no
functional impact) What do you think?
Thanks, Paolo.
///////////////////
[-- Attachment #2: patch_67184_again --]
[-- Type: text/plain, Size: 3179 bytes --]
Index: cp/call.c
===================================================================
--- cp/call.c (revision 272583)
+++ cp/call.c (working copy)
@@ -8241,12 +8241,6 @@ build_over_call (struct z_candidate *cand, int fla
return error_mark_node;
}
- /* See if the function member or the whole class type is declared
- final and the call can be devirtualized. */
- if (DECL_FINAL_P (fn)
- || CLASSTYPE_FINAL (TYPE_METHOD_BASETYPE (TREE_TYPE (fn))))
- flags |= LOOKUP_NONVIRTUAL;
-
/* [class.mfct.nonstatic]: If a nonstatic member function of a class
X is called for an object that is not of type X, or of a type
derived from X, the behavior is undefined.
@@ -8271,6 +8265,17 @@ build_over_call (struct z_candidate *cand, int fla
else
return error_mark_node;
}
+
+ /* See if the function member or the whole class type is declared
+ final and the call can be devirtualized. */
+ if (DECL_FINAL_P (fn)
+ || (CLASSTYPE_FINAL (TREE_TYPE (argtype))
+ /* Give up for now if fn was found by a using declaration,
+ the complex case, see c++/90909. */
+ && (TREE_TYPE (TREE_TYPE (converted_arg))
+ == TREE_TYPE (parmtype))))
+ flags |= LOOKUP_NONVIRTUAL;
+
/* If fn was found by a using declaration, the conversion path
will be to the derived class, not the base declaring fn. We
must convert from derived to base. */
Index: testsuite/g++.dg/other/final3.C
===================================================================
--- testsuite/g++.dg/other/final3.C (nonexistent)
+++ testsuite/g++.dg/other/final3.C (working copy)
@@ -0,0 +1,28 @@
+// PR c++/67184
+// { dg-do compile { target c++11 } }
+// { dg-options "-fdump-tree-original" }
+
+struct V {
+ virtual void foo();
+};
+
+struct wV final : V {
+};
+
+struct oV final : V {
+ void foo();
+};
+
+void call(wV& x)
+{
+ x.foo();
+ x.V::foo();
+}
+
+void call(oV& x)
+{
+ x.foo();
+ x.V::foo();
+}
+
+// { dg-final { scan-tree-dump-times "OBJ_TYPE_REF" 0 "original" } }
Index: testsuite/g++.dg/other/final4.C
===================================================================
--- testsuite/g++.dg/other/final4.C (nonexistent)
+++ testsuite/g++.dg/other/final4.C (working copy)
@@ -0,0 +1,16 @@
+// PR c++/67184
+// { dg-do compile { target c++11 } }
+// { dg-options "-fdump-tree-original" }
+
+struct B
+{
+ virtual void operator()();
+ virtual operator int();
+ virtual int operator++();
+};
+
+struct D final : B { };
+
+void foo(D& d) { d(); int t = d; ++d; }
+
+// { dg-final { scan-tree-dump-times "OBJ_TYPE_REF" 0 "original" } }
Index: testsuite/g++.dg/other/final5.C
===================================================================
--- testsuite/g++.dg/other/final5.C (nonexistent)
+++ testsuite/g++.dg/other/final5.C (working copy)
@@ -0,0 +1,19 @@
+// PR c++/69445
+// { dg-do compile { target c++11 } }
+// { dg-options "-fdump-tree-original" }
+
+struct Base {
+ virtual void foo() const = 0;
+ virtual void bar() const {}
+};
+
+struct C final : Base {
+ void foo() const { }
+};
+
+void func(const C & c) {
+ c.bar();
+ c.foo();
+}
+
+// { dg-final { scan-tree-dump-times "OBJ_TYPE_REF" 0 "original" } }
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [C++ Patch] PR 90909 ("[10 Regression] call devirtualized to pure virtual")
2019-06-23 11:53 ` Paolo Carlini
@ 2019-06-23 17:45 ` Jason Merrill
2019-06-24 8:54 ` Paolo Carlini
0 siblings, 1 reply; 13+ messages in thread
From: Jason Merrill @ 2019-06-23 17:45 UTC (permalink / raw)
To: Paolo Carlini, gcc-patches
On 6/23/19 7:53 AM, Paolo Carlini wrote:
> ... hi again ;)
>
> The other day I was having a look at using declarations for this issue
> and noticed that only a few lines below the de-virtualization check we
> have to handle functions found by a using declaration, for various
> reasons. In particular, we know whether we found a function fn where has
> been declared or in a derived class. Thus the idea: for the purpose of
> making some progress, in particular all the cases in c++/67184 & co,
> would it make sense for the time being to simply add a check to the
> de-virtualization condition restricting it to non-using declarations?
> See the below (it also moves the conditional a few lines below only for
> clarity and consistency with the code handling using declarations, no
> functional impact) What do you think?
Hmm, perhaps we should check CLASSTYPE_FINAL in resolves_to_fixed_type_p
rather than in build_over_call at all; then the code in
build_new_method_call ought to set LOOKUP_NONVIRTUAL when appropriate.
Jason
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [C++ Patch] PR 90909 ("[10 Regression] call devirtualized to pure virtual")
2019-06-23 17:45 ` Jason Merrill
@ 2019-06-24 8:54 ` Paolo Carlini
2019-06-27 21:19 ` Jason Merrill
0 siblings, 1 reply; 13+ messages in thread
From: Paolo Carlini @ 2019-06-24 8:54 UTC (permalink / raw)
To: Jason Merrill, gcc-patches
Hi,
On 23/06/19 19:45, Jason Merrill wrote:
> On 6/23/19 7:53 AM, Paolo Carlini wrote:
>> ... hi again ;)
>>
>> The other day I was having a look at using declarations for this
>> issue and noticed that only a few lines below the de-virtualization
>> check we have to handle functions found by a using declaration, for
>> various reasons. In particular, we know whether we found a function
>> fn where has been declared or in a derived class. Thus the idea: for
>> the purpose of making some progress, in particular all the cases in
>> c++/67184 & co, would it make sense for the time being to simply add
>> a check to the de-virtualization condition restricting it to
>> non-using declarations? See the below (it also moves the conditional
>> a few lines below only for clarity and consistency with the code
>> handling using declarations, no functional impact) What do you think?
>
> Hmm, perhaps we should check CLASSTYPE_FINAL in
> resolves_to_fixed_type_p rather than in build_over_call at all; then
> the code in build_new_method_call ought to set LOOKUP_NONVIRTUAL when
> appropriate.
I think your suggestion has to do with the initial implementation of
this optimization, as contributed by my friend Roberto Agostino: we had
the issue that it didn't handle at all user-defined operators and
Vincenzo filed c++/53186. Thus, upon your suggestion, we moved the code
to build_over_call, the current place:
   https://gcc.gnu.org/ml/gcc-patches/2012-05/msg00246.html
where it catched both member functions and operators. Now - before we
get to the details - if I move the CLASSTYPE_FINAL check to
resolves_to_fixed_type_p we exactly regress on c++/53186, that is
other/final2.C, because resolves_to_fixed_type_p is *never* called. The
pending final4.C, also involving operators (I constructed it exactly
because I knew operators could be tricky) is also not fixed, but in that
case at least resolves_to_fixed_type_p *is* called, only, too late (I
think, I more details later, if you like).
All the other existing and pending testcases - involving member
functions - appear to be Ok, even with a draft implementation of your
suggestion (I slapped a 'if (CLASS_TYPE_P (t) && CLASSTYPE_FINAL (t))
return true;' in the middle of resolves_to_fixed_type_p.
Thanks, Paolo.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [C++ Patch] PR 90909 ("[10 Regression] call devirtualized to pure virtual")
2019-06-24 8:54 ` Paolo Carlini
@ 2019-06-27 21:19 ` Jason Merrill
2019-07-04 13:23 ` Paolo Carlini
0 siblings, 1 reply; 13+ messages in thread
From: Jason Merrill @ 2019-06-27 21:19 UTC (permalink / raw)
To: Paolo Carlini, gcc-patches
On 6/24/19 4:52 AM, Paolo Carlini wrote:
> Hi,
>
> On 23/06/19 19:45, Jason Merrill wrote:
>> On 6/23/19 7:53 AM, Paolo Carlini wrote:
>>> ... hi again ;)
>>>
>>> The other day I was having a look at using declarations for this
>>> issue and noticed that only a few lines below the de-virtualization
>>> check we have to handle functions found by a using declaration, for
>>> various reasons. In particular, we know whether we found a function
>>> fn where has been declared or in a derived class. Thus the idea: for
>>> the purpose of making some progress, in particular all the cases in
>>> c++/67184 & co, would it make sense for the time being to simply add
>>> a check to the de-virtualization condition restricting it to
>>> non-using declarations? See the below (it also moves the conditional
>>> a few lines below only for clarity and consistency with the code
>>> handling using declarations, no functional impact) What do you think?
>>
>> Hmm, perhaps we should check CLASSTYPE_FINAL in
>> resolves_to_fixed_type_p rather than in build_over_call at all; then
>> the code in build_new_method_call ought to set LOOKUP_NONVIRTUAL when
>> appropriate.
>
> I think your suggestion has to do with the initial implementation of
> this optimization, as contributed by my friend Roberto Agostino: we had
> the issue that it didn't handle at all user-defined operators and
> Vincenzo filed c++/53186. Thus, upon your suggestion, we moved the code
> to build_over_call, the current place:
>
> Â Â Â https://gcc.gnu.org/ml/gcc-patches/2012-05/msg00246.html
>
> where it catched both member functions and operators. Now - before we
> get to the details - if I move the CLASSTYPE_FINAL check to
> resolves_to_fixed_type_p we exactly regress on c++/53186, that is
> other/final2.C, because resolves_to_fixed_type_p is *never* called. The
> pending final4.C, also involving operators (I constructed it exactly
> because I knew operators could be tricky) is also not fixed, but in that
> case at least resolves_to_fixed_type_p *is* called, only, too late (I
> think, I more details later, if you like).
Ah, thanks. Then perhaps we want to change the CLASSTYPE_FINAL in
build_over_call to resolves_to_fixed_type_p (arg), to also handle the
other reasons we might know the dynamic type of the argument, and remove
the related code from build_new_method_call_1?
You could avoid needing to move the conditional lower by comparing
DECL_CONTEXT (fn) and BINFO_TYPE (cand->conversion_path) rather than
parmtype and TREE_TYPE (converted_arg).
Jason
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [C++ Patch] PR 90909 ("[10 Regression] call devirtualized to pure virtual")
2019-06-27 21:19 ` Jason Merrill
@ 2019-07-04 13:23 ` Paolo Carlini
2019-07-05 14:05 ` Jason Merrill
2019-07-06 1:03 ` Jakub Jelinek
0 siblings, 2 replies; 13+ messages in thread
From: Paolo Carlini @ 2019-07-04 13:23 UTC (permalink / raw)
To: Jason Merrill, gcc-patches
[-- Attachment #1: Type: text/plain, Size: 1063 bytes --]
Hi,
On 27/06/19 23:19, Jason Merrill wrote:
> Ah, thanks. Then perhaps we want to change the CLASSTYPE_FINAL in
> build_over_call to resolves_to_fixed_type_p (arg), to also handle the
> other reasons we might know the dynamic type of the argument, and
> remove the related code from build_new_method_call_1?
>
> You could avoid needing to move the conditional lower by comparing
> DECL_CONTEXT (fn) and BINFO_TYPE (cand->conversion_path) rather than
> parmtype and TREE_TYPE (converted_arg).
Sorry for late replying, a few days off.
Anyway, great, it looks like we are reaching a nice synthesis. I must
admit that until yesterday I hadn't noticed that Fabien dealt precisely
with using declarations in order to fix c++/11750, thus the existing
check in build_new_method_call_1 is exactly what we need. The below does
that and passes testing, in it I didn't keep the checks of DECL_VINDEX
(fn) && ! (flags & LOOKUP_NONVIRTUAL) which don't seem necessary, might
avoid function calls, though. Let me know...
Thanks, Paolo.
////////////////////
[-- Attachment #2: patch_67184_new --]
[-- Type: text/plain, Size: 2549 bytes --]
Index: cp/call.c
===================================================================
--- cp/call.c (revision 273076)
+++ cp/call.c (working copy)
@@ -8241,10 +8241,17 @@ build_over_call (struct z_candidate *cand, int fla
return error_mark_node;
}
- /* See if the function member or the whole class type is declared
- final and the call can be devirtualized. */
+ /* Optimize away vtable lookup if we know that this
+ function can't be overridden. We need to check if
+ the context and the type where we found fn are the same,
+ actually FN might 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. Note that
+ resolves_to_fixed_type_p checks CLASSTYPE_FINAL too. */
if (DECL_FINAL_P (fn)
- || CLASSTYPE_FINAL (TYPE_METHOD_BASETYPE (TREE_TYPE (fn))))
+ || (resolves_to_fixed_type_p (arg, 0)
+ && same_type_ignoring_top_level_qualifiers_p
+ (DECL_CONTEXT (fn), BINFO_TYPE (cand->conversion_path))))
flags |= LOOKUP_NONVIRTUAL;
/* [class.mfct.nonstatic]: If a nonstatic member function of a class
@@ -9845,17 +9852,6 @@ build_new_method_call_1 (tree instance, tree fns,
if (call != error_mark_node)
{
- /* Optimize away vtable lookup if we know that this
- function can't be overridden. We need to check if
- the context and the type where we found fn are the same,
- actually FN might 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_ignoring_top_level_qualifiers_p
- (DECL_CONTEXT (fn), BINFO_TYPE (binfo))
- && resolves_to_fixed_type_p (instance, 0))
- flags |= LOOKUP_NONVIRTUAL;
if (explicit_targs)
flags |= LOOKUP_EXPLICIT_TMPL_ARGS;
/* Now we know what function is being called. */
Index: testsuite/g++.dg/other/final4.C
===================================================================
--- testsuite/g++.dg/other/final4.C (nonexistent)
+++ testsuite/g++.dg/other/final4.C (working copy)
@@ -0,0 +1,16 @@
+// PR c++/67184
+// { dg-do compile { target c++11 } }
+// { dg-options "-fdump-tree-original" }
+
+struct B
+{
+ virtual void operator()();
+ virtual operator int();
+ virtual int operator++();
+};
+
+struct D final : B { };
+
+void foo(D& d) { d(); int t = d; ++d; }
+
+// { dg-final { scan-tree-dump-times "OBJ_TYPE_REF" 0 "original" } }
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [C++ Patch] PR 90909 ("[10 Regression] call devirtualized to pure virtual")
2019-07-04 13:23 ` Paolo Carlini
@ 2019-07-05 14:05 ` Jason Merrill
2019-07-06 1:03 ` Jakub Jelinek
1 sibling, 0 replies; 13+ messages in thread
From: Jason Merrill @ 2019-07-05 14:05 UTC (permalink / raw)
To: Paolo Carlini, gcc-patches
On 7/4/19 8:56 AM, Paolo Carlini wrote:
> Hi,
>
> On 27/06/19 23:19, Jason Merrill wrote:
>> Ah, thanks. Then perhaps we want to change the CLASSTYPE_FINAL in
>> build_over_call to resolves_to_fixed_type_p (arg), to also handle the
>> other reasons we might know the dynamic type of the argument, and
>> remove the related code from build_new_method_call_1?
>>
>> You could avoid needing to move the conditional lower by comparing
>> DECL_CONTEXT (fn) and BINFO_TYPE (cand->conversion_path) rather than
>> parmtype and TREE_TYPE (converted_arg).
>
> Sorry for late replying, a few days off.
>
> Anyway, great, it looks like we are reaching a nice synthesis. I must
> admit that until yesterday I hadn't noticed that Fabien dealt precisely
> with using declarations in order to fix c++/11750, thus the existing
> check in build_new_method_call_1 is exactly what we need. The below does
> that and passes testing, in it I didn't keep the checks of DECL_VINDEX
> (fn) && ! (flags & LOOKUP_NONVIRTUAL) which don't seem necessary, might
> avoid function calls, though. Let me know...
Yeah, they're an optimization to avoid the calls when they aren't
necessary but I wouldn't expect that the difference is measurable. The
patch is OK, thanks.
Jason
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [C++ Patch] PR 90909 ("[10 Regression] call devirtualized to pure virtual")
2019-07-04 13:23 ` Paolo Carlini
2019-07-05 14:05 ` Jason Merrill
@ 2019-07-06 1:03 ` Jakub Jelinek
1 sibling, 0 replies; 13+ messages in thread
From: Jakub Jelinek @ 2019-07-06 1:03 UTC (permalink / raw)
To: Paolo Carlini; +Cc: Jason Merrill, gcc-patches
On Thu, Jul 04, 2019 at 02:56:47PM +0200, Paolo Carlini wrote:
> --- cp/call.c (revision 273076)
> +++ cp/call.c (working copy)
> @@ -9845,17 +9852,6 @@ build_new_method_call_1 (tree instance, tree fns,
>
> if (call != error_mark_node)
> {
> - /* Optimize away vtable lookup if we know that this
> - function can't be overridden. We need to check if
> - the context and the type where we found fn are the same,
> - actually FN might 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_ignoring_top_level_qualifiers_p
> - (DECL_CONTEXT (fn), BINFO_TYPE (binfo))
> - && resolves_to_fixed_type_p (instance, 0))
> - flags |= LOOKUP_NONVIRTUAL;
> if (explicit_targs)
> flags |= LOOKUP_EXPLICIT_TMPL_ARGS;
> /* Now we know what function is being called. */
This change broke bootstrap, as it removes the last use of binfo
variable besides the setter of that variable.
I'll commit following as obvious if I get successfully past that point in
bootstrap:
2019-07-05 Jakub Jelinek <jakub@redhat.com>
PR c++/67184
PR c++/69445
* call.c (build_new_method_call_1): Remove set but not used variable
binfo.
--- gcc/call.c.jj 2019-07-05 22:09:49.694367815 +0200
+++ gcc/call.c 2019-07-05 22:25:58.476016114 +0200
@@ -9564,7 +9564,7 @@ build_new_method_call_1 (tree instance,
struct z_candidate *candidates = 0, *cand;
tree explicit_targs = NULL_TREE;
tree basetype = NULL_TREE;
- tree access_binfo, binfo;
+ tree access_binfo;
tree optype;
tree first_mem_arg = NULL_TREE;
tree name;
@@ -9603,7 +9603,6 @@ build_new_method_call_1 (tree instance,
if (!conversion_path)
conversion_path = BASELINK_BINFO (fns);
access_binfo = BASELINK_ACCESS_BINFO (fns);
- binfo = BASELINK_BINFO (fns);
optype = BASELINK_OPTYPE (fns);
fns = BASELINK_FUNCTIONS (fns);
if (TREE_CODE (fns) == TEMPLATE_ID_EXPR)
Jakub
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2019-07-05 20:31 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-06-20 4:24 [C++ Patch] PR 90909 ("[10 Regression] call devirtualized to pure virtual") Paolo Carlini
2019-06-21 18:50 ` Jason Merrill
2019-06-21 19:29 ` Paolo Carlini
2019-06-21 19:41 ` Paolo Carlini
2019-06-21 19:56 ` Paolo Carlini
2019-06-21 21:53 ` Paolo Carlini
2019-06-23 11:53 ` Paolo Carlini
2019-06-23 17:45 ` Jason Merrill
2019-06-24 8:54 ` Paolo Carlini
2019-06-27 21:19 ` Jason Merrill
2019-07-04 13:23 ` Paolo Carlini
2019-07-05 14:05 ` Jason Merrill
2019-07-06 1:03 ` Jakub Jelinek
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).