[C++] PR 67184 ("Missed optimization with C++11 final specifier")

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.

Thanks, Paolo.

///////////////////////
/cp
2019-05-17  Paolo Carlini  <paolo.carlini@oracle.com>

	PR c++/67184
	PR c++/69445
	* call.c (build_over_call): Devirtualize when the final overrider
	comes from the base.

/testsuite
2019-05-17  Paolo Carlini  <paolo.carlini@oracle.com>

	PR c++/67184
	PR c++/69445
	* g++.dg/other/final3.C: New.
	* g++.dg/other/final4.C: Likewise.
	* g++.dg/other/final5.C: Likewise.

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