diff mbox

Disable if_conversion2 for Og

Message ID 000301cf5948$05399190$0facb4b0$@arm.com
State New
Headers show

Commit Message

Joey Ye April 16, 2014, 7:46 a.m. UTC 
> -----Original Message-----
> From: Joey Ye [mailto:joey.ye@arm.com]
> Sent: Tuesday, April 15, 2014 6:37 PM
> To: 'Richard Biener'
> Cc: GCC Patches
> Subject: RE: [patch] Disable if_conversion2 for Og
> 
> > Ok for trunk and branches after a while.  Why does if-conversion not have
> > the same problem?  On the GIMPLE part we avoid all kinds of if-conversion
> > with -Og.
> I think if-conversion should be disabled for Og too, but I don't have a case to
> show that it is harmful. If GIMPLE avoids all if-conversion, it is nature to do
> the same for RTL. I'll test and send another patch to also disable if-conversion.
New patch tested with more regressions with -Og, which are expected.
FAIL: gcc.target/arm/its.c scan-assembler-times \\tit 2
FAIL: gcc.target/arm/pr40956.c scan-assembler-times mov[\\\\t ]*r., #0 1
FAIL: gcc.target/arm/pr42835.c scan-assembler-times moveq[\\\\t ]*r.,[\\\\t ]*# 1
FAIL: gcc.target/arm/pr42835.c scan-assembler-times movne[\\\\t ]*r.,[\\\\t ]*# 1
FAIL: gcc.target/arm/shiftable.c scan-assembler sub.*[al]sl #6
FAIL: gcc.target/arm/thumb-ifcvt-2.c scan-assembler asreq
FAIL: gcc.target/arm/thumb-ifcvt-2.c scan-assembler lslne
FAIL: gcc.target/arm/thumb-ifcvt.c scan-assembler asrne
FAIL: gcc.target/arm/thumb-ifcvt.c scan-assembler lslne
FAIL: gcc.target/arm/vseleqdf.c scan-assembler-times vseleq.f64\\td[0-9]+ 1
FAIL: gcc.target/arm/vseleqsf.c scan-assembler-times vseleq.f32\\ts[0-9]+ 1
FAIL: gcc.target/arm/vselgedf.c scan-assembler-times vselge.f64\\td[0-9]+ 1
FAIL: gcc.target/arm/vselgesf.c scan-assembler-times vselge.f32\\ts[0-9]+ 1
FAIL: gcc.target/arm/vselgtdf.c scan-assembler-times vselgt.f64\\td[0-9]+ 1
FAIL: gcc.target/arm/vselgtsf.c scan-assembler-times vselgt.f32\\ts[0-9]+ 1
FAIL: gcc.target/arm/vselledf.c scan-assembler-times vselgt.f64\\td[0-9]+ 1
FAIL: gcc.target/arm/vsellesf.c scan-assembler-times vselgt.f32\\ts[0-9]+ 1
FAIL: gcc.target/arm/vselltdf.c scan-assembler-times vselge.f64\\td[0-9]+ 1
FAIL: gcc.target/arm/vselltsf.c scan-assembler-times vselge.f32\\ts[0-9]+ 1
FAIL: gcc.target/arm/vselnedf.c scan-assembler-times vseleq.f64\\td[0-9]+ 1
FAIL: gcc.target/arm/vselnesf.c scan-assembler-times vseleq.f32\\ts[0-9]+ 1
FAIL: gcc.target/arm/vselvcdf.c scan-assembler-times vselvs.f64\\td[0-9]+ 1
FAIL: gcc.target/arm/vselvcsf.c scan-assembler-times vselvs.f32\\ts[0-9]+ 1
FAIL: gcc.target/arm/vselvsdf.c scan-assembler-times vselvs.f64\\td[0-9]+ 1
FAIL: gcc.target/arm/vselvssf.c scan-assembler-times vselvs.f32\\ts[0-9]+ 1

OK?

ChangeLog:
    * opts.c (OPT_fif_conversion, OPT_fif_conversion2): Disable for Og.

Comments

