diff mbox

Fix thinko in SLSR that caused x86 bootstrap failure

Message ID 20130506192506.GQ28963@tucnak.redhat.com
State New
Headers show

Commit Message

Jakub Jelinek May 6, 2013, 7:25 p.m. UTC 
On Sun, May 05, 2013 at 03:45:17PM -0500, Bill Schmidt wrote:
> 2013-05-05  Bill Schmidt  <wschmidt@linux.vnet.ibm.com>
> 
> 	* gimple-ssa-strength-reduction.c (slsr_process_phi): Re-enable.
> 	(find_candidates_in_block): Re-enable slsr_process_phi.
> 	(create_phi_basis): Fix double counting of candidate adjustment.

This broke gcc.dg/pr33017.c testcase on i?86/x86_64 -m32.
./cc1 -O2 -ftree-vectorize -m32 -mno-sse pr33017.c
difference between r19862{6,7} is:

	Jakub

Comments

Bill Schmidt May 6, 2013, 7:29 p.m. UTC | #1 
On Mon, 2013-05-06 at 21:25 +0200, Jakub Jelinek wrote:
> On Sun, May 05, 2013 at 03:45:17PM -0500, Bill Schmidt wrote:
> > 2013-05-05  Bill Schmidt  <wschmidt@linux.vnet.ibm.com>
> > 
> > 	* gimple-ssa-strength-reduction.c (slsr_process_phi): Re-enable.
> > 	(find_candidates_in_block): Re-enable slsr_process_phi.
> > 	(create_phi_basis): Fix double counting of candidate adjustment.
> 
> This broke gcc.dg/pr33017.c testcase on i?86/x86_64 -m32.
> ./cc1 -O2 -ftree-vectorize -m32 -mno-sse pr33017.c

Jakub, thanks, I'll take a look.

Bill

> difference between r19862{6,7} is:
> --- pr33017.s1	2013-05-06 21:22:03.786745422 +0200
> +++ pr33017.s2	2013-05-06 21:22:16.844673015 +0200
> @@ -32,9 +32,9 @@ foo:
>  	movb	$87, var.1373+2(%eax)
>  	je	.L10
>  	cmpl	$3, %edx
> -	movb	$87, var.1373+3(%eax)
> +	movb	$87, var.1373+2(%eax,%eax)
>  	jne	.L11
> -	movb	$87, var.1373+4(%eax)
> +	movb	$87, var.1373+2(%eax,%eax,2)
>  	movl	$3, %ebp
>  	movl	$61, 28(%esp)
>  .L3:
> 
> 	Jakub
>
H.J. Lu May 7, 2013, 12:28 a.m. UTC | #2 
On Mon, May 6, 2013 at 12:29 PM, Bill Schmidt
<wschmidt@linux.vnet.ibm.com> wrote:
> On Mon, 2013-05-06 at 21:25 +0200, Jakub Jelinek wrote:
>> On Sun, May 05, 2013 at 03:45:17PM -0500, Bill Schmidt wrote:
>> > 2013-05-05  Bill Schmidt  <wschmidt@linux.vnet.ibm.com>
>> >
>> >     * gimple-ssa-strength-reduction.c (slsr_process_phi): Re-enable.
>> >     (find_candidates_in_block): Re-enable slsr_process_phi.
>> >     (create_phi_basis): Fix double counting of candidate adjustment.
>>
>> This broke gcc.dg/pr33017.c testcase on i?86/x86_64 -m32.
>> ./cc1 -O2 -ftree-vectorize -m32 -mno-sse pr33017.c
>
> Jakub, thanks, I'll take a look.
>
> Bill
>
>> difference between r19862{6,7} is:
>> --- pr33017.s1        2013-05-06 21:22:03.786745422 +0200
>> +++ pr33017.s2        2013-05-06 21:22:16.844673015 +0200
>> @@ -32,9 +32,9 @@ foo:
>>       movb    $87, var.1373+2(%eax)
>>       je      .L10
>>       cmpl    $3, %edx
>> -     movb    $87, var.1373+3(%eax)
>> +     movb    $87, var.1373+2(%eax,%eax)
>>       jne     .L11
>> -     movb    $87, var.1373+4(%eax)
>> +     movb    $87, var.1373+2(%eax,%eax,2)
>>       movl    $3, %ebp
>>       movl    $61, 28(%esp)
>>  .L3:
>>
>>       Jakub
>>
>
>

