Re: [PATCH/RFC] tree-if-conv.c: fix two ICEs seen with -fno-tree-forwprop (PR tree-optimization/84178)
On Fri, 2018-02-16 at 12:48 +0100, Richard Biener wrote: > On Thu, Feb 15, 2018 at 11:07 PM, David Malcolm > wrote: > > On Fri, 2018-02-09 at 12:02 +0100, Richard Biener wrote: > > > On Thu, Feb 8, 2018 at 11:23 PM, David Malcolm > > om> > > > wrote: > > > > PR tree-optimization/84178 reports a couple of source files > > > > that > > > > ICE inside > > > > ifcvt when compiled with -03 -fno-tree-forwprop (trunk and gcc > > > > 7). > > > > > > > > Both cases involve problems with ifcvt's per-BB gimplified > > > > predicates. > > > > > > > > Testcase 1 fails this assertion within release_bb_predicate > > > > during > > > > cleanup: > > > > > > > > 283 if (flag_checking) > > > > 284 for (gimple_stmt_iterator i = gsi_start > > > > (stmts); > > > > 285 !gsi_end_p (i); gsi_next (&i)) > > > > 286 gcc_assert (! gimple_use_ops (gsi_stmt (i))); > > > > > > > > The testcase contains a division in the loop, which leads to > > > > if_convertible_loop_p returning false (due to > > > > gimple_could_trap_p > > > > being true > > > > for the division). This happens *after* the per-BB gimplified > > > > predicates > > > > have been created in predicate_bbs (loop). > > > > Hence tree_if_conversion bails out to "cleanup", but the > > > > gimplified > > > > predicates > > > > exist and make use of SSA names; for example this conjunction > > > > for > > > > two BB > > > > conditions: > > > > > > > > _4 = h4.1_112 != 0; > > > > _175 = (signed char) _117; > > > > _176 = _175 >= 0; > > > > _174 = _4 & _176; > > > > > > > > is using SSA names. > > > > > > But then this shouldn't cause any stmt operands to be created - > > > who > > > is calling > > > update_stmt () on a stmt using the SSA names? Maybe something > > > calls > > > force_gimple_operand_gsi to add to the sequence? > > > > > > The immediate use is created deep within folding when the > > gimplified > > predicate is created. > > > > Here's the backtrace of exactly where: > > > > (gdb) bt > > #0 link_imm_use_stmt (linknode=0x71a0b8d0, def= > 0x71a196c0>, stmt=) > > at ../../src/gcc/ssa-iterators.h:307 > > #1 0x012531c5 in add_use_op (fn=0x71a03000, > > stmt=, op=0x71a236d8, > > last=0x7fffcb10) at ../../src/gcc/tree-ssa-operands.c:307 > > #2 0x01253607 in finalize_ssa_uses (fn=0x71a03000, > > stmt=) > > at ../../src/gcc/tree-ssa-operands.c:410 > > #3 0x0125368b in finalize_ssa_stmt_operands > > (fn=0x71a03000, stmt=) > > at ../../src/gcc/tree-ssa-operands.c:436 > > #4 0x01254b62 in build_ssa_operands (fn=0x71a03000, > > stmt=) > > at ../../src/gcc/tree-ssa-operands.c:948 > > #5 0x012550df in update_stmt_operands (fn=0x71a03000, > > stmt=) > > at ../../src/gcc/tree-ssa-operands.c:1081 > > #6 0x00c10642 in update_stmt_if_modified (s= > 0x71a23690>) at ../../src/gcc/gimple-ssa.h:185 > > #7 0x00c10e82 in update_modified_stmts > > (seq=0x71a23690) at ../../src/gcc/gimple-iterator.c:58 > > #8 0x00c111f1 in gsi_insert_seq_before (i=0x7fffcfb0, > > seq=0x71a23690, mode=GSI_SAME_STMT) > > at ../../src/gcc/gimple-iterator.c:217 > > #9 0x00c241d0 in replace_stmt_with_simplification > > (gsi=0x7fffcfb0, rcode=..., ops=0x7fffcdb0, > > seq=0x7fffcdd8, inplace=false) at ../../src/gcc/gimple- > > fold.c:4473 > > #10 0x00c25a63 in fold_stmt_1 (gsi=0x7fffcfb0, > > inplace=false, valueize=0xc2663b ) > > at ../../src/gcc/gimple-fold.c:4775 > > #11 0x00c266b7 in fold_stmt (gsi=0x7fffcfb0) at > > ../../src/gcc/gimple-fold.c:4996 > > #12 0x00c552b1 in maybe_fold_stmt (gsi=0x7fffcfb0) at > > ../../src/gcc/gimplify.c:3193 > > #13 0x00c5f1e9 in gimplify_modify_expr > > (expr_p=0x7fffd328, pre_p=0x7fffd910, > > post_p=0x7fffd1e0, > > want_value=false) at ../../src/gcc/gimplify.c:5803 > > #14 0x00c7b461 in gimplify_expr (expr_p=0x7fffd328, > > pre_p=0x7fffd910, post_p=0x7fffd1e0, > > gimple_test_f=0xc5d723 , fallback=0) at > > ../../src/gcc/gimplify.c:11434 > > #15 0x00c62661 in gimplify_stmt (stmt_p=0x7fffd328, > > seq_p=0x7fffd910) at ../../src/gcc/gimplify.c:6658 > > #16 0x00c4c449 in gimplify_and_add (t= > 0x71a26230>, seq_p=0x7fffd910) at > > ../../src/gcc/gimplify.c:441 > > #17 0x00c4cc89 in internal_get_tmp_var (val= > 0x71a26140>, pre_p=0x7fffd910, post_p=0x0, is_formal=true, > > allow_ssa=true) at ../../src/gcc/gimplify.c:597 > > #18 0x00c4ccd2 in get_formal_tmp_var (val= > 0x71a26140>, pre_p=0x7fffd910) at > > ../../src/gcc/gimplify.c:618 > > #19 0x00c7ee6a in gimplify_expr (expr_p=0x71a261b0, > > pre_p=0x7fffd910, post_p=0x7fffd790, > > gimple_test_f=0xc0f6d0 , fallback=1) > > at ../../src/gcc/gimplify.c:12383 > > #20 0x00c7e2e9 in gimplify_expr (expr_p=0x
Re: [PATCH/RFC] tree-if-conv.c: fix two ICEs seen with -fno-tree-forwprop (PR tree-optimization/84178)
On Thu, Feb 15, 2018 at 11:07 PM, David Malcolm wrote: > On Fri, 2018-02-09 at 12:02 +0100, Richard Biener wrote: >> On Thu, Feb 8, 2018 at 11:23 PM, David Malcolm >> wrote: >> > PR tree-optimization/84178 reports a couple of source files that >> > ICE inside >> > ifcvt when compiled with -03 -fno-tree-forwprop (trunk and gcc 7). >> > >> > Both cases involve problems with ifcvt's per-BB gimplified >> > predicates. >> > >> > Testcase 1 fails this assertion within release_bb_predicate during >> > cleanup: >> > >> > 283 if (flag_checking) >> > 284 for (gimple_stmt_iterator i = gsi_start (stmts); >> > 285 !gsi_end_p (i); gsi_next (&i)) >> > 286 gcc_assert (! gimple_use_ops (gsi_stmt (i))); >> > >> > The testcase contains a division in the loop, which leads to >> > if_convertible_loop_p returning false (due to gimple_could_trap_p >> > being true >> > for the division). This happens *after* the per-BB gimplified >> > predicates >> > have been created in predicate_bbs (loop). >> > Hence tree_if_conversion bails out to "cleanup", but the gimplified >> > predicates >> > exist and make use of SSA names; for example this conjunction for >> > two BB >> > conditions: >> > >> > _4 = h4.1_112 != 0; >> > _175 = (signed char) _117; >> > _176 = _175 >= 0; >> > _174 = _4 & _176; >> > >> > is using SSA names. >> >> But then this shouldn't cause any stmt operands to be created - who >> is calling >> update_stmt () on a stmt using the SSA names? Maybe something calls >> force_gimple_operand_gsi to add to the sequence? > > > The immediate use is created deep within folding when the gimplified > predicate is created. > > Here's the backtrace of exactly where: > > (gdb) bt > #0 link_imm_use_stmt (linknode=0x71a0b8d0, def= 0x71a196c0>, stmt=) > at ../../src/gcc/ssa-iterators.h:307 > #1 0x012531c5 in add_use_op (fn=0x71a03000, stmt= 0x71a23690>, op=0x71a236d8, > last=0x7fffcb10) at ../../src/gcc/tree-ssa-operands.c:307 > #2 0x01253607 in finalize_ssa_uses (fn=0x71a03000, > stmt=) > at ../../src/gcc/tree-ssa-operands.c:410 > #3 0x0125368b in finalize_ssa_stmt_operands (fn=0x71a03000, > stmt=) > at ../../src/gcc/tree-ssa-operands.c:436 > #4 0x01254b62 in build_ssa_operands (fn=0x71a03000, > stmt=) > at ../../src/gcc/tree-ssa-operands.c:948 > #5 0x012550df in update_stmt_operands (fn=0x71a03000, > stmt=) > at ../../src/gcc/tree-ssa-operands.c:1081 > #6 0x00c10642 in update_stmt_if_modified (s= 0x71a23690>) at ../../src/gcc/gimple-ssa.h:185 > #7 0x00c10e82 in update_modified_stmts (seq=0x71a23690) at > ../../src/gcc/gimple-iterator.c:58 > #8 0x00c111f1 in gsi_insert_seq_before (i=0x7fffcfb0, > seq=0x71a23690, mode=GSI_SAME_STMT) > at ../../src/gcc/gimple-iterator.c:217 > #9 0x00c241d0 in replace_stmt_with_simplification > (gsi=0x7fffcfb0, rcode=..., ops=0x7fffcdb0, > seq=0x7fffcdd8, inplace=false) at ../../src/gcc/gimple-fold.c:4473 > #10 0x00c25a63 in fold_stmt_1 (gsi=0x7fffcfb0, inplace=false, > valueize=0xc2663b ) > at ../../src/gcc/gimple-fold.c:4775 > #11 0x00c266b7 in fold_stmt (gsi=0x7fffcfb0) at > ../../src/gcc/gimple-fold.c:4996 > #12 0x00c552b1 in maybe_fold_stmt (gsi=0x7fffcfb0) at > ../../src/gcc/gimplify.c:3193 > #13 0x00c5f1e9 in gimplify_modify_expr (expr_p=0x7fffd328, > pre_p=0x7fffd910, post_p=0x7fffd1e0, > want_value=false) at ../../src/gcc/gimplify.c:5803 > #14 0x00c7b461 in gimplify_expr (expr_p=0x7fffd328, > pre_p=0x7fffd910, post_p=0x7fffd1e0, > gimple_test_f=0xc5d723 , fallback=0) at > ../../src/gcc/gimplify.c:11434 > #15 0x00c62661 in gimplify_stmt (stmt_p=0x7fffd328, > seq_p=0x7fffd910) at ../../src/gcc/gimplify.c:6658 > #16 0x00c4c449 in gimplify_and_add (t=, > seq_p=0x7fffd910) at ../../src/gcc/gimplify.c:441 > #17 0x00c4cc89 in internal_get_tmp_var (val=, > pre_p=0x7fffd910, post_p=0x0, is_formal=true, > allow_ssa=true) at ../../src/gcc/gimplify.c:597 > #18 0x00c4ccd2 in get_formal_tmp_var (val=, > pre_p=0x7fffd910) at ../../src/gcc/gimplify.c:618 > #19 0x00c7ee6a in gimplify_expr (expr_p=0x71a261b0, > pre_p=0x7fffd910, post_p=0x7fffd790, > gimple_test_f=0xc0f6d0 , fallback=1) at > ../../src/gcc/gimplify.c:12383 > #20 0x00c7e2e9 in gimplify_expr (expr_p=0x7fffd8b8, > pre_p=0x7fffd910, post_p=0x7fffd790, > gimple_test_f=0xc0ef75 , fallback=1) at > ../../src/gcc/gimplify.c:12160 > #21 0x00c83de5 in force_gimple_operand_1 (expr= 0x71a26190>, stmts=0x7fffd910, > gimple_test_f=0xc0ef75 , var=) > at ../../src/gcc/gimplify-me.c:78 > #22 0x010c6387 in add_to_predicate_list (loop=0x71a0a330, > bb=, > nc=) at ../../src/gcc/tree-if
Re: [PATCH/RFC] tree-if-conv.c: fix two ICEs seen with -fno-tree-forwprop (PR tree-optimization/84178)
On Fri, 2018-02-09 at 12:02 +0100, Richard Biener wrote: > On Thu, Feb 8, 2018 at 11:23 PM, David Malcolm > wrote: > > PR tree-optimization/84178 reports a couple of source files that > > ICE inside > > ifcvt when compiled with -03 -fno-tree-forwprop (trunk and gcc 7). > > > > Both cases involve problems with ifcvt's per-BB gimplified > > predicates. > > > > Testcase 1 fails this assertion within release_bb_predicate during > > cleanup: > > > > 283 if (flag_checking) > > 284 for (gimple_stmt_iterator i = gsi_start (stmts); > > 285 !gsi_end_p (i); gsi_next (&i)) > > 286 gcc_assert (! gimple_use_ops (gsi_stmt (i))); > > > > The testcase contains a division in the loop, which leads to > > if_convertible_loop_p returning false (due to gimple_could_trap_p > > being true > > for the division). This happens *after* the per-BB gimplified > > predicates > > have been created in predicate_bbs (loop). > > Hence tree_if_conversion bails out to "cleanup", but the gimplified > > predicates > > exist and make use of SSA names; for example this conjunction for > > two BB > > conditions: > > > > _4 = h4.1_112 != 0; > > _175 = (signed char) _117; > > _176 = _175 >= 0; > > _174 = _4 & _176; > > > > is using SSA names. > > But then this shouldn't cause any stmt operands to be created - who > is calling > update_stmt () on a stmt using the SSA names? Maybe something calls > force_gimple_operand_gsi to add to the sequence? The immediate use is created deep within folding when the gimplified predicate is created. Here's the backtrace of exactly where: (gdb) bt #0 link_imm_use_stmt (linknode=0x71a0b8d0, def=, stmt=) at ../../src/gcc/ssa-iterators.h:307 #1 0x012531c5 in add_use_op (fn=0x71a03000, stmt=, op=0x71a236d8, last=0x7fffcb10) at ../../src/gcc/tree-ssa-operands.c:307 #2 0x01253607 in finalize_ssa_uses (fn=0x71a03000, stmt=) at ../../src/gcc/tree-ssa-operands.c:410 #3 0x0125368b in finalize_ssa_stmt_operands (fn=0x71a03000, stmt=) at ../../src/gcc/tree-ssa-operands.c:436 #4 0x01254b62 in build_ssa_operands (fn=0x71a03000, stmt=) at ../../src/gcc/tree-ssa-operands.c:948 #5 0x012550df in update_stmt_operands (fn=0x71a03000, stmt=) at ../../src/gcc/tree-ssa-operands.c:1081 #6 0x00c10642 in update_stmt_if_modified (s=) at ../../src/gcc/gimple-ssa.h:185 #7 0x00c10e82 in update_modified_stmts (seq=0x71a23690) at ../../src/gcc/gimple-iterator.c:58 #8 0x00c111f1 in gsi_insert_seq_before (i=0x7fffcfb0, seq=0x71a23690, mode=GSI_SAME_STMT) at ../../src/gcc/gimple-iterator.c:217 #9 0x00c241d0 in replace_stmt_with_simplification (gsi=0x7fffcfb0, rcode=..., ops=0x7fffcdb0, seq=0x7fffcdd8, inplace=false) at ../../src/gcc/gimple-fold.c:4473 #10 0x00c25a63 in fold_stmt_1 (gsi=0x7fffcfb0, inplace=false, valueize=0xc2663b ) at ../../src/gcc/gimple-fold.c:4775 #11 0x00c266b7 in fold_stmt (gsi=0x7fffcfb0) at ../../src/gcc/gimple-fold.c:4996 #12 0x00c552b1 in maybe_fold_stmt (gsi=0x7fffcfb0) at ../../src/gcc/gimplify.c:3193 #13 0x00c5f1e9 in gimplify_modify_expr (expr_p=0x7fffd328, pre_p=0x7fffd910, post_p=0x7fffd1e0, want_value=false) at ../../src/gcc/gimplify.c:5803 #14 0x00c7b461 in gimplify_expr (expr_p=0x7fffd328, pre_p=0x7fffd910, post_p=0x7fffd1e0, gimple_test_f=0xc5d723 , fallback=0) at ../../src/gcc/gimplify.c:11434 #15 0x00c62661 in gimplify_stmt (stmt_p=0x7fffd328, seq_p=0x7fffd910) at ../../src/gcc/gimplify.c:6658 #16 0x00c4c449 in gimplify_and_add (t=, seq_p=0x7fffd910) at ../../src/gcc/gimplify.c:441 #17 0x00c4cc89 in internal_get_tmp_var (val=, pre_p=0x7fffd910, post_p=0x0, is_formal=true, allow_ssa=true) at ../../src/gcc/gimplify.c:597 #18 0x00c4ccd2 in get_formal_tmp_var (val=, pre_p=0x7fffd910) at ../../src/gcc/gimplify.c:618 #19 0x00c7ee6a in gimplify_expr (expr_p=0x71a261b0, pre_p=0x7fffd910, post_p=0x7fffd790, gimple_test_f=0xc0f6d0 , fallback=1) at ../../src/gcc/gimplify.c:12383 #20 0x00c7e2e9 in gimplify_expr (expr_p=0x7fffd8b8, pre_p=0x7fffd910, post_p=0x7fffd790, gimple_test_f=0xc0ef75 , fallback=1) at ../../src/gcc/gimplify.c:12160 #21 0x00c83de5 in force_gimple_operand_1 (expr=, stmts=0x7fffd910, gimple_test_f=0xc0ef75 , var=) at ../../src/gcc/gimplify-me.c:78 #22 0x010c6387 in add_to_predicate_list (loop=0x71a0a330, bb=, nc=) at ../../src/gcc/tree-if-conv.c:535 #23 0x010c6480 in add_to_dst_predicate_list (loop=0x71a0a330, e= 10)>, prev_cond=, cond=) at ../../src/gcc/tree-if-conv.c:557 #24 0x010c7f51 in predicate_bbs (loop=0x71a0a330) at ../../src/gcc/tree-if-conv.c:1277 #25 0x010c84af i
Re: [PATCH/RFC] tree-if-conv.c: fix two ICEs seen with -fno-tree-forwprop (PR tree-optimization/84178)
On Thu, Feb 8, 2018 at 11:23 PM, David Malcolm wrote: > PR tree-optimization/84178 reports a couple of source files that ICE inside > ifcvt when compiled with -03 -fno-tree-forwprop (trunk and gcc 7). > > Both cases involve problems with ifcvt's per-BB gimplified predicates. > > Testcase 1 fails this assertion within release_bb_predicate during cleanup: > > 283 if (flag_checking) > 284 for (gimple_stmt_iterator i = gsi_start (stmts); > 285 !gsi_end_p (i); gsi_next (&i)) > 286 gcc_assert (! gimple_use_ops (gsi_stmt (i))); > > The testcase contains a division in the loop, which leads to > if_convertible_loop_p returning false (due to gimple_could_trap_p being true > for the division). This happens *after* the per-BB gimplified predicates > have been created in predicate_bbs (loop). > Hence tree_if_conversion bails out to "cleanup", but the gimplified predicates > exist and make use of SSA names; for example this conjunction for two BB > conditions: > > _4 = h4.1_112 != 0; > _175 = (signed char) _117; > _176 = _175 >= 0; > _174 = _4 & _176; > > is using SSA names. But then this shouldn't cause any stmt operands to be created - who is calling update_stmt () on a stmt using the SSA names? Maybe something calls force_gimple_operand_gsi to add to the sequence? > This assertion was added in r236498 (aka > c3deca2519d97c55876869c57cf11ae1e5c6cf8b): > > 2016-05-20 Richard Biener > > * tree-if-conv.c (add_bb_predicate_gimplified_stmts): Use > gimple_seq_add_seq_without_update. > (release_bb_predicate): Assert we have no operands to free. > (if_convertible_loop_p_1): Calculate post dominators later. > Do not free BB predicates here. > (combine_blocks): Do not recompute BB predicates. > (version_loop_for_if_conversion): Save BB predicates around > loop versioning. > > * gcc.dg/tree-ssa/ifc-cd.c: Adjust. > > The following patch fixes this by removing the assertion, and reinstating the > cleanup of the operands. But that was supposed to be not necessary... I'll note that simply restoring the old behavior is not ideal either - luckily we now have gimple_seq_discard () which should do a better job here (and actually does what the function comment says!). > > Testcase 2 segfaults inside update_ssa when called from > version_loop_for_if_conversion when a loop is versioned. > > During loop_version, some blocks are duplicated, and this can duplicate > SSA names, leading to the duplicated SSA names being added to > tree-into-ssa.c's old_ssa_names. > > For example, _117 is an SSA name defined in one of these to-be-duplicated > blocks, and hence is added to old_ssa_names when duplicated. > > One of the BBs has this gimplified predicate (again, these stmts are *not* > yet in a BB): > _173 = h4.1_112 != 0; > _171 = (signed char) _117; > _172 = _171 >= 0; > _170 = ~_172; > _169 = _170 & _173; > > Note the reference to SSA name _117 in the above. > > When update_ssa runs later on in version_loop_for_if_conversion, > SSA name _117 is in the old_ssa_names bitmap, and thus has > prepare_use_sites_for called on it: > > 2771 /* If an old name is in NAMES_TO_RELEASE, we cannot remove it from > 2772 OLD_SSA_NAMES, but we have to ignore its definition site. */ > 2773 EXECUTE_IF_SET_IN_BITMAP (old_ssa_names, 0, i, sbi) > 2774{ > 2775 if (names_to_release == NULL || !bitmap_bit_p > (names_to_release, i)) > 2776prepare_def_site_for (ssa_name (i), insert_phi_p); > 2777 prepare_use_sites_for (ssa_name (i), insert_phi_p); > 2778} > > prepare_use_sites_for iterates over the immediate users, which includes > the: > _171 = (signed char) _117; > in the gimplified predicate. > > This tried to call "mark_block_for_update" on the BB of this def_stmt, > leading to a read-through-NULL segfault, since this statement isn't in a > BB yet. > > With the caveat that this is at the limit of my understanding of the > innards of gimple, I'm wondering how this ever works: we have gimplified > predicates that aren't in a BB yet, which typically refer to > SSA names in the CFG proper, and we're attempting non-trivial manipulations > of that CFG that can e.g. duplicate those SSA names. > > The following patch fixes the 2nd ICE by inserting the gimplified predicates > earlier: immediately before the possible version_loop_for_if_conversion, > rather than within combine_blocks. That way they're in their BBs before > we attempt any further manipulation of the CFG. Hum, but that will alter both copies of the loops, no? I think the fix is to instead delay the update_ssa call to _after_ combine_blocks () (and remember if it is necessary just in case we didn't version). Richard. > This fixes the ICE, though it introduces a regression of > gcc.target/i386/avx2-vec-mask-bit-not.c > which no longer vectorizes for some reason (I haven't investigated >