Richard Biener April 16, 2014, 9:30 a.m. UTC | #1 
On Wed, Apr 16, 2014 at 9:46 AM, Joey Ye <joey.ye@arm.com> wrote:
>
>
>> -----Original Message-----
>> From: Joey Ye [mailto:joey.ye@arm.com]
>> Sent: Tuesday, April 15, 2014 6:37 PM
>> To: 'Richard Biener'
>> Cc: GCC Patches
>> Subject: RE: [patch] Disable if_conversion2 for Og
>>
>> > Ok for trunk and branches after a while.  Why does if-conversion not have
>> > the same problem?  On the GIMPLE part we avoid all kinds of if-conversion
>> > with -Og.
>> I think if-conversion should be disabled for Og too, but I don't have a case to
>> show that it is harmful. If GIMPLE avoids all if-conversion, it is nature to do
>> the same for RTL. I'll test and send another patch to also disable if-conversion.
> New patch tested with more regressions with -Og, which are expected.
> FAIL: gcc.target/arm/its.c scan-assembler-times \\tit 2
> FAIL: gcc.target/arm/pr40956.c scan-assembler-times mov[\\\\t ]*r., #0 1
> FAIL: gcc.target/arm/pr42835.c scan-assembler-times moveq[\\\\t ]*r.,[\\\\t ]*# 1
> FAIL: gcc.target/arm/pr42835.c scan-assembler-times movne[\\\\t ]*r.,[\\\\t ]*# 1
> FAIL: gcc.target/arm/shiftable.c scan-assembler sub.*[al]sl #6
> FAIL: gcc.target/arm/thumb-ifcvt-2.c scan-assembler asreq
> FAIL: gcc.target/arm/thumb-ifcvt-2.c scan-assembler lslne
> FAIL: gcc.target/arm/thumb-ifcvt.c scan-assembler asrne
> FAIL: gcc.target/arm/thumb-ifcvt.c scan-assembler lslne
> FAIL: gcc.target/arm/vseleqdf.c scan-assembler-times vseleq.f64\\td[0-9]+ 1
> FAIL: gcc.target/arm/vseleqsf.c scan-assembler-times vseleq.f32\\ts[0-9]+ 1
> FAIL: gcc.target/arm/vselgedf.c scan-assembler-times vselge.f64\\td[0-9]+ 1
> FAIL: gcc.target/arm/vselgesf.c scan-assembler-times vselge.f32\\ts[0-9]+ 1
> FAIL: gcc.target/arm/vselgtdf.c scan-assembler-times vselgt.f64\\td[0-9]+ 1
> FAIL: gcc.target/arm/vselgtsf.c scan-assembler-times vselgt.f32\\ts[0-9]+ 1
> FAIL: gcc.target/arm/vselledf.c scan-assembler-times vselgt.f64\\td[0-9]+ 1
> FAIL: gcc.target/arm/vsellesf.c scan-assembler-times vselgt.f32\\ts[0-9]+ 1
> FAIL: gcc.target/arm/vselltdf.c scan-assembler-times vselge.f64\\td[0-9]+ 1
> FAIL: gcc.target/arm/vselltsf.c scan-assembler-times vselge.f32\\ts[0-9]+ 1
> FAIL: gcc.target/arm/vselnedf.c scan-assembler-times vseleq.f64\\td[0-9]+ 1
> FAIL: gcc.target/arm/vselnesf.c scan-assembler-times vseleq.f32\\ts[0-9]+ 1
> FAIL: gcc.target/arm/vselvcdf.c scan-assembler-times vselvs.f64\\td[0-9]+ 1
> FAIL: gcc.target/arm/vselvcsf.c scan-assembler-times vselvs.f32\\ts[0-9]+ 1
> FAIL: gcc.target/arm/vselvsdf.c scan-assembler-times vselvs.f64\\td[0-9]+ 1
> FAIL: gcc.target/arm/vselvssf.c scan-assembler-times vselvs.f32\\ts[0-9]+ 1
>
> OK?

I suppose the tests above are usually not run with -Og thus the patch
won't regress regular testing.

Ok for trunk.

Thanks,
Richard.

