On 3/23/23 17:03, Jakub Jelinek wrote:
On Thu, Mar 23, 2023 at 04:35:07PM -0400, Jason Merrill wrote:
Tested x86_64-pc-linux-gnu. Jakub, does this make sense to you? Do we have a
way of testing for compile-hog regressions?
-- 8< --
The patch for PR91415 fixed -Wsequence-point to treat shifts and ARRAY_REF
as sequenced in C++17, and COMPONENT_REF as well. But this is unnecessary
for COMPONENT_REF, since the RHS is just a FIELD_DECL with no actual
evaluation, and in this testcase handling COMPONENT_REF as sequenced blows
up fast in a deep inheritance tree.
PR c++/107163
gcc/c-family/ChangeLog:
* c-common.cc (verify_tree): Don't use sequenced handling
for COMPONENT_REF.
When we touch this for COMPONENT_REF, shouldn't we then handle it as
unary given that the second operand is FIELD_DECL and third/fourth
will likely be NULL and even if not, aren't user expressions that should be
inspected?
So, instead of doing this do:
case COMPONENT_REF:
x = TREE_OPERAND (x, 0);
writer = 0;
goto restart;
?
Is clearing 'writer' what we want, since an access to COMPONENT_REF is
an access to (a subobject of) its op0?
As for compile-hog, depends on how long it will take it to compile before
fix/after fix. If before fix can be above the normal timeout on reasonably
fast matchines and after fix can take a few seconds, great
Currently with the fix it takes <1s while gcc12 takes ~80s.
if after fix
would take longer but still not horribly long, one way to do it is
guard the test with run_expensive_tests effective target. Or another way
is have the test smaller in complexity normally and
// { dg-additional-options "-DEXPENSIVE" { target run_expensive_tests } }
and #ifdef EXPENSIVE make it more complex.
Curiously, making the recursion much deeper doesn't work for that; I
guess at some point the -Wsequence-point code decides the expression is
too complex and gives up?
But repeating the assignment brings it up over the timeout.
How about this?
From bb302f97929df9b854f7f929093441da60305254 Mon Sep 17 00:00:00 2001
From: Jason Merrill <ja...@redhat.com>
Date: Thu, 23 Mar 2023 15:57:39 -0400
Subject: [PATCH] c-family: -Wsequence-point and COMPONENT_REF [PR107163]
To: gcc-patches@gcc.gnu.org
The patch for PR91415 fixed -Wsequence-point to treat shifts and ARRAY_REF
as sequenced in C++17, and COMPONENT_REF as well. But this is unnecessary
for COMPONENT_REF, since the RHS is just a FIELD_DECL with no actual
evaluation, and in this testcase handling COMPONENT_REF as sequenced blows
up fast in a deep inheritance tree. Instead, look through it.
PR c++/107163
gcc/c-family/ChangeLog:
* c-common.cc (verify_tree): Don't use sequenced handling
for COMPONENT_REF.
gcc/testsuite/ChangeLog:
* g++.dg/template/recurse5.C: New test.
---
gcc/c-family/c-common.cc | 5 +++-
gcc/testsuite/g++.dg/template/recurse5.C | 37 ++++++++++++++++++++++++
2 files changed, 41 insertions(+), 1 deletion(-)
create mode 100644 gcc/testsuite/g++.dg/template/recurse5.C
diff --git a/gcc/c-family/c-common.cc b/gcc/c-family/c-common.cc
index bfb950e56db..07f7beac8fd 100644
--- a/gcc/c-family/c-common.cc
+++ b/gcc/c-family/c-common.cc
@@ -2154,12 +2154,15 @@ verify_tree (tree x, struct tlist **pbefore_sp, struct tlist **pno_sp,
case LSHIFT_EXPR:
case RSHIFT_EXPR:
- case COMPONENT_REF:
case ARRAY_REF:
if (cxx_dialect >= cxx17)
goto sequenced_binary;
goto do_default;
+ case COMPONENT_REF:
+ x = TREE_OPERAND (x, 0);
+ goto restart;
+
default:
do_default:
/* For other expressions, simply recurse on their operands.
diff --git a/gcc/testsuite/g++.dg/template/recurse5.C b/gcc/testsuite/g++.dg/template/recurse5.C
new file mode 100644
index 00000000000..0354ab09f53
--- /dev/null
+++ b/gcc/testsuite/g++.dg/template/recurse5.C
@@ -0,0 +1,37 @@
+// PR c++/107163
+// { dg-additional-options "-Wsequence-point" }
+
+struct BaseType {
+ int i;
+};
+
+template< int Seq >
+class DerivedType : public DerivedType< Seq - 1 > { };
+
+template<>
+class DerivedType< -1 > : public BaseType { };
+
+int main() {
+ DerivedType< 400 > d;
+ d.i = 42;
+ d.i = 42;
+ d.i = 42;
+ d.i = 42;
+ d.i = 42;
+ d.i = 42;
+ d.i = 42;
+ d.i = 42;
+ d.i = 42;
+ d.i = 42;
+ d.i = 42;
+ d.i = 42;
+ d.i = 42;
+ d.i = 42;
+ d.i = 42;
+ d.i = 42;
+ d.i = 42;
+ d.i = 42;
+ d.i = 42;
+ d.i = 42;
+ return d.i;
+}
--
2.31.1
