Re: [PATCH 4/N] Recover GOTO predictor.

[David Edelsohn]

This patch breaks bootstrap on AIX and probably is a hidden bug on all targets.

With the patch, libbacktrace/xcoff.c fails to compile with the error:

/nasfarm/edelsohn/src/src/libbacktrace/xcoff.c: In function 'xcoff_add':
/nasfarm/edelsohn/src/src/libbacktrace/xcoff.c:822:13: error: 'incl'
may be used uninitialized in this function
[-Werror=maybe-uninitialized]
filename = incl->filename;
~^~~~
/nasfarm/edelsohn/src/src/libbacktrace/xcoff.c:777:22: note: 'incl'
was declared here
   struct xcoff_incl *incl;
  ^~~~

incl is used immediately above the line that produces the warning --
with no warning.

I can provide a pre-processed xcoff.c, if you need it.

Thanks, David

Re: [PATCH 4/N] Recover GOTO predictor.

[Richard Biener]

On Mon, Jul 31, 2017 at 9:46 AM, Martin Liška  wrote:
> Richi?

4 is fine.

> Thanks
>
> On 06/30/2017 03:48 PM, Martin Liška wrote:
>> On 06/22/2017 12:27 PM, Richard Biener wrote:
>>> On Wed, Jun 21, 2017 at 3:06 PM, Martin Liška  wrote:
 Hello.

 There's one additional predictor enhancement that is GOTO predict that
 used to working. Following patch adds expect statement for C and C++ family
 languages.

 There's one fallout which is vrp24.c test-case, where only 'Simplified 
 relational'
 appears just once. Adding Richi and Patrick who can probably help how to 
 fix the
 test-case.
>>>
>>> Happens to be optimized better now, there's only one predicate to simplify
>>> left in the IL input to VRP1.  I suggest to change it to match 1 times and 
>>> add
>>> -fdump-tree-optimized to dg-options and
>>>
>>> /* { dg-final { scan-tree-dump-times "if " 3 "optimized" } } */
>>>
>>> to verify we have 3 conditions left.
>>
>> One small note, I see 4 conditions in optimized dump:
>>
>> sss (struct rtx_def * insn, int code1, int code2, int code3)
>> {
>>   int D1544;
>>   struct rtx_def * body;
>>   _Bool D1562;
>>
>>[100.00%] [count: INV]:
>>   body_5 = insn_4(D)->u.fld[5].rt_rtx;
>>   D1544_6 = body_5->code;
>>   if (D1544_6 == 55)
>> goto  (L7); [34.00%] [count: INV]
>>   else
>> goto ; [66.00%] [count: INV]
>>
>>[66.00%] [count: INV]:
>>   if (code3_7(D) == 99)
>> goto ; [34.00%] [count: INV]
>>   else
>> goto  (L16); [66.00%] [count: INV]
>>
>>[22.44%] [count: INV]:
>>   D1562_9 = code1_8(D) == 10;
>>   if (D1562_9 == 1)
>> goto  (L7); [64.00%] [count: INV]
>>   else
>> goto  (L16); [36.00%] [count: INV]
>>
>>[9.82%] [count: INV]:
>>   arf ();
>>
>>[46.68%] [count: INV]:
>>   frob (); [tail call]
>>   goto  (L16); [100.00%] [count: INV]
>>
>> L7 [48.36%] [count: INV]:
>>   if (code2_12(D) == 42)
>> goto ; [20.24%] [count: INV]
>>   else
>> goto ; [79.76%] [count: INV]
>>
>> L16 [100.00%] [count: INV]:
>>   return;
>>
>> }
>>
>> Is it a problem or adjusting to 4 is fine?
>>
>> Martin
>>
>>>
 Patch can bootstrap on ppc64le-redhat-linux and survives regression tests.

 Ready to be installed?
 Martin
>>
>

Re: [PATCH 4/N] Recover GOTO predictor.

[Martin Liška]

Richi?

Thanks