> ChangeLog:
>     * opts.c (OPT_fif_conversion, OPT_fif_conversion2): Disable for Og.
>
> diff --git a/gcc/opts.c b/gcc/opts.c
> index fdc903f..3f3db1a 100644
> --- a/gcc/opts.c
> +++ b/gcc/opts.c
> @@ -431,8 +431,8 @@ static const struct default_options default_options_table[] =
>      { OPT_LEVELS_1_PLUS, OPT_fguess_branch_probability, NULL, 1 },
>      { OPT_LEVELS_1_PLUS, OPT_fcprop_registers, NULL, 1 },
>      { OPT_LEVELS_1_PLUS, OPT_fforward_propagate, NULL, 1 },
> -    { OPT_LEVELS_1_PLUS, OPT_fif_conversion, NULL, 1 },
> -    { OPT_LEVELS_1_PLUS, OPT_fif_conversion2, NULL, 1 },
> +    { OPT_LEVELS_1_PLUS_NOT_DEBUG, OPT_fif_conversion, NULL, 1 },
> +    { OPT_LEVELS_1_PLUS_NOT_DEBUG, OPT_fif_conversion2, NULL, 1 },
>      { OPT_LEVELS_1_PLUS, OPT_fipa_pure_const, NULL, 1 },
>      { OPT_LEVELS_1_PLUS, OPT_fipa_reference, NULL, 1 },
>      { OPT_LEVELS_1_PLUS, OPT_fipa_profile, NULL, 1 },
>
>
Richard Earnshaw April 16, 2014, 9:45 a.m. UTC | #2 
On 16/04/14 10:30, Richard Biener wrote:
> On Wed, Apr 16, 2014 at 9:46 AM, Joey Ye <joey.ye@arm.com> wrote:
>>
>>
>>> -----Original Message-----
>>> From: Joey Ye [mailto:joey.ye@arm.com]
>>> Sent: Tuesday, April 15, 2014 6:37 PM
>>> To: 'Richard Biener'
>>> Cc: GCC Patches
>>> Subject: RE: [patch] Disable if_conversion2 for Og
>>>
>>>> Ok for trunk and branches after a while.  Why does if-conversion not have
>>>> the same problem?  On the GIMPLE part we avoid all kinds of if-conversion
>>>> with -Og.
>>> I think if-conversion should be disabled for Og too, but I don't have a case to
>>> show that it is harmful. If GIMPLE avoids all if-conversion, it is nature to do
>>> the same for RTL. I'll test and send another patch to also disable if-conversion.
>> New patch tested with more regressions with -Og, which are expected.
>> FAIL: gcc.target/arm/its.c scan-assembler-times \\tit 2
>> FAIL: gcc.target/arm/pr40956.c scan-assembler-times mov[\\\\t ]*r., #0 1
>> FAIL: gcc.target/arm/pr42835.c scan-assembler-times moveq[\\\\t ]*r.,[\\\\t ]*# 1
>> FAIL: gcc.target/arm/pr42835.c scan-assembler-times movne[\\\\t ]*r.,[\\\\t ]*# 1
>> FAIL: gcc.target/arm/shiftable.c scan-assembler sub.*[al]sl #6
>> FAIL: gcc.target/arm/thumb-ifcvt-2.c scan-assembler asreq
>> FAIL: gcc.target/arm/thumb-ifcvt-2.c scan-assembler lslne
>> FAIL: gcc.target/arm/thumb-ifcvt.c scan-assembler asrne
>> FAIL: gcc.target/arm/thumb-ifcvt.c scan-assembler lslne
>> FAIL: gcc.target/arm/vseleqdf.c scan-assembler-times vseleq.f64\\td[0-9]+ 1
>> FAIL: gcc.target/arm/vseleqsf.c scan-assembler-times vseleq.f32\\ts[0-9]+ 1
>> FAIL: gcc.target/arm/vselgedf.c scan-assembler-times vselge.f64\\td[0-9]+ 1
>> FAIL: gcc.target/arm/vselgesf.c scan-assembler-times vselge.f32\\ts[0-9]+ 1
>> FAIL: gcc.target/arm/vselgtdf.c scan-assembler-times vselgt.f64\\td[0-9]+ 1
>> FAIL: gcc.target/arm/vselgtsf.c scan-assembler-times vselgt.f32\\ts[0-9]+ 1
>> FAIL: gcc.target/arm/vselledf.c scan-assembler-times vselgt.f64\\td[0-9]+ 1
>> FAIL: gcc.target/arm/vsellesf.c scan-assembler-times vselgt.f32\\ts[0-9]+ 1
>> FAIL: gcc.target/arm/vselltdf.c scan-assembler-times vselge.f64\\td[0-9]+ 1
>> FAIL: gcc.target/arm/vselltsf.c scan-assembler-times vselge.f32\\ts[0-9]+ 1
>> FAIL: gcc.target/arm/vselnedf.c scan-assembler-times vseleq.f64\\td[0-9]+ 1
>> FAIL: gcc.target/arm/vselnesf.c scan-assembler-times vseleq.f32\\ts[0-9]+ 1
>> FAIL: gcc.target/arm/vselvcdf.c scan-assembler-times vselvs.f64\\td[0-9]+ 1
>> FAIL: gcc.target/arm/vselvcsf.c scan-assembler-times vselvs.f32\\ts[0-9]+ 1
>> FAIL: gcc.target/arm/vselvsdf.c scan-assembler-times vselvs.f64\\td[0-9]+ 1
>> FAIL: gcc.target/arm/vselvssf.c scan-assembler-times vselvs.f32\\ts[0-9]+ 1
>>
>> OK?
> 
> I suppose the tests above are usually not run with -Og thus the patch
> won't regress regular testing.
> 
> Ok for trunk.

