Re: [PATCH] Fix transform_to_exit_first_loop_alt with -g

[Richard Biener <rguenther at suse dot de>]

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