Message ID | 67378ced-6d34-2617-d933-2adba3872ae4@oracle.com |
---|---|
State | New |
Headers | show |
Series | [C++] PR 67184 ("Missed optimization with C++11 final specifier") | expand |
On 5/16/19 7:12 PM, Paolo Carlini wrote: > Hi, > > when Roberto Agostino and I implemented the front-end devirtualization > of final overriders we missed this case, where it comes from the base. > It seems to me that by way of access_path the existing approach can be > neatly extended. Tested x86_64-linux. > + || CLASSTYPE_FINAL (TREE_TYPE (cand->access_path))) This will give the wrong type when the function is called with an explicit scope; you probably want to look at argtype instead. Jason
Hi, On 21/05/19 16:57, Jason Merrill wrote: > On 5/16/19 7:12 PM, Paolo Carlini wrote: >> Hi, >> >> when Roberto Agostino and I implemented the front-end >> devirtualization of final overriders we missed this case, where it >> comes from the base. It seems to me that by way of access_path the >> existing approach can be neatly extended. Tested x86_64-linux. > >> + || CLASSTYPE_FINAL (TREE_TYPE (cand->access_path))) > > This will give the wrong type when the function is called with an > explicit scope; you probably want to look at argtype instead. Yes, thanks, that works fine and is even neater. I'm finishing testing the below. As you can see, I also added a line to final3.C where the two versions of the call..c conditional give different answers. Note, however, that in practice, in terms, say, of dumps, the difference is difficult to emphasize because the call would not be virtual anyway (if I'm not horribly mistaken ;) Thanks, Paolo. ///////////////// Index: cp/call.c =================================================================== --- cp/call.c (revision 271459) +++ cp/call.c (working copy) @@ -8244,7 +8244,7 @@ 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 (TYPE_METHOD_BASETYPE (TREE_TYPE (fn)))) + || CLASSTYPE_FINAL (TREE_TYPE (argtype))) flags |= LOOKUP_NONVIRTUAL; /* [class.mfct.nonstatic]: If a nonstatic member function of a class 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,27 @@ +// 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(); +} + +// { 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" } }
On 5/21/19 12:34 PM, Paolo Carlini wrote: > Hi, > > On 21/05/19 16:57, Jason Merrill wrote: >> On 5/16/19 7:12 PM, Paolo Carlini wrote: >>> Hi, >>> >>> when Roberto Agostino and I implemented the front-end >>> devirtualization of final overriders we missed this case, where it >>> comes from the base. It seems to me that by way of access_path the >>> existing approach can be neatly extended. Tested x86_64-linux. >> >>> + || CLASSTYPE_FINAL (TREE_TYPE (cand->access_path))) >> >> This will give the wrong type when the function is called with an >> explicit scope; you probably want to look at argtype instead. > > Yes, thanks, that works fine and is even neater. I'm finishing testing > the below. As you can see, I also added a line to final3.C where the two > versions of the call..c conditional give different answers. Note, > however, that in practice, in terms, say, of dumps, the difference is > difficult to emphasize because the call would not be virtual anyway (if > I'm not horribly mistaken ;) True enough. OK. Jason
Index: cp/call.c =================================================================== --- cp/call.c (revision 271296) +++ cp/call.c (working copy) @@ -8241,7 +8241,7 @@ 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 (TYPE_METHOD_BASETYPE (TREE_TYPE (fn)))) + || CLASSTYPE_FINAL (TREE_TYPE (cand->access_path))) flags |= LOOKUP_NONVIRTUAL; /* [class.mfct.nonstatic]: If a nonstatic member function of a class 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,26 @@ +// 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(); +} + +void call(oV& x) +{ + x.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" } }