I'd like to see these tests fixed to ensure that they're skipped if this
goes in.

R.

> 
> Thanks,
> Richard.
> 
>> ChangeLog:
>>     * opts.c (OPT_fif_conversion, OPT_fif_conversion2): Disable for Og.
>>
>> diff --git a/gcc/opts.c b/gcc/opts.c
>> index fdc903f..3f3db1a 100644
>> --- a/gcc/opts.c
>> +++ b/gcc/opts.c
>> @@ -431,8 +431,8 @@ static const struct default_options default_options_table[] =
>>      { OPT_LEVELS_1_PLUS, OPT_fguess_branch_probability, NULL, 1 },
>>      { OPT_LEVELS_1_PLUS, OPT_fcprop_registers, NULL, 1 },
>>      { OPT_LEVELS_1_PLUS, OPT_fforward_propagate, NULL, 1 },
>> -    { OPT_LEVELS_1_PLUS, OPT_fif_conversion, NULL, 1 },
>> -    { OPT_LEVELS_1_PLUS, OPT_fif_conversion2, NULL, 1 },
>> +    { OPT_LEVELS_1_PLUS_NOT_DEBUG, OPT_fif_conversion, NULL, 1 },
>> +    { OPT_LEVELS_1_PLUS_NOT_DEBUG, OPT_fif_conversion2, NULL, 1 },
>>      { OPT_LEVELS_1_PLUS, OPT_fipa_pure_const, NULL, 1 },
>>      { OPT_LEVELS_1_PLUS, OPT_fipa_reference, NULL, 1 },
>>      { OPT_LEVELS_1_PLUS, OPT_fipa_profile, NULL, 1 },
>>
>>
>
diff mbox

Patch

diff --git a/gcc/opts.c b/gcc/opts.c
index fdc903f..3f3db1a 100644
--- a/gcc/opts.c
+++ b/gcc/opts.c
@@ -431,8 +431,8 @@  static const struct default_options default_options_table[] =
     { OPT_LEVELS_1_PLUS, OPT_fguess_branch_probability, NULL, 1 },
     { OPT_LEVELS_1_PLUS, OPT_fcprop_registers, NULL, 1 },
     { OPT_LEVELS_1_PLUS, OPT_fforward_propagate, NULL, 1 },
-    { OPT_LEVELS_1_PLUS, OPT_fif_conversion, NULL, 1 },
-    { OPT_LEVELS_1_PLUS, OPT_fif_conversion2, NULL, 1 },
+    { OPT_LEVELS_1_PLUS_NOT_DEBUG, OPT_fif_conversion, NULL, 1 },
+    { OPT_LEVELS_1_PLUS_NOT_DEBUG, OPT_fif_conversion2, NULL, 1 },
     { OPT_LEVELS_1_PLUS, OPT_fipa_pure_const, NULL, 1 },
     { OPT_LEVELS_1_PLUS, OPT_fipa_reference, NULL, 1 },
     { OPT_LEVELS_1_PLUS, OPT_fipa_profile, NULL, 1 },