[PATCH] Change PRED_LOOP_EXIT from 85 to 92

[PATCH] Change PRED_LOOP_EXIT from 85 to 92

[David Esparza]

With a value of 85 GCC has a CPU performance degradation of 11%,
reverting PRED_LOOP_EXIT to 92 this degradation disappear.
Those values where measured by running c-ray ray-tracer that is a
floating point benchmark that runs out of L1 cache.

Signed-off-by: David Esparza <david.esparza.borquez@intel.com>
---
 contrib/analyze_brprob.py        | 4 ++--
 gcc/ChangeLog                    | 4 ----
 gcc/predict.def                  | 2 +-
 gcc/testsuite/gcc.dg/predict-9.c | 4 ++--
 4 files changed, 5 insertions(+), 9 deletions(-)

diff --git a/contrib/analyze_brprob.py b/contrib/analyze_brprob.py
index 2526623ff55..9808c46de16 100755
--- a/contrib/analyze_brprob.py
+++ b/contrib/analyze_brprob.py
@@ -119,10 +119,10 @@ class Profile:
         elif sorting == 'coverage':
             sorter = lambda x: x[1].count
 
-        print('%-40s %8s %6s  %-16s %14s %8s %6s' % ('HEURISTICS', 'BRANCHES', '(REL)',
+        print('%-36s %8s %6s  %-16s %14s %8s %6s' % ('HEURISTICS', 'BRANCHES', '(REL)',
               'HITRATE', 'COVERAGE', 'COVERAGE', '(REL)'))
         for (k, v) in sorted(self.heuristics.items(), key = sorter):
-            print('%-40s %8i %5.1f%% %6.2f%% / %6.2f%% %14i %8s %5.1f%%' %
+            print('%-36s %8i %5.1f%% %6.2f%% / %6.2f%% %14i %8s %5.1f%%' %
             (k, v.branches, percentage(v.branches, self.branches_max ()),
              percentage(v.hits, v.count), percentage(v.fits, v.count),
              v.count, v.count_formatted(), percentage(v.count, self.count_max()) ))
diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index a93464d33a7..0f4846c2c4d 100644
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,7 +1,3 @@
-2016-06-17  Martin Liska  <mliska@suse.cz>
-
-	* predict.def: PRED_LOOP_EXIT from 92 to 85.
-
 2016-06-17  James Greenhalgh  <james.greenhalgh@arm.com>
 
 	* config/arm/arm_neon.h (vadd_f32): replace __FAST_MATH with
diff --git a/gcc/predict.def b/gcc/predict.def
index d3bc757bb97..a0d0ba923a2 100644
--- a/gcc/predict.def
+++ b/gcc/predict.def
@@ -89,7 +89,7 @@ DEF_PREDICTOR (PRED_COLD_FUNCTION, "cold function call", PROB_VERY_LIKELY,
 	       PRED_FLAG_FIRST_MATCH)
 
 /* Edge causing loop to terminate is probably not taken.  */
-DEF_PREDICTOR (PRED_LOOP_EXIT, "loop exit", HITRATE (85),
+DEF_PREDICTOR (PRED_LOOP_EXIT, "loop exit", HITRATE (92),
 	       PRED_FLAG_FIRST_MATCH)
 
 /* Edge causing loop to terminate by computing value used by later
diff --git a/gcc/testsuite/gcc.dg/predict-9.c b/gcc/testsuite/gcc.dg/predict-9.c
index 196e31c60ee..a613961091d 100644
--- a/gcc/testsuite/gcc.dg/predict-9.c
+++ b/gcc/testsuite/gcc.dg/predict-9.c
@@ -19,5 +19,5 @@ void foo (int base)
   }
 }
 
-/* { dg-final { scan-tree-dump-times "first match heuristics: 3.0%" 3 "profile_estimate"} } */
-/* { dg-final { scan-tree-dump-times "first match heuristics: 7.5%" 1 "profile_estimate"} } */
+/* { dg-final { scan-tree-dump-times "first match heuristics: 2.0%" 3 "profile_estimate"} } */
+/* { dg-final { scan-tree-dump-times "first match heuristics: 4.0%" 1 "profile_estimate"} } */
-- 
2.15.1

Re: [PATCH] Change PRED_LOOP_EXIT from 85 to 92

[Segher Boessenkool]

On Fri, Dec 22, 2017 at 04:53:47PM -0600, David Esparza wrote:
> With a value of 85 GCC has a CPU performance degradation of 11%,
> reverting PRED_LOOP_EXIT to 92 this degradation disappear.
> Those values where measured by running c-ray ray-tracer that is a
> floating point benchmark that runs out of L1 cache.

Why is this single benchmark more important than everything else?

https://patchwork.ozlabs.org/patch/637073/


Segher

Re: [PATCH] Change PRED_LOOP_EXIT from 85 to 92

[Richard Sandiford]

Segher Boessenkool <segher@kernel.crashing.org> writes:
> On Fri, Dec 22, 2017 at 04:53:47PM -0600, David Esparza wrote:
>> With a value of 85 GCC has a CPU performance degradation of 11%,
>> reverting PRED_LOOP_EXIT to 92 this degradation disappear.
>> Those values where measured by running c-ray ray-tracer that is a
>> floating point benchmark that runs out of L1 cache.
>
> Why is this single benchmark more important than everything else?
>
> https://patchwork.ozlabs.org/patch/637073/

"Everything" else? :-)  It sounds from Andrew's reply like it wasn't
a win on other benchmarks too.

Neither covering message has really explained why the previous value was
too low/high, but maybe that's just the way it goes with these tuning
parameters...

Thanks,
Richard

Re: [PATCH] Change PRED_LOOP_EXIT from 85 to 92

[Segher Boessenkool]

On Sun, Dec 24, 2017 at 09:12:56AM +0000, Richard Sandiford wrote:
> Segher Boessenkool <segher@kernel.crashing.org> writes:
> > On Fri, Dec 22, 2017 at 04:53:47PM -0600, David Esparza wrote:
> >> With a value of 85 GCC has a CPU performance degradation of 11%,
> >> reverting PRED_LOOP_EXIT to 92 this degradation disappear.
> >> Those values where measured by running c-ray ray-tracer that is a
> >> floating point benchmark that runs out of L1 cache.
> >
> > Why is this single benchmark more important than everything else?
> >
> > https://patchwork.ozlabs.org/patch/637073/
> 
> "Everything" else? :-)  It sounds from Andrew's reply like it wasn't
> a win on other benchmarks too.

Yeah...  But at least Martin tested spec2006, instead of one single
tiny non-representative program.

> Neither covering message has really explained why the previous value was
> too low/high, but maybe that's just the way it goes with these tuning
> parameters...

It would be nice if they explained how they tested things.


Segher

Re: [PATCH] Change PRED_LOOP_EXIT from 85 to 92

[Jeff Law]

On 12/24/2017 05:03 AM, Segher Boessenkool wrote:
> On Sun, Dec 24, 2017 at 09:12:56AM +0000, Richard Sandiford wrote:
>> Segher Boessenkool <segher@kernel.crashing.org> writes:
>>> On Fri, Dec 22, 2017 at 04:53:47PM -0600, David Esparza wrote:
>>>> With a value of 85 GCC has a CPU performance degradation of 11%,
>>>> reverting PRED_LOOP_EXIT to 92 this degradation disappear.
>>>> Those values where measured by running c-ray ray-tracer that is a
>>>> floating point benchmark that runs out of L1 cache.
>>>
>>> Why is this single benchmark more important than everything else?
>>>
>>> https://patchwork.ozlabs.org/patch/637073/
>>
>> "Everything" else? :-)  It sounds from Andrew's reply like it wasn't
>> a win on other benchmarks too.
> 
> Yeah...  But at least Martin tested spec2006, instead of one single
> tiny non-representative program.
> 
>> Neither covering message has really explained why the previous value was
>> too low/high, but maybe that's just the way it goes with these tuning
>> parameters...
> 
> It would be nice if they explained how they tested things.
Agreed.  I can't see any way for this patch to go forward without some
explanation of why it helps this particular c-ray implementation and
some data showing it's not hurtful on a wider suite of benchmarks.


Jeff

Re: [PATCH] Change PRED_LOOP_EXIT from 85 to 92

[Martin Liška]

On 12/24/2017 07:58 PM, Jeff Law wrote:
> On 12/24/2017 05:03 AM, Segher Boessenkool wrote:
>> On Sun, Dec 24, 2017 at 09:12:56AM +0000, Richard Sandiford wrote:
>>> Segher Boessenkool <segher@kernel.crashing.org> writes:
>>>> On Fri, Dec 22, 2017 at 04:53:47PM -0600, David Esparza wrote:
>>>>> With a value of 85 GCC has a CPU performance degradation of 11%,
>>>>> reverting PRED_LOOP_EXIT to 92 this degradation disappear.
>>>>> Those values where measured by running c-ray ray-tracer that is a
>>>>> floating point benchmark that runs out of L1 cache.
>>>>
>>>> Why is this single benchmark more important than everything else?
>>>>
>>>> https://patchwork.ozlabs.org/patch/637073/
>>>
>>> "Everything" else? :-)  It sounds from Andrew's reply like it wasn't
>>> a win on other benchmarks too.
>>
>> Yeah...  But at least Martin tested spec2006, instead of one single
>> tiny non-representative program.
>>
>>> Neither covering message has really explained why the previous value was
>>> too low/high, but maybe that's just the way it goes with these tuning
>>> parameters...
>>
>> It would be nice if they explained how they tested things.
> Agreed.  I can't see any way for this patch to go forward without some
> explanation of why it helps this particular c-ray implementation and
> some data showing it's not hurtful on a wider suite of benchmarks.
> 
> 
> Jeff
> 