It also caused:


AIL: gcc.dg/vect/vect-28.c -flto execution test
FAIL: gcc.dg/vect/vect-28.c execution test
FAIL: gfortran.dg/array_constructor_9.f90  -O3 -fomit-frame-pointer
execution test
FAIL: gfortran.dg/array_constructor_9.f90  -O3 -fomit-frame-pointer
-funroll-all-loops -finline-functions  execution test
FAIL: gfortran.dg/array_constructor_9.f90  -O3 -fomit-frame-pointer
-funroll-loops  execution test
FAIL: gfortran.dg/array_constructor_9.f90  -O3 -g  execution test
FAIL: gfortran.dg/char_result_3.f90  -O3 -fomit-frame-pointer  execution test
FAIL: gfortran.dg/char_result_3.f90  -O3 -fomit-frame-pointer
-funroll-all-loops -finline-functions  execution test
FAIL: gfortran.dg/char_result_3.f90  -O3 -fomit-frame-pointer
-funroll-loops  execution test
FAIL: gfortran.dg/char_result_3.f90  -O3 -g  execution test
FAIL: gfortran.dg/cray_pointers_2.f90  -O  execution test
FAIL: gfortran.dg/realloc_on_assign_2.f03  -O3 -fomit-frame-pointer
execution test
FAIL: gfortran.dg/realloc_on_assign_2.f03  -O3 -fomit-frame-pointer
-funroll-all-loops -finline-functions  execution test
FAIL: gfortran.dg/realloc_on_assign_2.f03  -O3 -fomit-frame-pointer
-funroll-loops  execution test
FAIL: gfortran.dg/realloc_on_assign_2.f03  -O3 -g  execution test

on i686.

--
H.J.
Bill Schmidt May 7, 2013, 1:18 a.m. UTC | #3 
On Mon, 2013-05-06 at 17:28 -0700, H.J. Lu wrote:
> On Mon, May 6, 2013 at 12:29 PM, Bill Schmidt
> <wschmidt@linux.vnet.ibm.com> wrote:
> > On Mon, 2013-05-06 at 21:25 +0200, Jakub Jelinek wrote:
> >> On Sun, May 05, 2013 at 03:45:17PM -0500, Bill Schmidt wrote:
> >> > 2013-05-05  Bill Schmidt  <wschmidt@linux.vnet.ibm.com>
> >> >
> >> >     * gimple-ssa-strength-reduction.c (slsr_process_phi): Re-enable.
> >> >     (find_candidates_in_block): Re-enable slsr_process_phi.
> >> >     (create_phi_basis): Fix double counting of candidate adjustment.
> >>
> >> This broke gcc.dg/pr33017.c testcase on i?86/x86_64 -m32.
> >> ./cc1 -O2 -ftree-vectorize -m32 -mno-sse pr33017.c
> >
> > Jakub, thanks, I'll take a look.
> >
> > Bill
> >
> >> difference between r19862{6,7} is:
> >> --- pr33017.s1        2013-05-06 21:22:03.786745422 +0200
> >> +++ pr33017.s2        2013-05-06 21:22:16.844673015 +0200
> >> @@ -32,9 +32,9 @@ foo:
> >>       movb    $87, var.1373+2(%eax)
> >>       je      .L10
> >>       cmpl    $3, %edx
> >> -     movb    $87, var.1373+3(%eax)
> >> +     movb    $87, var.1373+2(%eax,%eax)
> >>       jne     .L11
> >> -     movb    $87, var.1373+4(%eax)
> >> +     movb    $87, var.1373+2(%eax,%eax,2)
> >>       movl    $3, %ebp
> >>       movl    $61, 28(%esp)
> >>  .L3:
> >>
> >>       Jakub
> >>
> >
> >
> 
> It also caused:
> 
> 
> AIL: gcc.dg/vect/vect-28.c -flto execution test
> FAIL: gcc.dg/vect/vect-28.c execution test
> FAIL: gfortran.dg/array_constructor_9.f90  -O3 -fomit-frame-pointer
> execution test
> FAIL: gfortran.dg/array_constructor_9.f90  -O3 -fomit-frame-pointer
> -funroll-all-loops -finline-functions  execution test
> FAIL: gfortran.dg/array_constructor_9.f90  -O3 -fomit-frame-pointer
> -funroll-loops  execution test
> FAIL: gfortran.dg/array_constructor_9.f90  -O3 -g  execution test
> FAIL: gfortran.dg/char_result_3.f90  -O3 -fomit-frame-pointer  execution test
> FAIL: gfortran.dg/char_result_3.f90  -O3 -fomit-frame-pointer
> -funroll-all-loops -finline-functions  execution test
> FAIL: gfortran.dg/char_result_3.f90  -O3 -fomit-frame-pointer
> -funroll-loops  execution test
> FAIL: gfortran.dg/char_result_3.f90  -O3 -g  execution test
> FAIL: gfortran.dg/cray_pointers_2.f90  -O  execution test
> FAIL: gfortran.dg/realloc_on_assign_2.f03  -O3 -fomit-frame-pointer
> execution test
> FAIL: gfortran.dg/realloc_on_assign_2.f03  -O3 -fomit-frame-pointer
> -funroll-all-loops -finline-functions  execution test
> FAIL: gfortran.dg/realloc_on_assign_2.f03  -O3 -fomit-frame-pointer
> -funroll-loops  execution test
> FAIL: gfortran.dg/realloc_on_assign_2.f03  -O3 -g  execution test
> 
> on i686.
> 
> --
> H.J.
> 

