Re: [PATCH] c++: devirtualization of array destruction [PR110057]
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.
JasonFrom a47e615fbf9c6f4b24e5032df5d720b6bf9b63b5 Mon Sep 17 00:00:00 2001
From: Ng YongXiang
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
---
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 000..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 000..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
Re: [PATCH] c++: devirtualization of array destruction [PR110057]
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 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
>
>
From 04e9b412e49a3966f84edd3afda66ebdb729efdc Mon Sep 17 00:00:00 2001
From: yongxiangng
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
---
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 000..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
Re: [PATCH] c++: devirtualization of array destruction [PR110057]
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
