public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* Devirtualization of objects in array
@ 2023-07-12  8:58 Ng YongXiang
  2023-07-12  9:02 ` Xi Ruoyao
  0 siblings, 1 reply; 8+ messages in thread
From: Ng YongXiang @ 2023-07-12  8:58 UTC (permalink / raw)
  To: gcc-patches

[-- Attachment #1: Type: text/plain, Size: 639 bytes --]

Hello,

I'm writing to seek for a review for an issue I filed some time ago.
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110057 . A proposed patch is
attached in the bug tracker as well.

The gist of the issue is that in an array, we know the underlying type of
the object. An array must hold the type it is declared with, and it will
never be objects of derived type. Hence, there is a "devirtualization"
opportunity here. GCC is currently invoking the virtual destructor of the
objects in the container when the array destructs which shouldn't be the
case since we know for sure the object's type.

Thank you.

Best regards,
Yong Xiang

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: Devirtualization of objects in array
  2023-07-12  8:58 Devirtualization of objects in array Ng YongXiang
@ 2023-07-12  9:02 ` Xi Ruoyao
  2023-07-12 14:10   ` [PATCH] - Devirtualization of array destruction (C++) - 110057 Ng YongXiang
  0 siblings, 1 reply; 8+ messages in thread
From: Xi Ruoyao @ 2023-07-12  9:02 UTC (permalink / raw)
  To: Ng YongXiang, gcc-patches

On Wed, 2023-07-12 at 16:58 +0800, Ng YongXiang via Gcc-patches wrote:
> I'm writing to seek for a review for an issue I filed some time ago.
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110057 . A proposed patch is
> attached in the bug tracker as well.

You should send the patch to gcc-patches@gcc.gnu.org for a review, see
https://gcc.gnu.org/contribute.html for the details.  Generally we
consider patches attached in bugzilla as drafts.

-- 
Xi Ruoyao <xry111@xry111.site>
School of Aerospace Science and Technology, Xidian University

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH] - Devirtualization of array destruction (C++) - 110057
  2023-07-12  9:02 ` Xi Ruoyao
@ 2023-07-12 14:10   ` Ng YongXiang
  2023-07-26  4:24     ` Jason Merrill
  0 siblings, 1 reply; 8+ messages in thread
From: Ng YongXiang @ 2023-07-12 14:10 UTC (permalink / raw)
  To: gcc-patches; +Cc: Xi Ruoyao


