Re: [PATCH] Fix stmt folding in the inliner (PR tree-optimization/88444)
On Thu, 13 Dec 2018, Jeff Law wrote: > On 12/13/18 3:59 PM, Jakub Jelinek wrote: > > Hi! > > > > The inliner doesn't want to fold statements immediately, but records them > > and then fold_marked_statements is meant to fold them when inlining is done. > > > > On the following testcase it doesn't fold some of them though. > > The problem is that it wants to scan only newly added basic blocks (i.e. > > those created during the inlining), but the way it is written only works if > > there are no gaps in the basic_block vector. If there are, it can fold > > stmts only in some of the basic blocks or none of them. > > > > Fixed thusly, bootstrapped/regtested on x86_64-linux and i686-linux, ok for > > trunk? > > > > 2018-12-13 Jakub Jelinek > > > > PR tree-optimization/88444 > > * tree-inline.c (fold_marked_statements): Iterate up to > > last_basic_block_for_fn rather than n_basic_blocks_for_fn. > > > > * gcc.dg/tree-ssa/pr88444.c: New test. > OK It also looks like it is written in the way to scan BBs because in older times gsi_for_stmt was O(n). Today we might be able to simply walk the hash-set and fold contained stmts. Iff we continue walking stmts we could consider to allow fold_stmt to walk single-use edges. Anyway, just sth to consider for GCC 10. Thanks for catching this subtle bug ;) Richard.
Re: [PATCH] Fix stmt folding in the inliner (PR tree-optimization/88444)
On 12/13/18 3:59 PM, Jakub Jelinek wrote: > Hi! > > The inliner doesn't want to fold statements immediately, but records them > and then fold_marked_statements is meant to fold them when inlining is done. > > On the following testcase it doesn't fold some of them though. > The problem is that it wants to scan only newly added basic blocks (i.e. > those created during the inlining), but the way it is written only works if > there are no gaps in the basic_block vector. If there are, it can fold > stmts only in some of the basic blocks or none of them. > > Fixed thusly, bootstrapped/regtested on x86_64-linux and i686-linux, ok for > trunk? > > 2018-12-13 Jakub Jelinek > > PR tree-optimization/88444 > * tree-inline.c (fold_marked_statements): Iterate up to > last_basic_block_for_fn rather than n_basic_blocks_for_fn. > > * gcc.dg/tree-ssa/pr88444.c: New test. OK jeff
[PATCH] Fix stmt folding in the inliner (PR tree-optimization/88444)
Hi!
The inliner doesn't want to fold statements immediately, but records them
and then fold_marked_statements is meant to fold them when inlining is done.
On the following testcase it doesn't fold some of them though.
The problem is that it wants to scan only newly added basic blocks (i.e.
those created during the inlining), but the way it is written only works if
there are no gaps in the basic_block vector. If there are, it can fold
stmts only in some of the basic blocks or none of them.
Fixed thusly, bootstrapped/regtested on x86_64-linux and i686-linux, ok for
trunk?
2018-12-13 Jakub Jelinek
PR tree-optimization/88444
* tree-inline.c (fold_marked_statements): Iterate up to
last_basic_block_for_fn rather than n_basic_blocks_for_fn.
* gcc.dg/tree-ssa/pr88444.c: New test.
--- gcc/tree-inline.c.jj2018-12-07 00:23:15.745986912 +0100
+++ gcc/tree-inline.c 2018-12-13 13:04:30.407956734 +0100
@@ -4906,7 +4906,7 @@ gimple_expand_calls_inline (basic_block
static void
fold_marked_statements (int first, hash_set *statements)
{
- for (; first < n_basic_blocks_for_fn (cfun); first++)
+ for (; first < last_basic_block_for_fn (cfun); first++)
if (BASIC_BLOCK_FOR_FN (cfun, first))
{
gimple_stmt_iterator gsi;
--- gcc/testsuite/gcc.dg/tree-ssa/pr88444.c.jj 2018-12-13 13:11:39.577990474
+0100
+++ gcc/testsuite/gcc.dg/tree-ssa/pr88444.c 2018-12-13 13:12:45.184925117
+0100
@@ -0,0 +1,6 @@
+/* PR tree-optimization/88444 */
+/* { dg-do compile } */
+/* { dg-options "-O1 -finline-functions -finline-small-functions
-fdump-tree-fixup_cfg3" } */
+/* { dg-final { scan-tree-dump-not " = \\(long int\\) 0;" "fixup_cfg3" } } */
+
+#include "../pr88444.c"
Jakub