On 06/30/2017 03:48 PM, Martin Liška wrote:
> On 06/22/2017 12:27 PM, Richard Biener wrote:
>> On Wed, Jun 21, 2017 at 3:06 PM, Martin Liška  wrote:
>>> Hello.
>>>
>>> There's one additional predictor enhancement that is GOTO predict that
>>> used to working. Following patch adds expect statement for C and C++ family
>>> languages.
>>>
>>> There's one fallout which is vrp24.c test-case, where only 'Simplified 
>>> relational'
>>> appears just once. Adding Richi and Patrick who can probably help how to 
>>> fix the
>>> test-case.
>>
>> Happens to be optimized better now, there's only one predicate to simplify
>> left in the IL input to VRP1.  I suggest to change it to match 1 times and 
>> add
>> -fdump-tree-optimized to dg-options and
>>
>> /* { dg-final { scan-tree-dump-times "if " 3 "optimized" } } */
>>
>> to verify we have 3 conditions left.
> 
> One small note, I see 4 conditions in optimized dump:
> 
> sss (struct rtx_def * insn, int code1, int code2, int code3)
> {
>   int D1544;
>   struct rtx_def * body;
>   _Bool D1562;
> 
>[100.00%] [count: INV]:
>   body_5 = insn_4(D)->u.fld[5].rt_rtx;
>   D1544_6 = body_5->code;
>   if (D1544_6 == 55)
> goto  (L7); [34.00%] [count: INV]
>   else
> goto ; [66.00%] [count: INV]
> 
>[66.00%] [count: INV]:
>   if (code3_7(D) == 99)
> goto ; [34.00%] [count: INV]
>   else
> goto  (L16); [66.00%] [count: INV]
> 
>[22.44%] [count: INV]:
>   D1562_9 = code1_8(D) == 10;
>   if (D1562_9 == 1)
> goto  (L7); [64.00%] [count: INV]
>   else
> goto  (L16); [36.00%] [count: INV]
> 
>[9.82%] [count: INV]:
>   arf ();
> 
>[46.68%] [count: INV]:
>   frob (); [tail call]
>   goto  (L16); [100.00%] [count: INV]
> 
> L7 [48.36%] [count: INV]:
>   if (code2_12(D) == 42)
> goto ; [20.24%] [count: INV]
>   else
> goto ; [79.76%] [count: INV]
> 
> L16 [100.00%] [count: INV]:
>   return;
> 
> }
> 
> Is it a problem or adjusting to 4 is fine?
> 
> Martin
> 
>>
>>> Patch can bootstrap on ppc64le-redhat-linux and survives regression tests.
>>>
>>> Ready to be installed?
>>> Martin
> 

Re: [PATCH 4/N] Recover GOTO predictor.

[Martin Liška]

On 06/22/2017 12:27 PM, Richard Biener wrote:
> On Wed, Jun 21, 2017 at 3:06 PM, Martin Liška  wrote:
>> Hello.
>>
>> There's one additional predictor enhancement that is GOTO predict that
>> used to working. Following patch adds expect statement for C and C++ family
>> languages.
>>
>> There's one fallout which is vrp24.c test-case, where only 'Simplified 
>> relational'
>> appears just once. Adding Richi and Patrick who can probably help how to fix 
>> the
>> test-case.
> 
> Happens to be optimized better now, there's only one predicate to simplify
> left in the IL input to VRP1.  I suggest to change it to match 1 times and add
> -fdump-tree-optimized to dg-options and
> 
> /* { dg-final { scan-tree-dump-times "if " 3 "optimized" } } */
> 
> to verify we have 3 conditions left.

One small note, I see 4 conditions in optimized dump:

sss (struct rtx_def * insn, int code1, int code2, int code3)
{
  int D1544;
  struct rtx_def * body;
  _Bool D1562;

   [100.00%] [count: INV]:
  body_5 = insn_4(D)->u.fld[5].rt_rtx;
  D1544_6 = body_5->code;
  if (D1544_6 == 55)
goto  (L7); [34.00%] [count: INV]
  else
goto ; [66.00%] [count: INV]

   [66.00%] [count: INV]:
  if (code3_7(D) == 99)
goto ; [34.00%] [count: INV]
  else
goto  (L16); [66.00%] [count: INV]

   [22.44%] [count: INV]:
  D1562_9 = code1_8(D) == 10;
  if (D1562_9 == 1)
goto  (L7); [64.00%] [count: INV]
  else
goto  (L16); [36.00%] [count: INV]

   [9.82%] [count: INV]:
  arf ();

   [46.68%] [count: INV]:
  frob (); [tail call]
  goto  (L16); [100.00%] [count: INV]

L7 [48.36%] [count: INV]:
  if (code2_12(D) == 42)
goto ; [20.24%] [count: INV]
  else
goto ; [79.76%] [count: INV]

L16 [100.00%] [count: INV]:
  return;

}

Is it a problem or adjusting to 4 is fine?

Martin

> 
>> Patch can bootstrap on ppc64le-redhat-linux and survives regression tests.
>>
>> Ready to be installed?
>> Martin

Re: [PATCH 4/N] Recover GOTO predictor.

[Jan Hubicka]

> PING^1
> 
> Can you please Honza give a formal approval for the patch?