[-- Attachment #1.1: Type: text/plain, Size: 1255 bytes --]

Component:
c++

Bug ID:
110057

Bugzilla link:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110057

Description:
Array should not call virtual destructor of object when array is destructed

ChangeLog:

2023-07-12  Ng YongXiang  <yongxiangng@gmail.com>        PR c++
* Devirtualize auto generated destructor calls of arraycp/        *
init.c: Call non virtual destructor of objects in arraytestsuite/
  * g++.dg/devirt-array-destructor-1.C: New.        *
g++.dg/devirt-array-destructor-2.C: New.
        * g++.dg/warn/pr83054.C: Change expected number of devirtualized calls


On Wed, Jul 12, 2023 at 5:02 PM Xi Ruoyao <xry111@xry111.site> wrote:

> On Wed, 2023-07-12 at 16:58 +0800, Ng YongXiang via Gcc-patches wrote:
> > I'm writing to seek for a review for an issue I filed some time ago.
> > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110057 . A proposed patch
> is
> > attached in the bug tracker as well.
>
> You should send the patch to gcc-patches@gcc.gnu.org for a review, see
> https://gcc.gnu.org/contribute.html for the details.  Generally we
> consider patches attached in bugzilla as drafts.
>
> --
> Xi Ruoyao <xry111@xry111.site>
> School of Aerospace Science and Technology, Xidian University
>

[-- Attachment #2: 0001-Devirtualize-auto-generated-destructor-calls-of-arra.patch --]
[-- Type: application/x-patch, Size: 4290 bytes --]

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] - Devirtualization of array destruction (C++) - 110057
  2023-07-12 14:10   ` [PATCH] - Devirtualization of array destruction (C++) - 110057 Ng YongXiang
@ 2023-07-26  4:24     ` Jason Merrill
  2023-07-26 16:00       ` [PATCH] c++: devirtualization of array destruction [PR110057] Ng YongXiang
  0 siblings, 1 reply; 8+ messages in thread
From: Jason Merrill @ 2023-07-26  4:24 UTC (permalink / raw)
  To: Ng YongXiang, gcc-patches; +Cc: Xi Ruoyao

On 7/12/23 10:10, Ng YongXiang via Gcc-patches wrote:
> Component:
> c++
> 
> Bug ID:
> 110057
> 
> Bugzilla link:
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110057
> 
> Description:
> Array should not call virtual destructor of object when array is destructed
> 
> ChangeLog:
> 
> 2023-07-12  Ng YongXiang  <yongxiangng@gmail.com>        PR c++
> * Devirtualize auto generated destructor calls of arraycp/        *
> init.c: Call non virtual destructor of objects in arraytestsuite/
>    * g++.dg/devirt-array-destructor-1.C: New.        *
> g++.dg/devirt-array-destructor-2.C: New.
> 
> 
> On Wed, Jul 12, 2023 at 5:02 PM Xi Ruoyao <xry111@xry111.site> wrote:
> 
>> On Wed, 2023-07-12 at 16:58 +0800, Ng YongXiang via Gcc-patches wrote:
>>> I'm writing to seek for a review for an issue I filed some time ago.
>>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110057 . A proposed patch
>> is
>>> attached in the bug tracker as well.
>>
>> You should send the patch to gcc-patches@gcc.gnu.org for a review, see
>> https://gcc.gnu.org/contribute.html for the details.  Generally we
>> consider patches attached in bugzilla as drafts.

Thanks!  The change makes sense under 
https://eel.is/c++draft/expr.delete#3.sentence-2 , but please look again 
at contribute.html.

In particular, the Legal section; you don't seem to have a copyright 
assignment with the FSF, nor do I see a DCO certification 
(https://gcc.gnu.org/dco.html) in your patch.

Like the examples in contribute.html, the subject line should be more 
like "[PATCH] c++: devirtualization of array destruction [PR110057]"

The ChangeLog entry should be in the commit message.

>          * g++.dg/warn/pr83054.C: Change expected number of devirtualized calls

This isn't just changing the expected number, it's also changing the 
array from a local variable to dynamically allocated, which is a big 
change to what's being tested.  If you want to test the dynamic case, 
please add a new test instead of making this change.

> diff --git a/gcc/testsuite/g++.dg/warn/pr83054.C b/gcc/testsuite/g++.dg/warn/pr83054.C
> index 5285f94acee..7cd0951713d 100644
> --- a/gcc/testsuite/g++.dg/warn/pr83054.C
> +++ b/gcc/testsuite/g++.dg/warn/pr83054.C
> @@ -10,7 +10,7 @@
>  #endif
>  
>  extern "C" int printf (const char *, ...);
> -struct foo // { dg-warning "final would enable devirtualization of 5 calls" }
> +struct foo // { dg-warning "final would enable devirtualization of 1 call" }
>  {
>    static int count;
>    void print (int i, int j) { printf ("foo[%d][%d] = %d\n", i, j, x); }
> @@ -29,19 +29,15 @@ int foo::count;
>  
>  int main ()
>  {
> -  {
> -    foo array[3][3];
> -    for (int i = 0; i < 3; i++)
> -      {
> -       for (int j = 0; j < 3; j++)
> -         {
> -           printf("&a[%d][%d] = %x\n", i, j, (void *)&array[i][j]);
> -         }
> -      }
> -      // The count should be nine, if not, fail the test.
> -      if (foo::count != 9)
> -       return 1;
> -  }
> +  foo* arr[9];
> +  for (int i = 0; i < 9; ++i)
> +    arr[i] = new foo();
> +  if (foo::count != 9)
> +    return 1;
> +  for (int i = 0; i < 9; ++i)
> +    arr[i]->print(i / 3, i % 3);
> +  for (int i = 0; i < 9; ++i)
> +    delete arr[i];



^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH] c++: devirtualization of array destruction [PR110057]
  2023-07-26  4:24     ` Jason Merrill
@ 2023-07-26 16:00       ` Ng YongXiang
  2023-07-26 16:30         ` Jason Merrill
  0 siblings, 1 reply; 8+ messages in thread
From: Ng YongXiang @ 2023-07-26 16:00 UTC (permalink / raw)
  To: Jason Merrill, gcc-patches; +Cc: Xi Ruoyao


[-- Attachment #1.1: Type: text/plain, Size: 4037 bytes --]

Hi Jason,

Thanks for the reply and review. I've attached an updated patch with the
change log and sign off.

The change made in gcc/testsuite/g++.dg/warn/pr83054.C is because I think
there is no more warning since we have already devirtualized the
destruction for the array.

Apologies for the poor formatting. It is my first time contributing. Do let
me know if there's any stuff I've missed and feel free to modify the patch
where you deem necessary.

Thanks!

On Wed, Jul 26, 2023 at 12:25 PM Jason Merrill <jason@redhat.com> wrote:

> On 7/12/23 10:10, Ng YongXiang via Gcc-patches wrote:
> > Component:
> > c++
> >
> > Bug ID:
> > 110057
> >
> > Bugzilla link:
> > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110057
> >
> > Description:
> > Array should not call virtual destructor of object when array is
> destructed
> >
> > ChangeLog:
> >
> > 2023-07-12  Ng YongXiang  <yongxiangng@gmail.com>        PR c++
> > * Devirtualize auto generated destructor calls of arraycp/        *
> > init.c: Call non virtual destructor of objects in arraytestsuite/
> >    * g++.dg/devirt-array-destructor-1.C: New.        *
> > g++.dg/devirt-array-destructor-2.C: New.
> >
> >
> > On Wed, Jul 12, 2023 at 5:02 PM Xi Ruoyao <xry111@xry111.site> wrote:
> >
> >> On Wed, 2023-07-12 at 16:58 +0800, Ng YongXiang via Gcc-patches wrote:
> >>> I'm writing to seek for a review for an issue I filed some time ago.
> >>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110057 . A proposed patch
> >> is
> >>> attached in the bug tracker as well.
> >>
> >> You should send the patch to gcc-patches@gcc.gnu.org for a review, see
> >> https://gcc.gnu.org/contribute.html for the details.  Generally we
> >> consider patches attached in bugzilla as drafts.
>
> Thanks!  The change makes sense under
> https://eel.is/c++draft/expr.delete#3.sentence-2 , but please look again
> at contribute.html.
>
> In particular, the Legal section; you don't seem to have a copyright
> assignment with the FSF, nor do I see a DCO certification
> (https://gcc.gnu.org/dco.html) in your patch.
>
> Like the examples in contribute.html, the subject line should be more
> like "[PATCH] c++: devirtualization of array destruction [PR110057]"
>
> The ChangeLog entry should be in the commit message.
>
> >          * g++.dg/warn/pr83054.C: Change expected number of
> devirtualized calls
>
> This isn't just changing the expected number, it's also changing the
> array from a local variable to dynamically allocated, which is a big
> change to what's being tested.  If you want to test the dynamic case,
> please add a new test instead of making this change.
>
> > diff --git a/gcc/testsuite/g++.dg/warn/pr83054.C
> b/gcc/testsuite/g++.dg/warn/pr83054.C
> > index 5285f94acee..7cd0951713d 100644
> > --- a/gcc/testsuite/g++.dg/warn/pr83054.C
> > +++ b/gcc/testsuite/g++.dg/warn/pr83054.C
> > @@ -10,7 +10,7 @@
> >  #endif
> >
> >  extern "C" int printf (const char *, ...);
> > -struct foo // { dg-warning "final would enable devirtualization of 5
> calls" }
> > +struct foo // { dg-warning "final would enable devirtualization of 1
> call" }
> >  {
> >    static int count;
> >    void print (int i, int j) { printf ("foo[%d][%d] = %d\n", i, j, x); }
> > @@ -29,19 +29,15 @@ int foo::count;
> >
> >  int main ()
> >  {
> > -  {
> > -    foo array[3][3];
> > -    for (int i = 0; i < 3; i++)
> > -      {
> > -       for (int j = 0; j < 3; j++)
> > -         {
> > -           printf("&a[%d][%d] = %x\n", i, j, (void *)&array[i][j]);
> > -         }
> > -      }
> > -      // The count should be nine, if not, fail the test.
> > -      if (foo::count != 9)
> > -       return 1;
> > -  }
> > +  foo* arr[9];
> > +  for (int i = 0; i < 9; ++i)
> > +    arr[i] = new foo();
> > +  if (foo::count != 9)
> > +    return 1;
> > +  for (int i = 0; i < 9; ++i)
> > +    arr[i]->print(i / 3, i % 3);
> > +  for (int i = 0; i < 9; ++i)
> > +    delete arr[i];
>
>
>

[-- Attachment #2: 0001-PATCH-c-devirtualization-of-array-destruction-PR1100.patch --]
[-- Type: application/x-patch, Size: 3947 bytes --]

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] c++: devirtualization of array destruction [PR110057]
  2023-07-26 16:00       ` [PATCH] c++: devirtualization of array destruction [PR110057] Ng YongXiang
@ 2023-07-26 16:30         ` Jason Merrill
  2023-07-27  0:06           ` Ng YongXiang
  0 siblings, 1 reply; 8+ messages in thread
From: Jason Merrill @ 2023-07-26 16:30 UTC (permalink / raw)
  To: Ng YongXiang, gcc-patches; +Cc: Xi Ruoyao

On 7/26/23 12:00, Ng YongXiang wrote:
> Hi Jason,
> 
> Thanks for the reply and review. I've attached an updated patch with the 
> change log and sign off.
> 
> The change made in gcc/testsuite/g++.dg/warn/pr83054.C is because I 
> think there is no more warning since we have already devirtualized the 
> destruction for the array.

Makes sense, and it's good to have your adjusted testcase in the 
testsuite, it should just be a new one (maybe pr83054-2.C).

> Apologies for the poor formatting. It is my first time contributing. Do 
> let me know if there's any stuff I've missed and feel free to modify the 
> patch where you deem necessary.

No worries!

The ChangeLog entries still need some adjustment, according to git 
gcc-verify (from contrib/gcc-git-customization.sh, see 
https://gcc.gnu.org/gitwrite.html):

ERR: line should start with a tab: "        * init.c: Call non virtual 
destructor of objects in array"
ERR: line should start with a tab: "        * 
g++.dg/devirt-array-destructor-1.C: New."
ERR: line should start with a tab: "        * 
g++.dg/devirt-array-destructor-2.C: New."
ERR: line should start with a tab: "        * g++.dg/warn/pr83054.C: 
Remove expected warnings caused by devirtualization"
ERR: PR 110057 in subject but not in changelog: "c++: devirtualization 
of array destruction [PR110057]"

git gcc-commit-mklog (also from gcc-git-customization.sh) makes 
generating ChangeLog entries a lot simpler.

>             * g++.dg/devirt-array-destructor-1.C: New.

Tests that look at tree-optimization dump files should go in the 
g++.dg/tree-ssa subdirectory.

> +/* { dg-do run } */

It seems unnecessary to execute these tests, I'd think the default of { 
dg-do compile } would be fine.

It's also good to have a

// PR c++/110057

line at the top of the testcase for future reference.  gcc-commit-mklog 
also uses that to add the PR number to the ChangeLog.

Jason


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] c++: devirtualization of array destruction [PR110057]
  2023-07-26 16:30         ` Jason Merrill
@ 2023-07-27  0:06           ` Ng YongXiang
  2023-07-28 15:42             ` Jason Merrill
  0 siblings, 1 reply; 8+ messages in thread
From: Ng YongXiang @ 2023-07-27  0:06 UTC (permalink / raw)
  To: Jason Merrill; +Cc: gcc-patches, Xi Ruoyao


[-- Attachment #1.1: Type: text/plain, Size: 2402 bytes --]

Hi Jason,

I've made the following changes.

1. Add pr83054-2.C
2. Move the devirt tests to tree-ssa.
3. Remove dg do run for devirt tests
4. Add // PR c++/110057
5. Generate commit message with git gcc-commit-mklog
6. Check commit format with git gcc-verify

Thanks!

On Thu, Jul 27, 2023 at 12:30 AM Jason Merrill <jason@redhat.com> wrote:

> On 7/26/23 12:00, Ng YongXiang wrote:
> > Hi Jason,
> >
> > Thanks for the reply and review. I've attached an updated patch with the
> > change log and sign off.
> >
> > The change made in gcc/testsuite/g++.dg/warn/pr83054.C is because I
> > think there is no more warning since we have already devirtualized the
> > destruction for the array.
>
> Makes sense, and it's good to have your adjusted testcase in the
> testsuite, it should just be a new one (maybe pr83054-2.C).
>
> > Apologies for the poor formatting. It is my first time contributing. Do
> > let me know if there's any stuff I've missed and feel free to modify the
> > patch where you deem necessary.
>
> No worries!
>
> The ChangeLog entries still need some adjustment, according to git
> gcc-verify (from contrib/gcc-git-customization.sh, see
> https://gcc.gnu.org/gitwrite.html):
>
> ERR: line should start with a tab: "        * init.c: Call non virtual
> destructor of objects in array"
> ERR: line should start with a tab: "        *
> g++.dg/devirt-array-destructor-1.C: New."
> ERR: line should start with a tab: "        *
> g++.dg/devirt-array-destructor-2.C: New."
> ERR: line should start with a tab: "        * g++.dg/warn/pr83054.C:
> Remove expected warnings caused by devirtualization"
> ERR: PR 110057 in subject but not in changelog: "c++: devirtualization
> of array destruction [PR110057]"
>
> git gcc-commit-mklog (also from gcc-git-customization.sh) makes
> generating ChangeLog entries a lot simpler.
>
> >             * g++.dg/devirt-array-destructor-1.C: New.
>
> Tests that look at tree-optimization dump files should go in the
> g++.dg/tree-ssa subdirectory.
>
> > +/* { dg-do run } */
>
> It seems unnecessary to execute these tests, I'd think the default of {
> dg-do compile } would be fine.
>
> It's also good to have a
>
> // PR c++/110057
>
> line at the top of the testcase for future reference.  gcc-commit-mklog
> also uses that to add the PR number to the ChangeLog.
>
> Jason
>
>

[-- Attachment #2: 0001-PATCH-c-devirtualization-of-array-destruction-PR1100.patch --]
[-- Type: text/x-patch, Size: 5429 bytes --]

From 04e9b412e49a3966f84edd3afda66ebdb729efdc Mon Sep 17 00:00:00 2001
From: yongxiangng <yongxiangng@gmail.com>
Date: Thu, 27 Jul 2023 08:01:42 +0800
Subject: [PATCH 1/1] [PATCH] c++: devirtualization of array destruction
 [PR110057]

	PR c++/110057
	PR ipa/83054

gcc/cp/ChangeLog:

	* init.cc (build_vec_delete_1): Devirtualize array destruction.

gcc/testsuite/ChangeLog:

	* g++.dg/warn/pr83054.C: Remove devirtualization warning.
	* g++.dg/tree-ssa/devirt-array-destructor-1.C: New test.
	* g++.dg/tree-ssa/devirt-array-destructor-2.C: New test.
	* g++.dg/warn/pr83054-2.C: New test.

Signed-off-by: Ng Yong Xiang <yongxiangng@gmail.com>
---
 gcc/cp/init.cc                                |  8 ++--
 .../tree-ssa/devirt-array-destructor-1.C      | 28 ++++++++++++
 .../tree-ssa/devirt-array-destructor-2.C      | 29 ++++++++++++
 gcc/testsuite/g++.dg/warn/pr83054-2.C         | 44 +++++++++++++++++++
 gcc/testsuite/g++.dg/warn/pr83054.C           |  2 +-
 5 files changed, 106 insertions(+), 5 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/tree-ssa/devirt-array-destructor-1.C
 create mode 100644 gcc/testsuite/g++.dg/tree-ssa/devirt-array-destructor-2.C
 create mode 100644 gcc/testsuite/g++.dg/warn/pr83054-2.C

diff --git a/gcc/cp/init.cc b/gcc/cp/init.cc
index 6ccda365b04..69ab51d0a4b 100644
--- a/gcc/cp/init.cc
+++ b/gcc/cp/init.cc
@@ -4112,8 +4112,8 @@ build_vec_delete_1 (location_t loc, tree base, tree maxindex, tree type,
       if (type_build_dtor_call (type))
 	{
 	  tmp = build_delete (loc, ptype, base, sfk_complete_destructor,
-			      LOOKUP_NORMAL|LOOKUP_DESTRUCTOR, 1,
-			      complain);
+			      LOOKUP_NORMAL|LOOKUP_DESTRUCTOR|LOOKUP_NONVIRTUAL,
+			      1, complain);
 	  if (tmp == error_mark_node)
 	    return error_mark_node;
 	}
@@ -4143,8 +4143,8 @@ build_vec_delete_1 (location_t loc, tree base, tree maxindex, tree type,
     return error_mark_node;
   body = build_compound_expr (loc, body, tmp);
   tmp = build_delete (loc, ptype, tbase, sfk_complete_destructor,
-		      LOOKUP_NORMAL|LOOKUP_DESTRUCTOR, 1,
-		      complain);
+		      LOOKUP_NORMAL|LOOKUP_DESTRUCTOR|LOOKUP_NONVIRTUAL,
+		      1, complain);
   if (tmp == error_mark_node)
     return error_mark_node;
   body = build_compound_expr (loc, body, tmp);
diff --git a/gcc/testsuite/g++.dg/tree-ssa/devirt-array-destructor-1.C b/gcc/testsuite/g++.dg/tree-ssa/devirt-array-destructor-1.C
new file mode 100644
index 00000000000..ce8dc2a57cd
--- /dev/null
+++ b/gcc/testsuite/g++.dg/tree-ssa/devirt-array-destructor-1.C
@@ -0,0 +1,28 @@
+// PR c++/110057
+/* { dg-do-compile } */
+/* Virtual calls should be devirtualized because we know dynamic type of object in array at compile time */
+/* { dg-options "-O3 -fdump-tree-optimized -fno-inline"  } */
+
+class A
+{
+public:
+  virtual ~A()
+  {
+  }
+};
+
+class B : public A
+{
+public:
+  virtual ~B()
+  {
+  }
+};
+
+int main()
+{
+  B b[10];
+  return 0;
+}
+
+/* { dg-final { scan-tree-dump-times "OBJ_TYPE_REF" 0 "optimized"} } */
diff --git a/gcc/testsuite/g++.dg/tree-ssa/devirt-array-destructor-2.C b/gcc/testsuite/g++.dg/tree-ssa/devirt-array-destructor-2.C
new file mode 100644
index 00000000000..6b44dc1a4ee
--- /dev/null
+++ b/gcc/testsuite/g++.dg/tree-ssa/devirt-array-destructor-2.C
@@ -0,0 +1,29 @@
+// PR c++/110057
+/* { dg-do-compile } */
+/* Virtual calls should be devirtualized because we know dynamic type of object in array at compile time */
+/* { dg-options "-O3 -fdump-tree-optimized -fno-inline"  } */
+
+class A
+{
+public:
+  virtual ~A()
+  {
+  }
+};
+
+class B : public A
+{
+public:
+  virtual ~B()
+  {
+  }
+};
+
+int main()
+{
+  B* ptr = new B[10];
+  delete[] ptr;
+  return 0;
+}
+
+/* { dg-final { scan-tree-dump-times "OBJ_TYPE_REF" 0 "optimized"} } */
diff --git a/gcc/testsuite/g++.dg/warn/pr83054-2.C b/gcc/testsuite/g++.dg/warn/pr83054-2.C
new file mode 100644
index 00000000000..6d8216d229f
--- /dev/null
+++ b/gcc/testsuite/g++.dg/warn/pr83054-2.C
@@ -0,0 +1,44 @@
+// PR ipa/83054
+// { dg-options "-O3 -Wsuggest-final-types" }
+// { dg-do compile }
+
+// A throwing dtor in C++98 mode changes the warning.
+#if __cplusplus < 201100L
+#define NOTHROW throw()
+#else
+#define NOTHROW noexcept
+#endif
+
+extern "C" int printf (const char *, ...);
+struct foo // { dg-warning "final would enable devirtualization of 1 call" }
+{
+  static int count;
+  void print (int i, int j) { printf ("foo[%d][%d] = %d\n", i, j, x); }
+  int x;
+  foo () {
+    x = count++;
+    printf("this %d = %x\n", x, (void *)this);
+  }
+  virtual ~foo () NOTHROW {
+    printf("this %d = %x\n", x, (void *)this);
+    --count;
+  }
+};
+int foo::count;
+
+
+int main ()
+{
+  foo *arr[9];
+  for (int i = 0; i < 9; ++i)
+    arr[i] = new foo();
+  if (foo::count != 9)
+    return 1;
+  for (int i = 0; i < 9; ++i)
+    arr[i]->print(i / 3, i % 3);
+  for (int i = 0; i < 9; ++i)
+    delete arr[i];
+  if (foo::count != 0)
+    return 1;
+  return 0;
+}
diff --git a/gcc/testsuite/g++.dg/warn/pr83054.C b/gcc/testsuite/g++.dg/warn/pr83054.C
index 5285f94acee..5a4a6abe248 100644
--- a/gcc/testsuite/g++.dg/warn/pr83054.C
+++ b/gcc/testsuite/g++.dg/warn/pr83054.C
@@ -10,7 +10,7 @@
 #endif
 
 extern "C" int printf (const char *, ...);
-struct foo // { dg-warning "final would enable devirtualization of 5 calls" }
+struct foo
 {
   static int count;
   void print (int i, int j) { printf ("foo[%d][%d] = %d\n", i, j, x); }
-- 
2.34.1


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] c++: devirtualization of array destruction [PR110057]
  2023-07-27  0:06           ` Ng YongXiang
@ 2023-07-28 15:42             ` Jason Merrill
  0 siblings, 0 replies; 8+ messages in thread
From: Jason Merrill @ 2023-07-28 15:42 UTC (permalink / raw)
  To: Ng YongXiang; +Cc: gcc-patches, Xi Ruoyao

[-- Attachment #1: Type: text/plain, Size: 444 bytes --]

On 7/26/23 20:06, Ng YongXiang wrote:
> Hi Jason,
> 
> I've made the following changes.
> 
> 1. Add pr83054-2.C
> 2. Move the devirt tests to tree-ssa.
> 3. Remove dg do run for devirt tests
> 4. Add // PR c++/110057
> 5. Generate commit message with git gcc-commit-mklog
> 6. Check commit format with git gcc-verify
> 
> Thanks!

Thanks.  I added a comment and fixed another test that was breaking with 
the patch; here's what I pushed.

Jason

[-- Attachment #2: 0001-c-devirtualization-of-array-destruction-PR110057.patch --]
[-- Type: text/x-patch, Size: 6186 bytes --]

From a47e615fbf9c6f4b24e5032df5d720b6bf9b63b5 Mon Sep 17 00:00:00 2001
From: Ng YongXiang <yongxiangng@gmail.com>
Date: Thu, 27 Jul 2023 08:06:14 +0800
Subject: [PATCH] c++: devirtualization of array destruction [PR110057]
To: gcc-patches@gcc.gnu.org

	PR c++/110057
	PR ipa/83054

gcc/cp/ChangeLog:

	* init.cc (build_vec_delete_1): Devirtualize array destruction.

gcc/testsuite/ChangeLog:

	* g++.dg/warn/pr83054.C: Remove devirtualization warning.
	* g++.dg/lto/pr89335_0.C: Likewise.
	* g++.dg/tree-ssa/devirt-array-destructor-1.C: New test.
	* g++.dg/tree-ssa/devirt-array-destructor-2.C: New test.
	* g++.dg/warn/pr83054-2.C: New test.

Signed-off-by: Ng Yong Xiang <yongxiangng@gmail.com>
---
 gcc/cp/init.cc                                | 11 +++--
 gcc/testsuite/g++.dg/lto/pr89335_0.C          |  2 +-
 .../tree-ssa/devirt-array-destructor-1.C      | 28 ++++++++++++
 .../tree-ssa/devirt-array-destructor-2.C      | 29 ++++++++++++
 gcc/testsuite/g++.dg/warn/pr83054-2.C         | 44 +++++++++++++++++++
 gcc/testsuite/g++.dg/warn/pr83054.C           |  2 +-
 6 files changed, 110 insertions(+), 6 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/tree-ssa/devirt-array-destructor-1.C
 create mode 100644 gcc/testsuite/g++.dg/tree-ssa/devirt-array-destructor-2.C
 create mode 100644 gcc/testsuite/g++.dg/warn/pr83054-2.C

diff --git a/gcc/cp/init.cc b/gcc/cp/init.cc
index ff5014ca576..3b9a7783391 100644
--- a/gcc/cp/init.cc
+++ b/gcc/cp/init.cc
@@ -4116,8 +4116,8 @@ build_vec_delete_1 (location_t loc, tree base, tree maxindex, tree type,
       if (type_build_dtor_call (type))
 	{
 	  tmp = build_delete (loc, ptype, base, sfk_complete_destructor,
-			      LOOKUP_NORMAL|LOOKUP_DESTRUCTOR, 1,
-			      complain);
+			      LOOKUP_NORMAL|LOOKUP_DESTRUCTOR|LOOKUP_NONVIRTUAL,
+			      1, complain);
 	  if (tmp == error_mark_node)
 	    return error_mark_node;
 	}
@@ -4146,9 +4146,12 @@ build_vec_delete_1 (location_t loc, tree base, tree maxindex, tree type,
   if (tmp == error_mark_node)
     return error_mark_node;
   body = build_compound_expr (loc, body, tmp);
+  /* [expr.delete]/3: "In an array delete expression, if the dynamic type of
+     the object to be deleted is not similar to its static type, the behavior
+     is undefined."  So we can set LOOKUP_NONVIRTUAL.  */
   tmp = build_delete (loc, ptype, tbase, sfk_complete_destructor,
-		      LOOKUP_NORMAL|LOOKUP_DESTRUCTOR, 1,
-		      complain);
+		      LOOKUP_NORMAL|LOOKUP_DESTRUCTOR|LOOKUP_NONVIRTUAL,
+		      1, complain);
   if (tmp == error_mark_node)
     return error_mark_node;
   body = build_compound_expr (loc, body, tmp);
diff --git a/gcc/testsuite/g++.dg/lto/pr89335_0.C b/gcc/testsuite/g++.dg/lto/pr89335_0.C
index 95bf4b3b0cb..76382f8d742 100644
--- a/gcc/testsuite/g++.dg/lto/pr89335_0.C
+++ b/gcc/testsuite/g++.dg/lto/pr89335_0.C
@@ -9,7 +9,7 @@ public:
   virtual ~Container ();
 };
 
-class List : public Container // { dg-lto-message "final would enable devirtualization" }
+class List : public Container
 {
 };
 
diff --git a/gcc/testsuite/g++.dg/tree-ssa/devirt-array-destructor-1.C b/gcc/testsuite/g++.dg/tree-ssa/devirt-array-destructor-1.C
new file mode 100644
index 00000000000..ce8dc2a57cd
--- /dev/null
+++ b/gcc/testsuite/g++.dg/tree-ssa/devirt-array-destructor-1.C
@@ -0,0 +1,28 @@
+// PR c++/110057
+/* { dg-do-compile } */
+/* Virtual calls should be devirtualized because we know dynamic type of object in array at compile time */
+/* { dg-options "-O3 -fdump-tree-optimized -fno-inline"  } */
+
+class A
+{
+public:
+  virtual ~A()
+  {
+  }
+};
+
+class B : public A
+{
+public:
+  virtual ~B()
+  {
+  }
+};
+
+int main()
+{
+  B b[10];
+  return 0;
+}
+
+/* { dg-final { scan-tree-dump-times "OBJ_TYPE_REF" 0 "optimized"} } */
diff --git a/gcc/testsuite/g++.dg/tree-ssa/devirt-array-destructor-2.C b/gcc/testsuite/g++.dg/tree-ssa/devirt-array-destructor-2.C
new file mode 100644
index 00000000000..6b44dc1a4ee
--- /dev/null
+++ b/gcc/testsuite/g++.dg/tree-ssa/devirt-array-destructor-2.C
@@ -0,0 +1,29 @@
+// PR c++/110057
+/* { dg-do-compile } */
+/* Virtual calls should be devirtualized because we know dynamic type of object in array at compile time */
+/* { dg-options "-O3 -fdump-tree-optimized -fno-inline"  } */
+
+class A
+{
+public:
+  virtual ~A()
+  {
+  }
+};
+
+class B : public A
+{
+public:
+  virtual ~B()
+  {
+  }
+};
+
+int main()
+{
+  B* ptr = new B[10];
+  delete[] ptr;
+  return 0;
+}
+
+/* { dg-final { scan-tree-dump-times "OBJ_TYPE_REF" 0 "optimized"} } */
diff --git a/gcc/testsuite/g++.dg/warn/pr83054-2.C b/gcc/testsuite/g++.dg/warn/pr83054-2.C
new file mode 100644
index 00000000000..6d8216d229f
--- /dev/null
+++ b/gcc/testsuite/g++.dg/warn/pr83054-2.C
@@ -0,0 +1,44 @@
+// PR ipa/83054
+// { dg-options "-O3 -Wsuggest-final-types" }
+// { dg-do compile }
+
+// A throwing dtor in C++98 mode changes the warning.
+#if __cplusplus < 201100L
+#define NOTHROW throw()
+#else
+#define NOTHROW noexcept
+#endif
+
+extern "C" int printf (const char *, ...);
+struct foo // { dg-warning "final would enable devirtualization of 1 call" }
+{
+  static int count;
+  void print (int i, int j) { printf ("foo[%d][%d] = %d\n", i, j, x); }
+  int x;
+  foo () {
+    x = count++;
+    printf("this %d = %x\n", x, (void *)this);
+  }
+  virtual ~foo () NOTHROW {
+    printf("this %d = %x\n", x, (void *)this);
+    --count;
+  }
+};
+int foo::count;
+
+
+int main ()
+{
+  foo *arr[9];
+  for (int i = 0; i < 9; ++i)
+    arr[i] = new foo();
+  if (foo::count != 9)
+    return 1;
+  for (int i = 0; i < 9; ++i)
+    arr[i]->print(i / 3, i % 3);
+  for (int i = 0; i < 9; ++i)
+    delete arr[i];
+  if (foo::count != 0)
+    return 1;
+  return 0;
+}
diff --git a/gcc/testsuite/g++.dg/warn/pr83054.C b/gcc/testsuite/g++.dg/warn/pr83054.C
index 5285f94acee..5a4a6abe248 100644
--- a/gcc/testsuite/g++.dg/warn/pr83054.C
+++ b/gcc/testsuite/g++.dg/warn/pr83054.C
@@ -10,7 +10,7 @@
 #endif
 
 extern "C" int printf (const char *, ...);
-struct foo // { dg-warning "final would enable devirtualization of 5 calls" }
+struct foo
 {
   static int count;
   void print (int i, int j) { printf ("foo[%d][%d] = %d\n", i, j, x); }
-- 
2.39.3


^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2023-07-28 15:42 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-07-12  8:58 Devirtualization of objects in array Ng YongXiang
2023-07-12  9:02 ` Xi Ruoyao
2023-07-12 14:10   ` [PATCH] - Devirtualization of array destruction (C++) - 110057 Ng YongXiang
2023-07-26  4:24     ` Jason Merrill
2023-07-26 16:00       ` [PATCH] c++: devirtualization of array destruction [PR110057] Ng YongXiang
2023-07-26 16:30         ` Jason Merrill
2023-07-27  0:06           ` Ng YongXiang
2023-07-28 15:42             ` 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).