| Message ID | 563C6806.1090102@mentor.com |
|---|---|
| State | New |
| Headers | show |
On Fri, 6 Nov 2015, Tom de Vries wrote: > Hi, > > This patch fixes a problem with -g compilation in > transform_to_exit_first_loop_alt. > > Consider test-case test.c: > ... > void > f (int *a, int n) > { > int i; > for (i = 0; i < n; ++i) > a[i] = 1; > } > ... > > If we add a "checking_verify_ssa (true, true)" call at the end of > transform_to_exit_first_loop_alt, and we compile with "-g -O2 > -ftree-parallelize-loops=4", we run into this ICE: > ... > test.c: In function âfâ: > test.c:2:1: error: definition in block 5 does not dominate use in block 13 > for SSA_NAME: i_10 in statement: > # DEBUG i => i_10 > test.c:2:1: internal compiler error: verify_ssa failed > ... > > Before transform_to_exit_first_loop_alt, the loop looks like: > ... > <bb 11>: > > <bb 5>: > # ivtmp_22 = PHI <0(11), ivtmp_23(7)> > i_13 = ivtmp_22; > # DEBUG i => i_13 > _5 = (long unsigned int) i_13; > _6 = _5 * 4; > _8 = a_7(D) + _6; > *_8 = 1; > i_10 = i_13 + 1; > # DEBUG i => i_10 > # DEBUG i => i_10 > if (ivtmp_22 < _1) > goto <bb 7>; > else > goto <bb 6>; > > <bb 7>: > ivtmp_23 = ivtmp_22 + 1; > goto <bb 5>; > ... > > > And after transform_to_exit_first_loop_alt, it looks like: > ... > <bb 11>: > goto <bb 13>; > > <bb 5>: > # ivtmp_22 = PHI <ivtmp_25(13)> > i_13 = ivtmp_22; > # DEBUG i => i_13 > _5 = (long unsigned int) i_13; > _6 = _5 * 4; > _8 = a_7(D) + _6; > *_8 = 1; > i_10 = i_13 + 1; > goto <bb 7>; > > <bb 13>: > # ivtmp_25 = PHI <ivtmp_23(7), 0(11)> > # DEBUG i => i_10 > # DEBUG i => i_10 > if (ivtmp_25 < _2) > goto <bb 5>; > else > goto <bb 14>; > > <bb 7>: > ivtmp_23 = ivtmp_22 + 1; > goto <bb 13>; > ... > > The ICE triggers because the use of i_10 in debug insn 'DEBUG i => i_10' in bb > 13 is no longer dominated by the defition of i_10 in bb 5. > > The patch fixes the ICE by ensuring that gimple_split_block_before_cond_jump > really splits before cond_jump, instead of after the last nondebug insn before > cond_jump, as it does now. This behaviour also better matches the rtl > implementation of the cfghook. Btw, note that the only user of cfghook > split_block_before_cond_jump is transform_to_exit_first_loop_alt. > > [ A similar fix for an openacc variant of this ICE was committed on the > gomp-4_0-branch: https://gcc.gnu.org/ml/gcc-patches/2015-07/msg00060.html ] > > Bootstrapped and reg-tested on x86_64. > > OK for trunk? Ok. Richard. > Thanks, > - Tom
Fix transform_to_exit_first_loop_alt with -g 2015-11-06 Tom de Vries <tom@codesourcery.com> * tree-cfg.c (gimple_split_block_before_cond_jump): Split before cond_jump, instead of split after last nondebug insn before cond_jump. * tree-parloops.c (transform_to_exit_first_loop_alt): Verify ssa before returning. --- gcc/tree-cfg.c | 2 +- gcc/tree-parloops.c | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/gcc/tree-cfg.c b/gcc/tree-cfg.c index cfed3c2..5d98eec 100644 --- a/gcc/tree-cfg.c +++ b/gcc/tree-cfg.c @@ -5783,7 +5783,7 @@ gimple_split_block_before_cond_jump (basic_block bb) if (gimple_code (last) != GIMPLE_COND && gimple_code (last) != GIMPLE_SWITCH) return NULL; - gsi_prev_nondebug (&gsi); + gsi_prev (&gsi); split_point = gsi_stmt (gsi); return split_block (bb, split_point)->dest; } diff --git a/gcc/tree-parloops.c b/gcc/tree-parloops.c index 6c85634..3d41275 100644 --- a/gcc/tree-parloops.c +++ b/gcc/tree-parloops.c @@ -1737,6 +1737,8 @@ transform_to_exit_first_loop_alt (struct loop *loop, /* Recalculate dominance info. */ free_dominance_info (CDI_DOMINATORS); calculate_dominance_info (CDI_DOMINATORS); + + checking_verify_ssa (true, true); } /* Tries to moves the exit condition of LOOP to the beginning of its header -- 1.9.1
Hi, This patch fixes a problem with -g compilation in transform_to_exit_first_loop_alt. Consider test-case test.c: ... void f (int *a, int n) { int i; for (i = 0; i < n; ++i) a[i] = 1; } ... If we add a "checking_verify_ssa (true, true)" call at the end of transform_to_exit_first_loop_alt, and we compile with "-g -O2 -ftree-parallelize-loops=4", we run into this ICE: ... test.c: In function âfâ: test.c:2:1: error: definition in block 5 does not dominate use in block 13 for SSA_NAME: i_10 in statement: # DEBUG i => i_10 test.c:2:1: internal compiler error: verify_ssa failed ... Before transform_to_exit_first_loop_alt, the loop looks like: ... <bb 11>: <bb 5>: # ivtmp_22 = PHI <0(11), ivtmp_23(7)> i_13 = ivtmp_22; # DEBUG i => i_13 _5 = (long unsigned int) i_13; _6 = _5 * 4; _8 = a_7(D) + _6; *_8 = 1; i_10 = i_13 + 1; # DEBUG i => i_10 # DEBUG i => i_10 if (ivtmp_22 < _1) goto <bb 7>; else goto <bb 6>; <bb 7>: ivtmp_23 = ivtmp_22 + 1; goto <bb 5>; ... And after transform_to_exit_first_loop_alt, it looks like: ... <bb 11>: goto <bb 13>; <bb 5>: # ivtmp_22 = PHI <ivtmp_25(13)> i_13 = ivtmp_22; # DEBUG i => i_13 _5 = (long unsigned int) i_13; _6 = _5 * 4; _8 = a_7(D) + _6; *_8 = 1; i_10 = i_13 + 1; goto <bb 7>; <bb 13>: # ivtmp_25 = PHI <ivtmp_23(7), 0(11)> # DEBUG i => i_10 # DEBUG i => i_10 if (ivtmp_25 < _2) goto <bb 5>; else goto <bb 14>; <bb 7>: ivtmp_23 = ivtmp_22 + 1; goto <bb 13>; ... The ICE triggers because the use of i_10 in debug insn 'DEBUG i => i_10' in bb 13 is no longer dominated by the defition of i_10 in bb 5. The patch fixes the ICE by ensuring that gimple_split_block_before_cond_jump really splits before cond_jump, instead of after the last nondebug insn before cond_jump, as it does now. This behaviour also better matches the rtl implementation of the cfghook. Btw, note that the only user of cfghook split_block_before_cond_jump is transform_to_exit_first_loop_alt. [ A similar fix for an openacc variant of this ICE was committed on the gomp-4_0-branch: https://gcc.gnu.org/ml/gcc-patches/2015-07/msg00060.html ] Bootstrapped and reg-tested on x86_64. OK for trunk? Thanks, - Tom