H.J., do you know whether the proposed patch fixes these failures?  I'll
take a look at these tomorrow if not.

Thanks,
Bill
Jakub Jelinek May 7, 2013, 9:34 a.m. UTC | #4 
On Mon, May 06, 2013 at 08:18:27PM -0500, Bill Schmidt wrote:
> > AIL: gcc.dg/vect/vect-28.c -flto execution test
> > FAIL: gcc.dg/vect/vect-28.c execution test
> > FAIL: gfortran.dg/array_constructor_9.f90  -O3 -fomit-frame-pointer
> > execution test
> > FAIL: gfortran.dg/array_constructor_9.f90  -O3 -fomit-frame-pointer
> > -funroll-all-loops -finline-functions  execution test
> > FAIL: gfortran.dg/array_constructor_9.f90  -O3 -fomit-frame-pointer
> > -funroll-loops  execution test
> > FAIL: gfortran.dg/array_constructor_9.f90  -O3 -g  execution test
> > FAIL: gfortran.dg/char_result_3.f90  -O3 -fomit-frame-pointer  execution test
> > FAIL: gfortran.dg/char_result_3.f90  -O3 -fomit-frame-pointer
> > -funroll-all-loops -finline-functions  execution test
> > FAIL: gfortran.dg/char_result_3.f90  -O3 -fomit-frame-pointer
> > -funroll-loops  execution test
> > FAIL: gfortran.dg/char_result_3.f90  -O3 -g  execution test
> > FAIL: gfortran.dg/cray_pointers_2.f90  -O  execution test
> > FAIL: gfortran.dg/realloc_on_assign_2.f03  -O3 -fomit-frame-pointer
> > execution test
> > FAIL: gfortran.dg/realloc_on_assign_2.f03  -O3 -fomit-frame-pointer
> > -funroll-all-loops -finline-functions  execution test
> > FAIL: gfortran.dg/realloc_on_assign_2.f03  -O3 -fomit-frame-pointer
> > -funroll-loops  execution test
> > FAIL: gfortran.dg/realloc_on_assign_2.f03  -O3 -g  execution test
> > 
> > on i686.
> > 
> > --
> > H.J.
> > 
> 
> H.J., do you know whether the proposed patch fixes these failures?  I'll
> take a look at these tomorrow if not.

+FAIL: gcc.dg/vect/vect-28.c execution test
+FAIL: gcc.dg/vect/vect-28.c -flto execution test

caused by the r198627 change isn't fixed with this patch (on i686-linux),
while pr33017.c is.  Similarly, on x86_64-linux, most of the new FAILs
H.J. lists above are fixed (including many libgomp FAILs), but
+FAIL: gfortran.dg/cray_pointers_2.f90  -O  execution test

