diff options
author | Jakub Jelinek <jakub@redhat.com> | 2025-03-11 11:01:55 +0100 |
---|---|---|
committer | Jakub Jelinek <jakub@gcc.gnu.org> | 2025-03-11 11:01:55 +0100 |
commit | 20e5aa9cc1519f871cce25dbfdc149d9d60da779 (patch) | |
tree | 5fb6c83635de44ce9ae069f3fa88ed03e70fe93c | |
parent | e1da6283a1cbd5db474c0f7e5cca9b9876768199 (diff) | |
download | gcc-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.
-rw-r--r-- | gcc/testsuite/gcc.dg/pr119183.c | 12 | ||||
-rw-r--r-- | gcc/tree.cc | 14 |
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); |