OK,
thanks!
Honza
> 
> Thanks,
> Martin
> 
> On 06/22/2017 01:47 PM, Richard Biener wrote:
> > On Thu, Jun 22, 2017 at 12:57 PM, Martin Liška  wrote:
> >> On 06/22/2017 12:27 PM, Richard Biener wrote:
> >>> On Wed, Jun 21, 2017 at 3:06 PM, Martin Liška  wrote:
>  Hello.
> 
>  There's one additional predictor enhancement that is GOTO predict that
>  used to working. Following patch adds expect statement for C and C++ 
>  family
>  languages.
> 
>  There's one fallout which is vrp24.c test-case, where only 'Simplified 
>  relational'
>  appears just once. Adding Richi and Patrick who can probably help how to 
>  fix the
>  test-case.
> >>>
> >>> Happens to be optimized better now, there's only one predicate to simplify
> >>> left in the IL input to VRP1.  I suggest to change it to match 1 times 
> >>> and add
> >>> -fdump-tree-optimized to dg-options and
> >>>
> >>> /* { dg-final { scan-tree-dump-times "if " 3 "optimized" } } */
> >>>
> >>> to verify we have 3 conditions left.
> >>
> >> Thanks for help.
> >> What about the comment:
> >>
> >> /* The first n_sets > 0 test can be simplfiied into n_sets == 1 since
> >>n_sets can only have the values [0, 1] as it's the result of a
> >>boolean operation.
> >>
> >>The second n_sets > 0 test can also be simplified into n_sets == 1
> >>as the only way to reach the tests is when n_sets <= 1 and the only
> >>value which satisfies both conditions is n_sets == 1.  */
> >>
> >> I guess just only one can be valid? Or is it a different story now?
> > 
> > The 2nd one is handled by earlier jump-threading.
> > 
> >> Martin
> >>
> >>>
>  Patch can bootstrap on ppc64le-redhat-linux and survives regression 
>  tests.
> 
>  Ready to be installed?
>  Martin
> >>

Re: [PATCH 4/N] Recover GOTO predictor.

[Martin Liška]

PING^1

Can you please Honza give a formal approval for the patch?

Thanks,
Martin

On 06/22/2017 01:47 PM, Richard Biener wrote:
> On Thu, Jun 22, 2017 at 12:57 PM, Martin Liška  wrote:
>> On 06/22/2017 12:27 PM, Richard Biener wrote:
>>> On Wed, Jun 21, 2017 at 3:06 PM, Martin Liška  wrote:
 Hello.

 There's one additional predictor enhancement that is GOTO predict that
 used to working. Following patch adds expect statement for C and C++ family
 languages.

 There's one fallout which is vrp24.c test-case, where only 'Simplified 
 relational'
 appears just once. Adding Richi and Patrick who can probably help how to 
 fix the
 test-case.
>>>
>>> Happens to be optimized better now, there's only one predicate to simplify
>>> left in the IL input to VRP1.  I suggest to change it to match 1 times and 
>>> add
>>> -fdump-tree-optimized to dg-options and
>>>
>>> /* { dg-final { scan-tree-dump-times "if " 3 "optimized" } } */
>>>
>>> to verify we have 3 conditions left.
>>
>> Thanks for help.
>> What about the comment:
>>
>> /* The first n_sets > 0 test can be simplfiied into n_sets == 1 since
>>n_sets can only have the values [0, 1] as it's the result of a
>>boolean operation.
>>
>>The second n_sets > 0 test can also be simplified into n_sets == 1
>>as the only way to reach the tests is when n_sets <= 1 and the only
>>value which satisfies both conditions is n_sets == 1.  */
>>
>> I guess just only one can be valid? Or is it a different story now?
> 
> The 2nd one is handled by earlier jump-threading.
> 
>> Martin
>>
>>>
 Patch can bootstrap on ppc64le-redhat-linux and survives regression tests.

 Ready to be installed?
 Martin
>>

Re: [PATCH 4/N] Recover GOTO predictor.

[Richard Biener]

