aboutsummaryrefslogtreecommitdiff
path: root/gcc
diff options
context:
space:
mode:
authorJakub Jelinek <jakub@redhat.com>2025-03-11 11:01:55 +0100
committerJakub Jelinek <jakub@gcc.gnu.org>2025-03-11 11:01:55 +0100
commit20e5aa9cc1519f871cce25dbfdc149d9d60da779 (patch)
tree5fb6c83635de44ce9ae069f3fa88ed03e70fe93c /gcc
parente1da6283a1cbd5db474c0f7e5cca9b9876768199 (diff)
downloadgcc-20e5aa9cc1519f871cce25dbfdc149d9d60da779.zip
gcc-20e5aa9cc1519f871cce25dbfdc149d9d60da779.tar.gz
gcc-20e5aa9cc1519f871cce25dbfdc149d9d60da779.tar.bz2
tree: Improve skip_simple_arithmetic [PR119183]
The following testcase takes very long time to compile, because skip_simple_arithmetic decides to first call tree_invariant_p on the second argument (and indirectly recurse there). I think before canonicalization of operands for commutative binary expressions (and for non-commutative ones always) it is pretty common that the first operand is a constant, something which tree_invariant_p handles immediately, so the following patch special cases that; I've added there a tree_invariant_p call too after the checks, while it is not really needed currently, tree_invariant_p has the same checks, I wanted to be prepared in case tree_invariant_p changes. But if you think I should avoid it, I can drop it too. This is just a partial fix, I think one can certainly construct a testcase which will still have horrible compile time complexity (but I've tried and haven't managed to do so), so perhaps we should just limit the recursion depth through skip_simple_arithmetic/tree_invariant_p with some defaulted argument. 2025-03-11 Jakub Jelinek <jakub@redhat.com> PR c/119183 * tree.cc (skip_simple_arithmetic): If first operand of binary expr is TREE_CONSTANT or TREE_READONLY with no side-effects, call tree_invariant_p on that operand first instead of on the second. * gcc.dg/pr119183.c: New test.
Diffstat (limited to 'gcc')
-rw-r--r--gcc/testsuite/gcc.dg/pr119183.c12
-rw-r--r--gcc/tree.cc14
2 files changed, 25 insertions, 1 deletions
diff --git a/gcc/testsuite/gcc.dg/pr119183.c b/gcc/testsuite/gcc.dg/pr119183.c
new file mode 100644
index 0000000..a98d779
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr119183.c
@@ -0,0 +1,12 @@
+/* PR c/119183 */
+/* { dg-do compile } */
+
+int foo (void);
+#define A(x) (1.0f * (1.0f * (1.0f * (1.0f * (1.0f * (1.0f * (1.0f * (1.0f * (x)))))))))
+
+float
+bar (float r)
+{
+ r += A (A (A (A (A (A (A (A (foo ()))))))));
+ return r;
+}
diff --git a/gcc/tree.cc b/gcc/tree.cc
index a35b4db..eccfcc8 100644
--- a/gcc/tree.cc
+++ b/gcc/tree.cc
@@ -4105,7 +4105,19 @@ skip_simple_arithmetic (tree expr)
expr = TREE_OPERAND (expr, 0);
else if (BINARY_CLASS_P (expr))
{
- if (tree_invariant_p (TREE_OPERAND (expr, 1)))
+ /* Before commutative binary operands are canonicalized,
+ it is quite common to have constants in the first operand.
+ Check for that common case first so that we don't walk
+ large expressions with tree_invariant_p unnecessarily.
+ This can still have terrible compile time complexity,
+ we should limit the depth of the tree_invariant_p and
+ skip_simple_arithmetic recursion. */
+ if ((TREE_CONSTANT (TREE_OPERAND (expr, 0))
+ || (TREE_READONLY (TREE_OPERAND (expr, 0))
+ && !TREE_SIDE_EFFECTS (TREE_OPERAND (expr, 0))))
+ && tree_invariant_p (TREE_OPERAND (expr, 0)))
+ expr = TREE_OPERAND (expr, 1);
+ else if (tree_invariant_p (TREE_OPERAND (expr, 1)))
expr = TREE_OPERAND (expr, 0);
else if (tree_invariant_p (TREE_OPERAND (expr, 0)))
expr = TREE_OPERAND (expr, 1);