Hello.

Sorry for late answer. I've been currently working on returning of predictors
according to SPEC2006 and SPE2017. I've got prepared patches and currently testing
it's impact on speed of SPEC benchmarks.

From what I've measured, suggested adjustment for this particular predictor would be
increase to 89.

Note that this predictor has quite high coverage on SPEC: 5.3% (of all branching).

Martin

Re: [PATCH] Change PRED_LOOP_EXIT from 85 to 92

[Jeff Law]

On 01/02/2018 04:57 AM, Martin Liška wrote:
> On 12/24/2017 07:58 PM, Jeff Law wrote:
>> On 12/24/2017 05:03 AM, Segher Boessenkool wrote:
>>> On Sun, Dec 24, 2017 at 09:12:56AM +0000, Richard Sandiford wrote:
>>>> Segher Boessenkool <segher@kernel.crashing.org> writes:
>>>>> On Fri, Dec 22, 2017 at 04:53:47PM -0600, David Esparza wrote:
>>>>>> With a value of 85 GCC has a CPU performance degradation of 11%,
>>>>>> reverting PRED_LOOP_EXIT to 92 this degradation disappear.
>>>>>> Those values where measured by running c-ray ray-tracer that is a
>>>>>> floating point benchmark that runs out of L1 cache.
>>>>>
>>>>> Why is this single benchmark more important than everything else?
>>>>>
>>>>> https://patchwork.ozlabs.org/patch/637073/
>>>>
>>>> "Everything" else? :-)  It sounds from Andrew's reply like it wasn't
>>>> a win on other benchmarks too.
>>>
>>> Yeah...  But at least Martin tested spec2006, instead of one single
>>> tiny non-representative program.
>>>
>>>> Neither covering message has really explained why the previous value was
>>>> too low/high, but maybe that's just the way it goes with these tuning
>>>> parameters...
>>>
>>> It would be nice if they explained how they tested things.
>> Agreed.  I can't see any way for this patch to go forward without some
>> explanation of why it helps this particular c-ray implementation and
>> some data showing it's not hurtful on a wider suite of benchmarks.
>>
>>
>> Jeff
>>
> 
> Hello.
> 
> Sorry for late answer. I've been currently working on returning of predictors
> according to SPEC2006 and SPE2017. I've got prepared patches and currently testing
> it's impact on speed of SPEC benchmarks.
> 
> From what I've measured, suggested adjustment for this particular predictor would be
> increase to 89.
> 
> Note that this predictor has quite high coverage on SPEC: 5.3% (of all branching).
Understood.  I think using SPEC to guide here makes much more sense.

Jeff

Re: [PATCH] Change PRED_LOOP_EXIT from 85 to 92

[Martin Liška]

On 01/02/2018 12:57 PM, Martin Liška wrote:
> From what I've measured, suggested adjustment for this particular predictor would be
> increase to 89.

Hello.

Note that I've installed the patch as r256888.

Martin