On Thu, Jun 22, 2017 at 12:57 PM, Martin Liška  wrote:
> On 06/22/2017 12:27 PM, Richard Biener wrote:
>> On Wed, Jun 21, 2017 at 3:06 PM, Martin Liška  wrote:
>>> Hello.
>>>
>>> There's one additional predictor enhancement that is GOTO predict that
>>> used to working. Following patch adds expect statement for C and C++ family
>>> languages.
>>>
>>> There's one fallout which is vrp24.c test-case, where only 'Simplified 
>>> relational'
>>> appears just once. Adding Richi and Patrick who can probably help how to 
>>> fix the
>>> test-case.
>>
>> Happens to be optimized better now, there's only one predicate to simplify
>> left in the IL input to VRP1.  I suggest to change it to match 1 times and 
>> add
>> -fdump-tree-optimized to dg-options and
>>
>> /* { dg-final { scan-tree-dump-times "if " 3 "optimized" } } */
>>
>> to verify we have 3 conditions left.
>
> Thanks for help.
> What about the comment:
>
> /* The first n_sets > 0 test can be simplfiied into n_sets == 1 since
>n_sets can only have the values [0, 1] as it's the result of a
>boolean operation.
>
>The second n_sets > 0 test can also be simplified into n_sets == 1
>as the only way to reach the tests is when n_sets <= 1 and the only
>value which satisfies both conditions is n_sets == 1.  */
>
> I guess just only one can be valid? Or is it a different story now?

The 2nd one is handled by earlier jump-threading.

> Martin
>
>>
>>> Patch can bootstrap on ppc64le-redhat-linux and survives regression tests.
>>>
>>> Ready to be installed?
>>> Martin
>

Re: [PATCH 4/N] Recover GOTO predictor.

[Martin Liška]

On 06/22/2017 12:27 PM, Richard Biener wrote:
> On Wed, Jun 21, 2017 at 3:06 PM, Martin Liška  wrote:
>> Hello.
>>
>> There's one additional predictor enhancement that is GOTO predict that
>> used to working. Following patch adds expect statement for C and C++ family
>> languages.
>>
>> There's one fallout which is vrp24.c test-case, where only 'Simplified 
>> relational'
>> appears just once. Adding Richi and Patrick who can probably help how to fix 
>> the
>> test-case.
> 
> Happens to be optimized better now, there's only one predicate to simplify
> left in the IL input to VRP1.  I suggest to change it to match 1 times and add
> -fdump-tree-optimized to dg-options and
> 
> /* { dg-final { scan-tree-dump-times "if " 3 "optimized" } } */
> 
> to verify we have 3 conditions left.

Thanks for help.
What about the comment:

/* The first n_sets > 0 test can be simplfiied into n_sets == 1 since
   n_sets can only have the values [0, 1] as it's the result of a
   boolean operation.

   The second n_sets > 0 test can also be simplified into n_sets == 1
   as the only way to reach the tests is when n_sets <= 1 and the only
   value which satisfies both conditions is n_sets == 1.  */

I guess just only one can be valid? Or is it a different story now?

Martin

> 
>> Patch can bootstrap on ppc64le-redhat-linux and survives regression tests.
>>
>> Ready to be installed?
>> Martin

Re: [PATCH 4/N] Recover GOTO predictor.

[Richard Biener]

On Wed, Jun 21, 2017 at 3:06 PM, Martin Liška  wrote:
> Hello.
>
> There's one additional predictor enhancement that is GOTO predict that
> used to working. Following patch adds expect statement for C and C++ family
> languages.
>
> There's one fallout which is vrp24.c test-case, where only 'Simplified 
> relational'
> appears just once. Adding Richi and Patrick who can probably help how to fix 
> the
> test-case.

Happens to be optimized better now, there's only one predicate to simplify
left in the IL input to VRP1.  I suggest to change it to match 1 times and add
-fdump-tree-optimized to dg-options and

/* { dg-final { scan-tree-dump-times "if " 3 "optimized" } } */

to verify we have 3 conditions left.

> Patch can bootstrap on ppc64le-redhat-linux and survives regression tests.
>
> Ready to be installed?
> Martin

Re: [PATCH 4/N] Recover GOTO predictor.

[Martin Liška]

On 06/21/2017 03:06 PM, Martin Liška wrote:
> Hello.
> 
> There's one additional predictor enhancement that is GOTO predict that
> used to working. Following patch adds expect statement for C and C++ family
> languages.
> 
> There's one fallout which is vrp24.c test-case, where only 'Simplified 
> relational'
> appears just once. Adding Richi and Patrick who can probably help how to fix 
> the
> test-case.
> 
> Patch can bootstrap on ppc64le-redhat-linux and survives regression tests.
> 
> Ready to be installed?
> Martin
> 

And I forgot to mention hitrate on SPEC2017:

HEURISTICS   BRANCHES  (REL)  BR. HITRATE   
 HITRATE   COVERAGE COVERAGE  (REL)  predict.def  (REL)
goto  622   1.0%   64.31%   65.92% 
/  83.70%  725127790  725.13M   0.1%

Which says it's quite rare predictor, but with quite nice hitrate.

Martin