+FAIL: libgomp.fortran/threadprivate2.f90  -O3 -fomit-frame-pointer execution test
+FAIL: libgomp.fortran/threadprivate2.f90  -O3 -fomit-frame-pointer -funroll-loops  execution test
+FAIL: libgomp.fortran/threadprivate2.f90  -O3 -fomit-frame-pointer -funroll-all-loops -finline-functions  execution test
+FAIL: libgomp.fortran/threadprivate2.f90  -O3 -g  execution test
still remain.  For these x86_64 FAILs I haven't verified they were
introduced by r198627, but they didn't fail May 3rd.

	Jakub
Bill Schmidt May 7, 2013, 11:45 a.m. UTC | #5 
On Tue, 2013-05-07 at 11:34 +0200, Jakub Jelinek wrote:
> On Mon, May 06, 2013 at 08:18:27PM -0500, Bill Schmidt wrote:
> > > AIL: gcc.dg/vect/vect-28.c -flto execution test
> > > FAIL: gcc.dg/vect/vect-28.c execution test
> > > FAIL: gfortran.dg/array_constructor_9.f90  -O3 -fomit-frame-pointer
> > > execution test
> > > FAIL: gfortran.dg/array_constructor_9.f90  -O3 -fomit-frame-pointer
> > > -funroll-all-loops -finline-functions  execution test
> > > FAIL: gfortran.dg/array_constructor_9.f90  -O3 -fomit-frame-pointer
> > > -funroll-loops  execution test
> > > FAIL: gfortran.dg/array_constructor_9.f90  -O3 -g  execution test
> > > FAIL: gfortran.dg/char_result_3.f90  -O3 -fomit-frame-pointer  execution test
> > > FAIL: gfortran.dg/char_result_3.f90  -O3 -fomit-frame-pointer
> > > -funroll-all-loops -finline-functions  execution test
> > > FAIL: gfortran.dg/char_result_3.f90  -O3 -fomit-frame-pointer
> > > -funroll-loops  execution test
> > > FAIL: gfortran.dg/char_result_3.f90  -O3 -g  execution test
> > > FAIL: gfortran.dg/cray_pointers_2.f90  -O  execution test
> > > FAIL: gfortran.dg/realloc_on_assign_2.f03  -O3 -fomit-frame-pointer
> > > execution test
> > > FAIL: gfortran.dg/realloc_on_assign_2.f03  -O3 -fomit-frame-pointer
> > > -funroll-all-loops -finline-functions  execution test
> > > FAIL: gfortran.dg/realloc_on_assign_2.f03  -O3 -fomit-frame-pointer
> > > -funroll-loops  execution test
> > > FAIL: gfortran.dg/realloc_on_assign_2.f03  -O3 -g  execution test
> > > 
> > > on i686.
> > > 
> > > --
> > > H.J.
> > > 
> > 
> > H.J., do you know whether the proposed patch fixes these failures?  I'll
> > take a look at these tomorrow if not.
> 
> +FAIL: gcc.dg/vect/vect-28.c execution test
> +FAIL: gcc.dg/vect/vect-28.c -flto execution test
> 
> caused by the r198627 change isn't fixed with this patch (on i686-linux),
> while pr33017.c is.  Similarly, on x86_64-linux, most of the new FAILs
> H.J. lists above are fixed (including many libgomp FAILs), but
> +FAIL: gfortran.dg/cray_pointers_2.f90  -O  execution test
> 
> +FAIL: libgomp.fortran/threadprivate2.f90  -O3 -fomit-frame-pointer execution test
> +FAIL: libgomp.fortran/threadprivate2.f90  -O3 -fomit-frame-pointer -funroll-loops  execution test
> +FAIL: libgomp.fortran/threadprivate2.f90  -O3 -fomit-frame-pointer -funroll-all-loops -finline-functions  execution test
> +FAIL: libgomp.fortran/threadprivate2.f90  -O3 -g  execution test
> still remain.  For these x86_64 FAILs I haven't verified they were
> introduced by r198627, but they didn't fail May 3rd.

Thanks for investigating!  I'll research these today.

Bill

