Re: [PATCH GCC]Remove support for -funsafe-loop-optimizations
On 07/15/2016 07:07 PM, Bin Cheng wrote: Bootstrap and test on x86_64. Is it OK? If you do this you'll also need to remove the use in config/bfin. Bernd
Re: [PATCH GCC]Remove support for -funsafe-loop-optimizations
On July 15, 2016 7:16:42 PM GMT+02:00, Bernd Schmidt wrote: >On 07/15/2016 07:07 PM, Bin Cheng wrote: > >> Bootstrap and test on x86_64. Is it OK? > >If you do this you'll also need to remove the use in config/bfin. OK with that change. Richard. > >Bernd
Re: [PATCH GCC]Remove support for -funsafe-loop-optimizations
On Fri, Jul 15, 2016 at 1:07 PM, Bin Cheng wrote: > Hi, > This patch removes support for -funsafe-loop-optimizations, as well as > -Wunsafe-loop-optimizations. By its name, this option does unsafe > optimizations by assuming all loops must terminate and doesn't wrap. > Unfortunately, it's not as useful as expected because: > 1) Simply assuming loop must terminate isn't enough. What we really want is > to analyze scalar evolution and loop niter bound under such assumptions. > This option does nothing in this aspect. > 2) IIRC, this option generates bogus code for some common programs, that's > why it's disabled by default even at Ofast level. > > After I sent patches handling possible infinite loops in both (scev/niter) > analyzer and vectorizer, it's a natural step to remove such options in GCC. > This patch does so by deleting code for -funsafe-loop-optimizations, as well > as -Wunsafe-loop-optimizations. It also deletes the two now useless tests, > while the option interface is preserved for backward compatibility purpose. There are a number of bugs opened against those options, including one that I just opened rather recently: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71769 but some go back far, in this case 9 years: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=34114 If you are going to remove the options, you should address open bugs related to those options.
Re: [PATCH GCC]Remove support for -funsafe-loop-optimizations
On Sat, Jul 16, 2016 at 6:28 PM, NightStrike wrote: > On Fri, Jul 15, 2016 at 1:07 PM, Bin Cheng wrote: >> Hi, >> This patch removes support for -funsafe-loop-optimizations, as well as >> -Wunsafe-loop-optimizations. By its name, this option does unsafe >> optimizations by assuming all loops must terminate and doesn't wrap. >> Unfortunately, it's not as useful as expected because: >> 1) Simply assuming loop must terminate isn't enough. What we really want is >> to analyze scalar evolution and loop niter bound under such assumptions. >> This option does nothing in this aspect. >> 2) IIRC, this option generates bogus code for some common programs, that's >> why it's disabled by default even at Ofast level. >> >> After I sent patches handling possible infinite loops in both (scev/niter) >> analyzer and vectorizer, it's a natural step to remove such options in GCC. >> This patch does so by deleting code for -funsafe-loop-optimizations, as well >> as -Wunsafe-loop-optimizations. It also deletes the two now useless tests, >> while the option interface is preserved for backward compatibility purpose. > > There are a number of bugs opened against those options, including one > that I just opened rather recently: > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71769 > > but some go back far, in this case 9 years: > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=34114 > > If you are going to remove the options, you should address open bugs > related to those options. Hi, Thanks for pointing me to these PRs, I will have a look at them. IMHO, the old one reports weakness in loop niter analyzer, the issue exists whether I remove unsafe-loop-optimization or not. The new one is a little bit trickier, I will put some comments on PR, and again, the issue (if it is) is in niter analyzer which has nothing to do with the option really. Thanks, bin
Re: [PATCH GCC]Remove support for -funsafe-loop-optimizations
On Fri, Jul 15, 2016 at 6:23 PM, Richard Biener wrote: > On July 15, 2016 7:16:42 PM GMT+02:00, Bernd Schmidt > wrote: >>On 07/15/2016 07:07 PM, Bin Cheng wrote: >> >>> Bootstrap and test on x86_64. Is it OK? >> >>If you do this you'll also need to remove the use in config/bfin. > > OK with that change. Hi all, Attachment is the updated patch, given it's pre-approved with this change, I will commit it later. Thanks, bin > > Richard. > >> >>Bernd > > diff --git a/gcc/common.opt b/gcc/common.opt index a7c5125..331e1da 100644 --- a/gcc/common.opt +++ b/gcc/common.opt @@ -625,8 +625,8 @@ Common Var(warn_null_dereference) Warning Warn if dereferencing a NULL pointer may lead to erroneous or undefined behavior. Wunsafe-loop-optimizations -Common Var(warn_unsafe_loop_optimizations) Warning -Warn if the loop cannot be optimized due to nontrivial assumptions. +Common Ignore +Does nothing. Preserved for backward compatibility. Wmissing-noreturn Common Warning Alias(Wsuggest-attribute=noreturn) @@ -2500,8 +2500,8 @@ Perform loop unrolling for all loops. ; that control loops do not overflow and that the loops with nontrivial ; exit condition are not infinite funsafe-loop-optimizations -Common Report Var(flag_unsafe_loop_optimizations) Optimization -Allow loop optimizations to assume that the loops behave in normal way. +Common Ignore +Does nothing. Preserved for backward compatibility. fassociative-math Common Report Var(flag_associative_math) SetByCombined Optimization diff --git a/gcc/config/bfin/bfin.c b/gcc/config/bfin/bfin.c index 75ddcf0..b6edf2c 100644 --- a/gcc/config/bfin/bfin.c +++ b/gcc/config/bfin/bfin.c @@ -3375,10 +3375,7 @@ bfin_can_use_doloop_p (const widest_int &, const widest_int &iterations_max, /* Due to limitations in the hardware (an initial loop count of 0 does not loop 2^32 times) we must avoid to generate a hardware loops when we cannot rule out this case. */ - if (!flag_unsafe_loop_optimizations - && wi::geu_p (iterations_max, 0x)) -return false; - return true; + return (wi::ltu_p (iterations_max, 0x)); } /* Increment the counter for the number of loop instructions in the diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi index ca8c1b4..4241956 100644 --- a/gcc/doc/invoke.texi +++ b/gcc/doc/invoke.texi @@ -302,8 +302,7 @@ Objective-C and Objective-C++ Dialects}. -Wswitch -Wswitch-bool -Wswitch-default -Wswitch-enum @gol -Wswitch-unreachable -Wsync-nand @gol -Wsystem-headers -Wtautological-compare -Wtrampolines -Wtrigraphs @gol --Wtype-limits -Wundef @gol --Wuninitialized -Wunknown-pragmas -Wunsafe-loop-optimizations @gol +-Wtype-limits -Wundef -Wuninitialized -Wunknown-pragmas @gol -Wunsuffixed-float-constants -Wunused -Wunused-function @gol -Wunused-label -Wunused-local-typedefs -Wunused-parameter @gol -Wno-unused-result -Wunused-value @gol -Wunused-variable @gol @@ -414,7 +413,7 @@ Objective-C and Objective-C++ Dialects}. -ftree-switch-conversion -ftree-tail-merge -ftree-ter @gol -ftree-vectorize -ftree-vrp -funconstrained-commons @gol -funit-at-a-time -funroll-all-loops -funroll-loops @gol --funsafe-loop-optimizations -funsafe-math-optimizations -funswitch-loops @gol +-funsafe-math-optimizations -funswitch-loops @gol -fipa-ra -fvariable-expansion-in-unroller -fvect-cost-model -fvpt @gol -fweb -fwhole-program -fwpa -fuse-linker-plugin @gol --param @var{name}=@var{value} @@ -4986,14 +4985,6 @@ If the stack usage is (partly) dynamic and not bounded, it's: @end smallexample @end itemize -@item -Wunsafe-loop-optimizations -@opindex Wunsafe-loop-optimizations -@opindex Wno-unsafe-loop-optimizations -Warn if the loop cannot be optimized because the compiler cannot -assume anything on the bounds of the loop indices. With -@option{-funsafe-loop-optimizations} warn if the compiler makes -such assumptions. - @item -Wno-pedantic-ms-format @r{(MinGW targets only)} @opindex Wno-pedantic-ms-format @opindex Wpedantic-ms-format @@ -6817,15 +6808,6 @@ number of iterations of a loop are used to guide loop unrolling and peeling and loop exit test optimizations. This option is enabled by default. -@item -funsafe-loop-optimizations -@opindex funsafe-loop-optimizations -This option tells the loop optimizer to assume that loop indices do not -overflow, and that loops with nontrivial exit condition are not -infinite. This enables a wider range of loop optimizations even if -the loop optimizer itself cannot prove that these assumptions are valid. -If you use @option{-Wunsafe-loop-optimizations}, the compiler warns you -if it finds this kind of loop. - @item -funconstrained-commons @opindex funconstrained-commons This option tells the compiler that variables declared in common blocks diff --git a/gcc/ipa-inline-analysis.c b/gcc/ipa-inline-analysis.c index da29d22..6b2cf96 100644 --- a/gcc/ipa-inline-analysis.c +++ b/gcc/ipa-inline-analysis.c @@ -2820,7 +2820,7 @@ estimate_function_body_sizes (stru
Re: [PATCH GCC]Remove support for -funsafe-loop-optimizations
On Mon, Jul 18, 2016 at 3:55 AM, Bin.Cheng wrote: > On Sat, Jul 16, 2016 at 6:28 PM, NightStrike wrote: >> On Fri, Jul 15, 2016 at 1:07 PM, Bin Cheng wrote: >>> Hi, >>> This patch removes support for -funsafe-loop-optimizations, as well as >>> -Wunsafe-loop-optimizations. By its name, this option does unsafe >>> optimizations by assuming all loops must terminate and doesn't wrap. >>> Unfortunately, it's not as useful as expected because: >>> 1) Simply assuming loop must terminate isn't enough. What we really want >>> is to analyze scalar evolution and loop niter bound under such assumptions. >>> This option does nothing in this aspect. >>> 2) IIRC, this option generates bogus code for some common programs, that's >>> why it's disabled by default even at Ofast level. >>> >>> After I sent patches handling possible infinite loops in both (scev/niter) >>> analyzer and vectorizer, it's a natural step to remove such options in GCC. >>> This patch does so by deleting code for -funsafe-loop-optimizations, as >>> well as -Wunsafe-loop-optimizations. It also deletes the two now useless >>> tests, while the option interface is preserved for backward compatibility >>> purpose. >> >> There are a number of bugs opened against those options, including one >> that I just opened rather recently: >> >> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71769 >> >> but some go back far, in this case 9 years: >> >> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=34114 >> >> If you are going to remove the options, you should address open bugs >> related to those options. > Hi, > Thanks for pointing me to these PRs, I will have a look at them. I only highlighted two PRs, I was suggesting that you look for all of them. > IMHO, the old one reports weakness in loop niter analyzer, the issue > exists whether I remove unsafe-loop-optimization or not. The new one > is a little bit trickier, I will put some comments on PR, and again, > the issue (if it is) is in niter analyzer which has nothing to do with > the option really. Well, one thing to note is that the warning is an easy way to get a notice of a possible missed optimization (and I have many more occurrences of it in a particular code base that I use). If the warning is highlighted potential issues that aren't due to the -f option but are issues nonetheless, and we remove the warning, then how should I go about finding these missed opportunities in the future? Is there a different mechanism that does the same thing?
Re: [PATCH GCC]Remove support for -funsafe-loop-optimizations
On Mon, Jul 18, 2016 at 4:28 PM, NightStrike wrote: > On Mon, Jul 18, 2016 at 3:55 AM, Bin.Cheng wrote: >> On Sat, Jul 16, 2016 at 6:28 PM, NightStrike wrote: >>> On Fri, Jul 15, 2016 at 1:07 PM, Bin Cheng wrote: Hi, This patch removes support for -funsafe-loop-optimizations, as well as -Wunsafe-loop-optimizations. By its name, this option does unsafe optimizations by assuming all loops must terminate and doesn't wrap. Unfortunately, it's not as useful as expected because: 1) Simply assuming loop must terminate isn't enough. What we really want is to analyze scalar evolution and loop niter bound under such assumptions. This option does nothing in this aspect. 2) IIRC, this option generates bogus code for some common programs, that's why it's disabled by default even at Ofast level. After I sent patches handling possible infinite loops in both (scev/niter) analyzer and vectorizer, it's a natural step to remove such options in GCC. This patch does so by deleting code for -funsafe-loop-optimizations, as well as -Wunsafe-loop-optimizations. It also deletes the two now useless tests, while the option interface is preserved for backward compatibility purpose. >>> >>> There are a number of bugs opened against those options, including one >>> that I just opened rather recently: >>> >>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71769 >>> >>> but some go back far, in this case 9 years: >>> >>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=34114 >>> >>> If you are going to remove the options, you should address open bugs >>> related to those options. >> Hi, >> Thanks for pointing me to these PRs, I will have a look at them. > > I only highlighted two PRs, I was suggesting that you look for all of them. > >> IMHO, the old one reports weakness in loop niter analyzer, the issue >> exists whether I remove unsafe-loop-optimization or not. The new one >> is a little bit trickier, I will put some comments on PR, and again, >> the issue (if it is) is in niter analyzer which has nothing to do with >> the option really. > > Well, one thing to note is that the warning is an easy way to get a > notice of a possible missed optimization (and I have many more > occurrences of it in a particular code base that I use). If the > warning is highlighted potential issues that aren't due to the -f > option but are issues nonetheless, and we remove the warning, then how > should I go about finding these missed opportunities in the future? > Is there a different mechanism that does the same thing? Hmm, good point, I will iterate the patch to see if I can only remove -funsafe-loop-optimizations, while keep -Wunsafe-loop-optimizations. Thanks, bin
Re: [PATCH GCC]Remove support for -funsafe-loop-optimizations
On Mon, Jul 18, 2016 at 5:36 PM, Bin.Cheng wrote: > On Mon, Jul 18, 2016 at 4:28 PM, NightStrike wrote: >> On Mon, Jul 18, 2016 at 3:55 AM, Bin.Cheng wrote: >>> On Sat, Jul 16, 2016 at 6:28 PM, NightStrike wrote: On Fri, Jul 15, 2016 at 1:07 PM, Bin Cheng wrote: > Hi, > This patch removes support for -funsafe-loop-optimizations, as well as > -Wunsafe-loop-optimizations. By its name, this option does unsafe > optimizations by assuming all loops must terminate and doesn't wrap. > Unfortunately, it's not as useful as expected because: > 1) Simply assuming loop must terminate isn't enough. What we really want > is to analyze scalar evolution and loop niter bound under such > assumptions. This option does nothing in this aspect. > 2) IIRC, this option generates bogus code for some common programs, > that's why it's disabled by default even at Ofast level. > > After I sent patches handling possible infinite loops in both > (scev/niter) analyzer and vectorizer, it's a natural step to remove such > options in GCC. This patch does so by deleting code for > -funsafe-loop-optimizations, as well as -Wunsafe-loop-optimizations. It > also deletes the two now useless tests, while the option interface is > preserved for backward compatibility purpose. There are a number of bugs opened against those options, including one that I just opened rather recently: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71769 but some go back far, in this case 9 years: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=34114 If you are going to remove the options, you should address open bugs related to those options. >>> Hi, >>> Thanks for pointing me to these PRs, I will have a look at them. >> >> I only highlighted two PRs, I was suggesting that you look for all of them. >> >>> IMHO, the old one reports weakness in loop niter analyzer, the issue >>> exists whether I remove unsafe-loop-optimization or not. The new one >>> is a little bit trickier, I will put some comments on PR, and again, >>> the issue (if it is) is in niter analyzer which has nothing to do with >>> the option really. >> >> Well, one thing to note is that the warning is an easy way to get a >> notice of a possible missed optimization (and I have many more >> occurrences of it in a particular code base that I use). If the >> warning is highlighted potential issues that aren't due to the -f >> option but are issues nonetheless, and we remove the warning, then how >> should I go about finding these missed opportunities in the future? >> Is there a different mechanism that does the same thing? > Hmm, good point, I will iterate the patch to see if I can only remove > -funsafe-loop-optimizations, while keep -Wunsafe-loop-optimizations. Of course the naming of -Wunsafe-loop-optimizations is misleading then. Maybe provide an alias -Wmissed-loop-optimizations and re-word it to say "disable _some_ loop optimizations" as I hope more loop optimizations get aware of "assumptions" and deal with them. Richard. > Thanks, > bin
Re: [PATCH GCC]Remove support for -funsafe-loop-optimizations
On Tue, Jul 19, 2016 at 10:00 AM, Richard Biener wrote: > On Mon, Jul 18, 2016 at 5:36 PM, Bin.Cheng wrote: >> On Mon, Jul 18, 2016 at 4:28 PM, NightStrike wrote: >>> On Mon, Jul 18, 2016 at 3:55 AM, Bin.Cheng wrote: On Sat, Jul 16, 2016 at 6:28 PM, NightStrike wrote: > On Fri, Jul 15, 2016 at 1:07 PM, Bin Cheng wrote: >> Hi, >> This patch removes support for -funsafe-loop-optimizations, as well as >> -Wunsafe-loop-optimizations. By its name, this option does unsafe >> optimizations by assuming all loops must terminate and doesn't wrap. >> Unfortunately, it's not as useful as expected because: >> 1) Simply assuming loop must terminate isn't enough. What we really >> want is to analyze scalar evolution and loop niter bound under such >> assumptions. This option does nothing in this aspect. >> 2) IIRC, this option generates bogus code for some common programs, >> that's why it's disabled by default even at Ofast level. >> >> After I sent patches handling possible infinite loops in both >> (scev/niter) analyzer and vectorizer, it's a natural step to remove such >> options in GCC. This patch does so by deleting code for >> -funsafe-loop-optimizations, as well as -Wunsafe-loop-optimizations. It >> also deletes the two now useless tests, while the option interface is >> preserved for backward compatibility purpose. > > There are a number of bugs opened against those options, including one > that I just opened rather recently: > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71769 > > but some go back far, in this case 9 years: > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=34114 > > If you are going to remove the options, you should address open bugs > related to those options. Hi, Thanks for pointing me to these PRs, I will have a look at them. >>> >>> I only highlighted two PRs, I was suggesting that you look for all of them. >>> IMHO, the old one reports weakness in loop niter analyzer, the issue exists whether I remove unsafe-loop-optimization or not. The new one is a little bit trickier, I will put some comments on PR, and again, the issue (if it is) is in niter analyzer which has nothing to do with the option really. >>> >>> Well, one thing to note is that the warning is an easy way to get a >>> notice of a possible missed optimization (and I have many more >>> occurrences of it in a particular code base that I use). If the >>> warning is highlighted potential issues that aren't due to the -f >>> option but are issues nonetheless, and we remove the warning, then how >>> should I go about finding these missed opportunities in the future? >>> Is there a different mechanism that does the same thing? >> Hmm, good point, I will iterate the patch to see if I can only remove >> -funsafe-loop-optimizations, while keep -Wunsafe-loop-optimizations. > > Of course the naming of -Wunsafe-loop-optimizations is misleading then. > Maybe provide an alias -Wmissed-loop-optimizations and re-word it to > say "disable _some_ loop optimizations" as I hope more loop optimizations > get aware of "assumptions" and deal with them. In which case a way to "re-introduce" -funsafe-loop-optimizations would be to add a #pragma that can be used to annotate loops to tell GCC of various properties like that it terminates without IV wrapping. Richard. > Richard. > >> Thanks, >> bin
Re: [PATCH GCC]Remove support for -funsafe-loop-optimizations
Hi, On Mon, Jul 18, 2016 at 11:28:48AM -0400, NightStrike wrote: > Well, one thing to note is that the warning is an easy way to get a > notice of a possible missed optimization (and I have many more > occurrences of it in a particular code base that I use). If the > warning is highlighted potential issues that aren't due to the -f > option but are issues nonetheless, and we remove the warning, then how > should I go about finding these missed opportunities in the future? > Is there a different mechanism that does the same thing? Yes, -fopt-info and -fopt-info-OPTIONS switches. It certainly seems to be a more natural means for manual compiler-guided optimization than warnings. Martin
Re: [PATCH GCC]Remove support for -funsafe-loop-optimizations
On Tue, Jul 19, 2016 at 9:00 AM, Richard Biener wrote: > On Mon, Jul 18, 2016 at 5:36 PM, Bin.Cheng wrote: >> On Mon, Jul 18, 2016 at 4:28 PM, NightStrike wrote: >>> On Mon, Jul 18, 2016 at 3:55 AM, Bin.Cheng wrote: On Sat, Jul 16, 2016 at 6:28 PM, NightStrike wrote: > On Fri, Jul 15, 2016 at 1:07 PM, Bin Cheng wrote: >> Hi, >> This patch removes support for -funsafe-loop-optimizations, as well as >> -Wunsafe-loop-optimizations. By its name, this option does unsafe >> optimizations by assuming all loops must terminate and doesn't wrap. >> Unfortunately, it's not as useful as expected because: >> 1) Simply assuming loop must terminate isn't enough. What we really >> want is to analyze scalar evolution and loop niter bound under such >> assumptions. This option does nothing in this aspect. >> 2) IIRC, this option generates bogus code for some common programs, >> that's why it's disabled by default even at Ofast level. >> >> After I sent patches handling possible infinite loops in both >> (scev/niter) analyzer and vectorizer, it's a natural step to remove such >> options in GCC. This patch does so by deleting code for >> -funsafe-loop-optimizations, as well as -Wunsafe-loop-optimizations. It >> also deletes the two now useless tests, while the option interface is >> preserved for backward compatibility purpose. > > There are a number of bugs opened against those options, including one > that I just opened rather recently: > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71769 > > but some go back far, in this case 9 years: > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=34114 > > If you are going to remove the options, you should address open bugs > related to those options. Hi, Thanks for pointing me to these PRs, I will have a look at them. >>> >>> I only highlighted two PRs, I was suggesting that you look for all of them. >>> IMHO, the old one reports weakness in loop niter analyzer, the issue exists whether I remove unsafe-loop-optimization or not. The new one is a little bit trickier, I will put some comments on PR, and again, the issue (if it is) is in niter analyzer which has nothing to do with the option really. >>> >>> Well, one thing to note is that the warning is an easy way to get a >>> notice of a possible missed optimization (and I have many more >>> occurrences of it in a particular code base that I use). If the >>> warning is highlighted potential issues that aren't due to the -f >>> option but are issues nonetheless, and we remove the warning, then how >>> should I go about finding these missed opportunities in the future? >>> Is there a different mechanism that does the same thing? >> Hmm, good point, I will iterate the patch to see if I can only remove >> -funsafe-loop-optimizations, while keep -Wunsafe-loop-optimizations. > > Of course the naming of -Wunsafe-loop-optimizations is misleading then. > Maybe provide an alias -Wmissed-loop-optimizations and re-word it to > say "disable _some_ loop optimizations" as I hope more loop optimizations The current behavior is to only warn possible missed loop optimization in IVOPT, which effectively is the last niter related loop optimization. I would rather to keep this behavior because warning against specific optimization would be a real hassle. This leads to another problem about precise warning message: If a loop optimization has already handled loop with assumptions, we should not warn against the loop afterwards. This again reminds me of the patch adding flag/constraint (or whatever the name finally made) extension. We can annotate loop structure once it's handled, so that warning message won't be issued. How about this? > get aware of "assumptions" and deal with them. The first question needs to be answered is how we export assumptions to various loop optimizers. For now, I only added one specific interface number_of_iteration_exit_assumptions and this will only be used in vectorizer in my following patches. A generic method exporting assumptions in loop structure would be great, but that's difficult because sometime we not only need assumptions itself, but also need to analyzer scev under assumptions. This is another problem though. > In which case a way to "re-introduce" -funsafe-loop-optimizations would be to > add a #pragma that can be used to annotate loops to tell GCC of various > properties like that it terminates without IV wrapping. Yeah, this could be easily done using the flag/constraint method. Thanks, bin > > Richard. > >> Thanks, >> bin