Patchwork PING: [PATCH]: Fix -fbranch-probabilities

login
register
mail settings
Submitter Christian Bruel
Date Aug. 29, 2011, 11:59 a.m.
Message ID <4E5B7F1E.9020808@st.com>
Download mbox | patch
Permalink /patch/112028/
State New
Headers show

Comments

Christian Bruel - Aug. 29, 2011, 11:59 a.m.
On 08/27/2011 02:04 AM, Jan Hubicka wrote:
>> Hello,
>>
>> Could I have a review for the trivial patch posted in
>> http://gcc.gnu.org/ml/gcc-patches/2011-08/msg01123.html
>>
>> -fprofile-use sets flag_branch_probabilities.
>>
>> But we should also be able to use -fbranch-probabilities on its own
>> using the information generated by -fprofile-arcs, as documented.
>
> OK, thanks!  I was under impression that some of gcov tests still use
> -fprofile-arcs -fbranch-probabilities pair.

yes, indeed, this is was the documentation claims:
http://gcc.gnu.org/onlinedocs/gccint/C-Tests.html#C-Tests.

e.g for :

gcc.misc-tests
     ...
     bprob*.c
         Test -fbranch-probabilities using gcc.misc-tests/bprob.exp,
     ...

but bprob.exp sets feedback_options to "-fprofile-use"

 > It don't seem to be the
 > case, so if you add a testcase, you get extra score ;)

I feel more like fixing the bprob.exp discrepancy to have the correct 
pairing with the following. This will act as the testcase, since those 
tests fail without the patch.

OK ?


      foreach src [lsort [glob -nocomplain $srcdir/$subdir/bprob-*.c]] {

---------
2011-08-29  Christian Bruel  <christian.bruel@st.com>

	* gcc.misc-tests/bprob.exp (feedback_options): Set -fbranch-probabilities.
---------

Thanks

Christian


>
> Honza
>>
>> Many thanks
>>
>> Christian
Richard Guenther - Aug. 29, 2011, 12:10 p.m.
On Mon, Aug 29, 2011 at 1:59 PM, Christian Bruel <christian.bruel@st.com> wrote:
>
>
> On 08/27/2011 02:04 AM, Jan Hubicka wrote:
>>>
>>> Hello,
>>>
>>> Could I have a review for the trivial patch posted in
>>> http://gcc.gnu.org/ml/gcc-patches/2011-08/msg01123.html
>>>
>>> -fprofile-use sets flag_branch_probabilities.
>>>
>>> But we should also be able to use -fbranch-probabilities on its own
>>> using the information generated by -fprofile-arcs, as documented.
>>
>> OK, thanks!  I was under impression that some of gcov tests still use
>> -fprofile-arcs -fbranch-probabilities pair.
>
> yes, indeed, this is was the documentation claims:
> http://gcc.gnu.org/onlinedocs/gccint/C-Tests.html#C-Tests.
>
> e.g for :
>
> gcc.misc-tests
>    ...
>    bprob*.c
>        Test -fbranch-probabilities using gcc.misc-tests/bprob.exp,
>    ...
>
> but bprob.exp sets feedback_options to "-fprofile-use"
>
>> It don't seem to be the
>> case, so if you add a testcase, you get extra score ;)
>
> I feel more like fixing the bprob.exp discrepancy to have the correct
> pairing with the following. This will act as the testcase, since those tests
> fail without the patch.
>
> OK ?

If it works, yes.

Richard.

>
> Index: gcc.misc-tests/bprob.exp
> ===================================================================
> --- gcc.misc-tests/bprob.exp    (revision 178096)
> +++ gcc.misc-tests/bprob.exp    (working copy)
> @@ -48,7 +48,7 @@
>  load_lib profopt.exp
>
>  set profile_options "-fprofile-arcs"
> -set feedback_options "-fprofile-use"
> +set feedback_options "-fbranch-probabilities"
>
>  foreach profile_option $profile_options feedback_option $feedback_options {
>     foreach src [lsort [glob -nocomplain $srcdir/$subdir/bprob-*.c]] {
>
> ---------
> 2011-08-29  Christian Bruel  <christian.bruel@st.com>
>
>        * gcc.misc-tests/bprob.exp (feedback_options): Set
> -fbranch-probabilities.
> ---------
>
> Thanks
>
> Christian
>
>
>>
>> Honza
>>>
>>> Many thanks
>>>
>>> Christian
>

Patch

Index: gcc.misc-tests/bprob.exp
===================================================================
--- gcc.misc-tests/bprob.exp    (revision 178096)
+++ gcc.misc-tests/bprob.exp    (working copy)
@@ -48,7 +48,7 @@ 
  load_lib profopt.exp

  set profile_options "-fprofile-arcs"
-set feedback_options "-fprofile-use"
+set feedback_options "-fbranch-probabilities"

  foreach profile_option $profile_options feedback_option 
$feedback_options {