> 
> 	Jakub
>
Bill Schmidt May 7, 2013, 4:22 p.m. UTC | #6 
On Tue, 2013-05-07 at 11:34 +0200, Jakub Jelinek wrote:
> On Mon, May 06, 2013 at 08:18:27PM -0500, Bill Schmidt wrote:
> > > AIL: gcc.dg/vect/vect-28.c -flto execution test
> > > FAIL: gcc.dg/vect/vect-28.c execution test
> > > FAIL: gfortran.dg/array_constructor_9.f90  -O3 -fomit-frame-pointer
> > > execution test
> > > FAIL: gfortran.dg/array_constructor_9.f90  -O3 -fomit-frame-pointer
> > > -funroll-all-loops -finline-functions  execution test
> > > FAIL: gfortran.dg/array_constructor_9.f90  -O3 -fomit-frame-pointer
> > > -funroll-loops  execution test
> > > FAIL: gfortran.dg/array_constructor_9.f90  -O3 -g  execution test
> > > FAIL: gfortran.dg/char_result_3.f90  -O3 -fomit-frame-pointer  execution test
> > > FAIL: gfortran.dg/char_result_3.f90  -O3 -fomit-frame-pointer
> > > -funroll-all-loops -finline-functions  execution test
> > > FAIL: gfortran.dg/char_result_3.f90  -O3 -fomit-frame-pointer
> > > -funroll-loops  execution test
> > > FAIL: gfortran.dg/char_result_3.f90  -O3 -g  execution test
> > > FAIL: gfortran.dg/cray_pointers_2.f90  -O  execution test
> > > FAIL: gfortran.dg/realloc_on_assign_2.f03  -O3 -fomit-frame-pointer
> > > execution test
> > > FAIL: gfortran.dg/realloc_on_assign_2.f03  -O3 -fomit-frame-pointer
> > > -funroll-all-loops -finline-functions  execution test
> > > FAIL: gfortran.dg/realloc_on_assign_2.f03  -O3 -fomit-frame-pointer
> > > -funroll-loops  execution test
> > > FAIL: gfortran.dg/realloc_on_assign_2.f03  -O3 -g  execution test
> > > 
> > > on i686.
> > > 
> > > --
> > > H.J.
> > > 
> > 
> > H.J., do you know whether the proposed patch fixes these failures?  I'll
> > take a look at these tomorrow if not.
> 
> +FAIL: gcc.dg/vect/vect-28.c execution test
> +FAIL: gcc.dg/vect/vect-28.c -flto execution test
> 
> caused by the r198627 change isn't fixed with this patch (on i686-linux),
> while pr33017.c is.  Similarly, on x86_64-linux, most of the new FAILs
> H.J. lists above are fixed (including many libgomp FAILs), but
> +FAIL: gfortran.dg/cray_pointers_2.f90  -O  execution test
> 
> +FAIL: libgomp.fortran/threadprivate2.f90  -O3 -fomit-frame-pointer execution test
> +FAIL: libgomp.fortran/threadprivate2.f90  -O3 -fomit-frame-pointer -funroll-loops  execution test
> +FAIL: libgomp.fortran/threadprivate2.f90  -O3 -fomit-frame-pointer -funroll-all-loops -finline-functions  execution test
> +FAIL: libgomp.fortran/threadprivate2.f90  -O3 -g  execution test
> still remain.  For these x86_64 FAILs I haven't verified they were
> introduced by r198627, but they didn't fail May 3rd.

I see what's going on now.  A rather fundamental mistake.  Conditional
candidates only work for CAND_MULTs, but I neglected to enforce this.
*blush*  Working on a fix.  Amazed I didn't hit this.

Bill

> 
> 	Jakub
>
diff mbox

Patch

--- pr33017.s1	2013-05-06 21:22:03.786745422 +0200
+++ pr33017.s2	2013-05-06 21:22:16.844673015 +0200
@@ -32,9 +32,9 @@  foo:
 	movb	$87, var.1373+2(%eax)
 	je	.L10
 	cmpl	$3, %edx
-	movb	$87, var.1373+3(%eax)
+	movb	$87, var.1373+2(%eax,%eax)
 	jne	.L11
-	movb	$87, var.1373+4(%eax)
+	movb	$87, var.1373+2(%eax,%eax,2)
 	movl	$3, %ebp
 	movl	$61, 28(%esp)
 .L3: