public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH]Partially fix PR61529, bound basic block frequency
@ 2014-10-29  9:35 Renlin Li
  2014-10-29 12:55 ` Teresa Johnson
  0 siblings, 1 reply; 12+ messages in thread
From: Renlin Li @ 2014-10-29  9:35 UTC (permalink / raw)
  To: gcc-patches; +Cc: teresa Johnson, ramana Radhakrishnan, su

[-- Attachment #1: Type: text/plain, Size: 825 bytes --]

Hi all,

This is a simple patch to fix ICE in comment 2 of PR61529: 
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=61529

Bound checking code is added to make sure the frequency is within legal 
range.

As far as I have observed, r215830 patch fixes the glibc building ICE. 
And this patch should fix the ICE while building the sample code in 
comment 2 using aarch64-none-elf toolchain. Until now, all the ICEs 
reported in this bug ticket should be fixed.

x86_64-unknown-linux-gnu bootstrap and regression test have been done, 
no new issue.
aarch64-none-elf toolchain has been test on the model. No new regression.

Is this Okay for trunk?

gcc/ChangeLog:

2014-10-29  Renlin Li  <Renlin.Li@arm.com>
      PR middle-end/61529
     * tree-ssa-threadupdate.c (compute_path_counts): Bound path_in_freq.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: patch-1.diff --]
[-- Type: text/x-patch; name=patch-1.diff, Size: 703 bytes --]

commit c44195cb52ec8ac6386b2b7afe467b680422fb2e
Author: Renlin Li <renlin.li@arm.com>
Date:   Tue Oct 28 16:30:42 2014 +0000

    fix pr61529
    
    Change-Id: Ie5e58510f21a4d7a609306006270c3168ab48d06

diff --git a/gcc/tree-ssa-threadupdate.c b/gcc/tree-ssa-threadupdate.c
index d2cf4de..e3077a1 100644
--- a/gcc/tree-ssa-threadupdate.c
+++ b/gcc/tree-ssa-threadupdate.c
@@ -730,6 +730,10 @@ compute_path_counts (struct redirection_data *rd,
             nonpath_count += ein->count;
         }
     }
+
+  if (path_in_freq > BB_FREQ_MAX)
+    path_in_freq = BB_FREQ_MAX;
+
   BITMAP_FREE (in_edge_srcs);
 
   /* Now compute the fraction of the total count coming into the first

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH]Partially fix PR61529, bound basic block frequency
  2014-10-29  9:35 [PATCH]Partially fix PR61529, bound basic block frequency Renlin Li
@ 2014-10-29 12:55 ` Teresa Johnson
  2014-11-03 15:29   ` Renlin Li
  0 siblings, 1 reply; 12+ messages in thread
From: Teresa Johnson @ 2014-10-29 12:55 UTC (permalink / raw)
  To: Renlin Li, Dehao Chen; +Cc: gcc-patches, ramana Radhakrishnan, su

Hi Renlin,

Are the incoming edge counts or probabilities insane in this case? I
guess the patch is ok if we need to do this to handle those incoming
insanitiles. But I can't approve patches myself.

However, this is a fix to code (r215739) committed after the ICE in
the original bug report and in comment 2 were reported, so I wonder if
it is just hiding the original problem. Originally this was reported
to be due to r210538 - ccing Dehao who was the author of that patch.
Dehao, did you get a chance to look at this bug and see why your
change triggered it? It is possible that Dehao's patch simply
amplified an even further upstream profile insanity, but it would be
good to confirm.

Thanks!
Teresa

On Wed, Oct 29, 2014 at 2:26 AM, Renlin Li <renlin.li@arm.com> wrote:
> Hi all,
>
> This is a simple patch to fix ICE in comment 2 of PR61529:
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=61529
>
> Bound checking code is added to make sure the frequency is within legal
> range.
>
> As far as I have observed, r215830 patch fixes the glibc building ICE. And
> this patch should fix the ICE while building the sample code in comment 2
> using aarch64-none-elf toolchain. Until now, all the ICEs reported in this
> bug ticket should be fixed.
>
> x86_64-unknown-linux-gnu bootstrap and regression test have been done, no
> new issue.
> aarch64-none-elf toolchain has been test on the model. No new regression.
>
> Is this Okay for trunk?
>
> gcc/ChangeLog:
>
> 2014-10-29  Renlin Li  <Renlin.Li@arm.com>
>      PR middle-end/61529
>     * tree-ssa-threadupdate.c (compute_path_counts): Bound path_in_freq.



-- 
Teresa Johnson | Software Engineer | tejohnson@google.com | 408-460-2413

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH]Partially fix PR61529, bound basic block frequency
  2014-10-29 12:55 ` Teresa Johnson
@ 2014-11-03 15:29   ` Renlin Li
  2014-11-04 21:59     ` Jeff Law
  0 siblings, 1 reply; 12+ messages in thread
From: Renlin Li @ 2014-11-03 15:29 UTC (permalink / raw)
  To: Teresa Johnson, Dehao Chen; +Cc: gcc-patches, Ramana Radhakrishnan, su

On 29/10/14 12:42, Teresa Johnson wrote:
> Hi Renlin,
>
> Are the incoming edge counts or probabilities insane in this case? I
> guess the patch is ok if we need to do this to handle those incoming
> insanitiles. But I can't approve patches myself.

Not really, it's just a little bigger than the limit.

For this particular test case, ABC is a threaded path.
B is the fallthrough basic block of A, D is a basic block split from B 
(used to be a self loop). A, B and D have roughly the same frequency ( 
8281, 9100, 8281).
When calculating the path_in_freq, frequencies from AB and DB edges are 
accumulated, and the final result is large than BB_FREQ_MAX.


           A
100% |
           |      9%
------>B---------->C
|         |
|100%| 91%
|         |
--------D



There are 2 suspicious points:
1, The BD edge is not correctly guessed at the profile stage. However, 
anyway it's heuristic, so I don't think, it's here the problem starts.
2, The BD edge is not eliminated before jump threading. But the jump 
threading pass will analysis the condition jump statement in B block (In 
this particular case, the BD probability should be zero), and makes the 
decision to thread it.

Later in the dom pass, the BD edge is indeed removed.

> However, this is a fix to code (r215739) committed after the ICE in
> the original bug report and in comment 2 were reported, so I wonder if
> it is just hiding the original problem. Originally this was reported
> to be due to r210538 - ccing Dehao who was the author of that patch.
> Dehao, did you get a chance to look at this bug and see why your
> change triggered it? It is possible that Dehao's patch simply
> amplified an even further upstream profile insanity, but it would be
> good to confirm.
>
> Thanks!
> Teresa
>
> On Wed, Oct 29, 2014 at 2:26 AM, Renlin Li<renlin.li@arm.com>  wrote:
>> Hi all,
>>
>> This is a simple patch to fix ICE in comment 2 of PR61529:
>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=61529
>>
>> Bound checking code is added to make sure the frequency is within legal
>> range.
>>
>> As far as I have observed, r215830 patch fixes the glibc building ICE. And
>> this patch should fix the ICE while building the sample code in comment 2
>> using aarch64-none-elf toolchain. Until now, all the ICEs reported in this
>> bug ticket should be fixed.
>>
>> x86_64-unknown-linux-gnu bootstrap and regression test have been done, no
>> new issue.
>> aarch64-none-elf toolchain has been test on the model. No new regression.
>>
>> Is this Okay for trunk?
>>
>> gcc/ChangeLog:
>>
>> 2014-10-29  Renlin Li<Renlin.Li@arm.com>
>>       PR middle-end/61529
>>      * tree-ssa-threadupdate.c (compute_path_counts): Bound path_in_freq.



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH]Partially fix PR61529, bound basic block frequency
  2014-11-03 15:29   ` Renlin Li
@ 2014-11-04 21:59     ` Jeff Law
  2014-11-06 15:09       ` Renlin Li
  0 siblings, 1 reply; 12+ messages in thread
From: Jeff Law @ 2014-11-04 21:59 UTC (permalink / raw)
  To: Renlin Li, Teresa Johnson, Dehao Chen
  Cc: gcc-patches, Ramana Radhakrishnan, su

On 11/03/14 08:29, Renlin Li wrote:
> On 29/10/14 12:42, Teresa Johnson wrote:
>> Hi Renlin,
>>
>> Are the incoming edge counts or probabilities insane in this case? I
>> guess the patch is ok if we need to do this to handle those incoming
>> insanitiles. But I can't approve patches myself.
>
> Not really, it's just a little bigger than the limit.
>
> For this particular test case, ABC is a threaded path.
> B is the fallthrough basic block of A, D is a basic block split from B
> (used to be a self loop). A, B and D have roughly the same frequency (
> 8281, 9100, 8281).
> When calculating the path_in_freq, frequencies from AB and DB edges are
> accumulated, and the final result is large than BB_FREQ_MAX.
>
>
>            A
> 100% |
>            |      9%
> ------>B---------->C
> |         |
> |100%| 91%
> |         |
> --------D
>
>
>
> There are 2 suspicious points:
> 1, The BD edge is not correctly guessed at the profile stage. However,
> anyway it's heuristic, so I don't think, it's here the problem starts.
> 2, The BD edge is not eliminated before jump threading. But the jump
> threading pass will analysis the condition jump statement in B block (In
> this particular case, the BD probability should be zero), and makes the
> decision to thread it.
>
> Later in the dom pass, the BD edge is indeed removed.
Can you add a testcase please?  With a testcase, this patch is OK for 
the trunk.

jeff

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH]Partially fix PR61529, bound basic block frequency
  2014-11-04 21:59     ` Jeff Law
@ 2014-11-06 15:09       ` Renlin Li
  2014-11-06 15:38         ` Teresa Johnson
  0 siblings, 1 reply; 12+ messages in thread
From: Renlin Li @ 2014-11-06 15:09 UTC (permalink / raw)
  To: Jeff Law, Teresa Johnson, Dehao Chen
  Cc: gcc-patches, Ramana Radhakrishnan, su

[-- Attachment #1: Type: text/plain, Size: 2166 bytes --]


Hi Jeff,

Test case has been added. With the patch, both x86_64-unknown-linux-gnu 
and aarch64-none-elf compile the test case successfully.

Okay to commit?

On 04/11/14 21:59, Jeff Law wrote:
> On 11/03/14 08:29, Renlin Li wrote:
>> On 29/10/14 12:42, Teresa Johnson wrote:
>>> Hi Renlin,
>>>
>>> Are the incoming edge counts or probabilities insane in this case? I
>>> guess the patch is ok if we need to do this to handle those incoming
>>> insanitiles. But I can't approve patches myself.
>> Not really, it's just a little bigger than the limit.
>>
>> For this particular test case, ABC is a threaded path.
>> B is the fallthrough basic block of A, D is a basic block split from B
>> (used to be a self loop). A, B and D have roughly the same frequency (
>> 8281, 9100, 8281).
>> When calculating the path_in_freq, frequencies from AB and DB edges are
>> accumulated, and the final result is large than BB_FREQ_MAX.
>>
>>
>>             A
>> 100% |
>>             |      9%
>> ------>B---------->C
>> |         |
>> |100%| 91%
>> |         |
>> --------D
>>
>>
>>
>> There are 2 suspicious points:
>> 1, The BD edge is not correctly guessed at the profile stage. However,
>> anyway it's heuristic, so I don't think, it's here the problem starts.
>> 2, The BD edge is not eliminated before jump threading. But the jump
>> threading pass will analysis the condition jump statement in B block (In
>> this particular case, the BD probability should be zero), and makes the
>> decision to thread it.
>>
>> Later in the dom pass, the BD edge is indeed removed.
> Can you add a testcase please?  With a testcase, this patch is OK for
> the trunk.
>
> jeff
>

x86_64-unknown-linux-gnu bootstrap and regression test have been done, 
no new issue.
aarch64-none-elf toolchain has been test on the model. No new regression.

gcc/ChangeLog:

2014-11-06  Renlin Li  <Renlin.Li@arm.com>
     PR middle-end/61529
     * tree-ssa-threadupdate.c (compute_path_counts): Bound path_in_freq.

gcc/testsuite/ChangeLog:

2014-11-06  Renlin Li  <Renlin.Li@arm.com>
     PR middle-end/61529
     * gcc.dg/pr61529.c: New.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: patch.diff --]
[-- Type: text/x-patch; name=patch.diff, Size: 1202 bytes --]

commit fead29d30b2985a1ba338759054f99d71d81f3c0
Author: Renlin Li <renlin.li@arm.com>
Date:   Tue Oct 28 16:30:42 2014 +0000

    fix pr61529

diff --git a/gcc/testsuite/gcc.dg/pr61529.c b/gcc/testsuite/gcc.dg/pr61529.c
new file mode 100644
index 0000000..9ae2e06
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr61529.c
@@ -0,0 +1,26 @@
+/* PR middle-end/61529 */
+/* { dg-do compile } */
+/* { dg-options "-O3" } */
+
+unsigned int a, b, c;
+
+int
+main ()
+{
+  unsigned int d;
+  int e[5];
+
+  for (; b < 1; b++)
+    d = 0;
+  for (; d < 1; d++)
+    a = 0;
+  for (; a < 1; a++)
+    ;
+
+  for (c = 0; c < 5; c++)
+    e[c] = 1;
+  if (e[0])
+    c = 0;
+
+  return 0;
+}
diff --git a/gcc/tree-ssa-threadupdate.c b/gcc/tree-ssa-threadupdate.c
index d2cf4de..e3077a1 100644
--- a/gcc/tree-ssa-threadupdate.c
+++ b/gcc/tree-ssa-threadupdate.c
@@ -730,6 +730,10 @@ compute_path_counts (struct redirection_data *rd,
             nonpath_count += ein->count;
         }
     }
+
+  if (path_in_freq > BB_FREQ_MAX)
+    path_in_freq = BB_FREQ_MAX;
+
   BITMAP_FREE (in_edge_srcs);
 
   /* Now compute the fraction of the total count coming into the first

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH]Partially fix PR61529, bound basic block frequency
  2014-11-06 15:09       ` Renlin Li
@ 2014-11-06 15:38         ` Teresa Johnson
  2014-11-06 17:54           ` Renlin Li
  0 siblings, 1 reply; 12+ messages in thread
From: Teresa Johnson @ 2014-11-06 15:38 UTC (permalink / raw)
  To: Renlin Li; +Cc: Jeff Law, Dehao Chen, gcc-patches, Ramana Radhakrishnan, su

On Thu, Nov 6, 2014 at 7:09 AM, Renlin Li <renlin.li@arm.com> wrote:
>
> Hi Jeff,
>
> Test case has been added. With the patch, both x86_64-unknown-linux-gnu and
> aarch64-none-elf compile the test case successfully.
>
> Okay to commit?
>
>
> On 04/11/14 21:59, Jeff Law wrote:
>>
>> On 11/03/14 08:29, Renlin Li wrote:
>>>
>>> On 29/10/14 12:42, Teresa Johnson wrote:
>>>>
>>>> Hi Renlin,
>>>>
>>>> Are the incoming edge counts or probabilities insane in this case? I
>>>> guess the patch is ok if we need to do this to handle those incoming
>>>> insanitiles. But I can't approve patches myself.
>>>
>>> Not really, it's just a little bigger than the limit.
>>>
>>> For this particular test case, ABC is a threaded path.
>>> B is the fallthrough basic block of A, D is a basic block split from B
>>> (used to be a self loop). A, B and D have roughly the same frequency (
>>> 8281, 9100, 8281).
>>> When calculating the path_in_freq, frequencies from AB and DB edges are
>>> accumulated, and the final result is large than BB_FREQ_MAX.
>>>
>>>
>>>             A
>>> 100% |
>>>             |      9%
>>> ------>B---------->C
>>> |         |
>>> |100%| 91%
>>> |         |
>>> --------D

The frequencies look insane given these probabilities. If most of the
execution stays in the loop then B should have a much higher frequency
than A.

>>>
>>>
>>>
>>> There are 2 suspicious points:
>>> 1, The BD edge is not correctly guessed at the profile stage. However,
>>> anyway it's heuristic, so I don't think, it's here the problem starts.
>>> 2, The BD edge is not eliminated before jump threading. But the jump
>>> threading pass will analysis the condition jump statement in B block (In
>>> this particular case, the BD probability should be zero), and makes the
>>> decision to thread it.
>>>
>>> Later in the dom pass, the BD edge is indeed removed.
>>
>> Can you add a testcase please?  With a testcase, this patch is OK for
>> the trunk.
>>
>> jeff
>>
>
> x86_64-unknown-linux-gnu bootstrap and regression test have been done, no
> new issue.
> aarch64-none-elf toolchain has been test on the model. No new regression.
>
> gcc/ChangeLog:
>
> 2014-11-06  Renlin Li  <Renlin.Li@arm.com>
>     PR middle-end/61529
>     * tree-ssa-threadupdate.c (compute_path_counts): Bound path_in_freq.

Please add a comment that this is needed due to insane incoming frequencies.

>
> gcc/testsuite/ChangeLog:
>
> 2014-11-06  Renlin Li  <Renlin.Li@arm.com>
>     PR middle-end/61529
>     * gcc.dg/pr61529.c: New.

The 'b' variable is uninitialized. Also, 'd' and 'a' may end up
uninitialized depending on the initial value of 'b'. Please initialize
these.

Thanks,
Teresa


-- 
Teresa Johnson | Software Engineer | tejohnson@google.com | 408-460-2413

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH]Partially fix PR61529, bound basic block frequency
  2014-11-06 15:38         ` Teresa Johnson
@ 2014-11-06 17:54           ` Renlin Li
  2014-11-06 17:59             ` Teresa Johnson
  0 siblings, 1 reply; 12+ messages in thread
From: Renlin Li @ 2014-11-06 17:54 UTC (permalink / raw)
  To: Teresa Johnson
  Cc: Jeff Law, Dehao Chen, gcc-patches, Ramana Radhakrishnan, su

[-- Attachment #1: Type: text/plain, Size: 944 bytes --]

Hi Teresa,

Thank you for the suggestion, updated!

> Please add a comment that this is needed due to insane incoming frequencies.
>
> The 'b' variable is uninitialized. Also, 'd' and 'a' may end up
> uninitialized depending on the initial value of 'b'. Please initialize
> these.
>

Test case has been added. With the patch, both x86_64-unknown-linux-gnu 
and aarch64-none-elf compile the test case successfully.

x86_64-unknown-linux-gnu bootstrap and regression test have been done, 
no new issue.
aarch64-none-elf toolchain has been test on the model. No new regression.

gcc/ChangeLog:

2014-11-06  Renlin Li  <Renlin.Li@arm.com>
     PR middle-end/61529
     * tree-ssa-threadupdate.c (compute_path_counts): Bound path_in_freq.
      This is needed due to insane incoming frequencies.

gcc/testsuite/ChangeLog:

2014-11-06  Renlin Li  <Renlin.Li@arm.com>
     PR middle-end/61529
     * gcc.dg/pr61529.c: New.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: patch-1.diff --]
[-- Type: text/x-patch; name=patch-1.diff, Size: 1229 bytes --]

commit b38cf02619f03be9d200849734f4454502d1e5ac
Author: Renlin Li <renlin.li@arm.com>
Date:   Tue Oct 28 16:30:42 2014 +0000

    fix pr61529

diff --git a/gcc/testsuite/gcc.dg/pr61529.c b/gcc/testsuite/gcc.dg/pr61529.c
new file mode 100644
index 0000000..392239e
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr61529.c
@@ -0,0 +1,27 @@
+/* PR middle-end/61529 */
+/* { dg-do compile } */
+/* { dg-options "-O3" } */
+
+unsigned int a = 0, b = 0;
+unsigned int c;
+
+int
+main ()
+{
+  unsigned int d = 0;
+  int e[5];
+
+  for (; b < 1; b++)
+    d = 0;
+  for (; d < 1; d++)
+    a = 0;
+  for (; a < 1; a++)
+    ;
+
+  for (c = 0; c < 5; c++)
+    e[c] = 1;
+  if (e[0])
+    c = 0;
+
+  return 0;
+}
diff --git a/gcc/tree-ssa-threadupdate.c b/gcc/tree-ssa-threadupdate.c
index d2cf4de..e3077a1 100644
--- a/gcc/tree-ssa-threadupdate.c
+++ b/gcc/tree-ssa-threadupdate.c
@@ -730,6 +730,10 @@ compute_path_counts (struct redirection_data *rd,
             nonpath_count += ein->count;
         }
     }
+
+  if (path_in_freq > BB_FREQ_MAX)
+    path_in_freq = BB_FREQ_MAX;
+
   BITMAP_FREE (in_edge_srcs);
 
   /* Now compute the fraction of the total count coming into the first

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH]Partially fix PR61529, bound basic block frequency
  2014-11-06 17:54           ` Renlin Li
@ 2014-11-06 17:59             ` Teresa Johnson
  2014-11-06 18:07               ` Renlin Li
  0 siblings, 1 reply; 12+ messages in thread
From: Teresa Johnson @ 2014-11-06 17:59 UTC (permalink / raw)
  To: Renlin Li; +Cc: Jeff Law, Dehao Chen, gcc-patches, Ramana Radhakrishnan, su

Thanks for fixing the test case. Can you also add the comment I
suggested to the source change?

> Please add a comment that this is needed due to insane incoming frequencies.

Thanks,
Teresa

On Thu, Nov 6, 2014 at 9:53 AM, Renlin Li <renlin.li@arm.com> wrote:
> Hi Teresa,
>
> Thank you for the suggestion, updated!
>
>> Please add a comment that this is needed due to insane incoming
>> frequencies.
>>
>> The 'b' variable is uninitialized. Also, 'd' and 'a' may end up
>> uninitialized depending on the initial value of 'b'. Please initialize
>> these.
>>
>
> Test case has been added. With the patch, both x86_64-unknown-linux-gnu and
> aarch64-none-elf compile the test case successfully.
>
> x86_64-unknown-linux-gnu bootstrap and regression test have been done, no
> new issue.
> aarch64-none-elf toolchain has been test on the model. No new regression.
>
> gcc/ChangeLog:
>
> 2014-11-06  Renlin Li  <Renlin.Li@arm.com>
>     PR middle-end/61529
>     * tree-ssa-threadupdate.c (compute_path_counts): Bound path_in_freq.
>      This is needed due to insane incoming frequencies.
>
> gcc/testsuite/ChangeLog:
>
> 2014-11-06  Renlin Li  <Renlin.Li@arm.com>
>     PR middle-end/61529
>     * gcc.dg/pr61529.c: New.



-- 
Teresa Johnson | Software Engineer | tejohnson@google.com | 408-460-2413

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH]Partially fix PR61529, bound basic block frequency
  2014-11-06 17:59             ` Teresa Johnson
@ 2014-11-06 18:07               ` Renlin Li
  2014-11-10 17:00                 ` [PING][PATCH]Partially " Renlin Li
  0 siblings, 1 reply; 12+ messages in thread
From: Renlin Li @ 2014-11-06 18:07 UTC (permalink / raw)
  To: Teresa Johnson
  Cc: Jeff Law, Dehao Chen, gcc-patches, Ramana Radhakrishnan, su

[-- Attachment #1: Type: text/plain, Size: 762 bytes --]

On 06/11/14 17:59, Teresa Johnson wrote:
> Thanks for fixing the test case. Can you also add the comment I
> suggested to the source change?
>
>> Please add a comment that this is needed due to insane incoming frequencies.
>>

Sorry, I mistakenly add it to the ChangeLog. Should be correct now.


x86_64-unknown-linux-gnu bootstrap and regression test have been done, 
no new issue.
aarch64-none-elf toolchain has been test on the model. No new regression.

gcc/ChangeLog:

2014-11-06  Renlin Li <Renlin.Li@arm.com>
     PR middle-end/61529
     * tree-ssa-threadupdate.c (compute_path_counts): Bound path_in_freq.

gcc/testsuite/ChangeLog:

2014-11-06  Renlin Li <Renlin.Li@arm.com>
     PR middle-end/61529
     * gcc.dg/pr61529.c: New.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: patch-1.diff --]
[-- Type: text/x-patch; name=patch-1.diff, Size: 1291 bytes --]

commit 35614b9b49d2478ad30225e4a3e864cd5df2e67f
Author: Renlin Li <renlin.li@arm.com>
Date:   Tue Oct 28 16:30:42 2014 +0000

    fix pr61529

diff --git a/gcc/testsuite/gcc.dg/pr61529.c b/gcc/testsuite/gcc.dg/pr61529.c
new file mode 100644
index 0000000..392239e
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr61529.c
@@ -0,0 +1,27 @@
+/* PR middle-end/61529 */
+/* { dg-do compile } */
+/* { dg-options "-O3" } */
+
+unsigned int a = 0, b = 0;
+unsigned int c;
+
+int
+main ()
+{
+  unsigned int d = 0;
+  int e[5];
+
+  for (; b < 1; b++)
+    d = 0;
+  for (; d < 1; d++)
+    a = 0;
+  for (; a < 1; a++)
+    ;
+
+  for (c = 0; c < 5; c++)
+    e[c] = 1;
+  if (e[0])
+    c = 0;
+
+  return 0;
+}
diff --git a/gcc/tree-ssa-threadupdate.c b/gcc/tree-ssa-threadupdate.c
index d2cf4de..3e20916 100644
--- a/gcc/tree-ssa-threadupdate.c
+++ b/gcc/tree-ssa-threadupdate.c
@@ -730,6 +730,11 @@ compute_path_counts (struct redirection_data *rd,
             nonpath_count += ein->count;
         }
     }
+
+  /* This is needed due to insane incoming frequencies.  */
+  if (path_in_freq > BB_FREQ_MAX)
+    path_in_freq = BB_FREQ_MAX;
+
   BITMAP_FREE (in_edge_srcs);
 
   /* Now compute the fraction of the total count coming into the first

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PING][PATCH]Partially fix PR61529, bound basic block frequency
  2014-11-06 18:07               ` Renlin Li
@ 2014-11-10 17:00                 ` Renlin Li
  2014-11-10 17:10                   ` Teresa Johnson
  2014-11-11 21:43                   ` Jeff Law
  0 siblings, 2 replies; 12+ messages in thread
From: Renlin Li @ 2014-11-10 17:00 UTC (permalink / raw)
  To: Teresa Johnson
  Cc: Jeff Law, Dehao Chen, gcc-patches, Ramana Radhakrishnan, su

On 06/11/14 18:07, Renlin Li wrote:
> On 06/11/14 17:59, Teresa Johnson wrote:
>> Thanks for fixing the test case. Can you also add the comment I
>> suggested to the source change?
>>
>>> Please add a comment that this is needed due to insane incoming 
>>> frequencies.
>>>
>
> Sorry, I mistakenly add it to the ChangeLog. Should be correct now.
>
>
> x86_64-unknown-linux-gnu bootstrap and regression test have been done, 
> no new issue.
> aarch64-none-elf toolchain has been test on the model. No new regression.
>
> gcc/ChangeLog:
>
> 2014-11-06  Renlin Li <Renlin.Li@arm.com>
>     PR middle-end/61529
>     * tree-ssa-threadupdate.c (compute_path_counts): Bound path_in_freq.
>
> gcc/testsuite/ChangeLog:
>
> 2014-11-06  Renlin Li <Renlin.Li@arm.com>
>     PR middle-end/61529
>     * gcc.dg/pr61529.c: New.

Hi,

Can anybody help to review or approve this?

Kind regards,
Renlin Li


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PING][PATCH]Partially fix PR61529, bound basic block frequency
  2014-11-10 17:00                 ` [PING][PATCH]Partially " Renlin Li
@ 2014-11-10 17:10                   ` Teresa Johnson
  2014-11-11 21:43                   ` Jeff Law
  1 sibling, 0 replies; 12+ messages in thread
From: Teresa Johnson @ 2014-11-10 17:10 UTC (permalink / raw)
  To: Renlin Li; +Cc: Jeff Law, Dehao Chen, gcc-patches, Ramana Radhakrishnan, su

Hi Renlin,

Looks like Jeff already approved it:

>Can you add a testcase please?  With a testcase, this patch is OK for the trunk.

Teresa

On Mon, Nov 10, 2014 at 8:59 AM, Renlin Li <renlin.li@arm.com> wrote:
> On 06/11/14 18:07, Renlin Li wrote:
>>
>> On 06/11/14 17:59, Teresa Johnson wrote:
>>>
>>> Thanks for fixing the test case. Can you also add the comment I
>>> suggested to the source change?
>>>
>>>> Please add a comment that this is needed due to insane incoming
>>>> frequencies.
>>>>
>>
>> Sorry, I mistakenly add it to the ChangeLog. Should be correct now.
>>
>>
>> x86_64-unknown-linux-gnu bootstrap and regression test have been done, no
>> new issue.
>> aarch64-none-elf toolchain has been test on the model. No new regression.
>>
>> gcc/ChangeLog:
>>
>> 2014-11-06  Renlin Li <Renlin.Li@arm.com>
>>     PR middle-end/61529
>>     * tree-ssa-threadupdate.c (compute_path_counts): Bound path_in_freq.
>>
>> gcc/testsuite/ChangeLog:
>>
>> 2014-11-06  Renlin Li <Renlin.Li@arm.com>
>>     PR middle-end/61529
>>     * gcc.dg/pr61529.c: New.
>
>
> Hi,
>
> Can anybody help to review or approve this?
>
> Kind regards,
> Renlin Li
>
>



-- 
Teresa Johnson | Software Engineer | tejohnson@google.com | 408-460-2413

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PING][PATCH]Partially fix PR61529, bound basic block frequency
  2014-11-10 17:00                 ` [PING][PATCH]Partially " Renlin Li
  2014-11-10 17:10                   ` Teresa Johnson
@ 2014-11-11 21:43                   ` Jeff Law
  1 sibling, 0 replies; 12+ messages in thread
From: Jeff Law @ 2014-11-11 21:43 UTC (permalink / raw)
  To: Renlin Li, Teresa Johnson
  Cc: Dehao Chen, gcc-patches, Ramana Radhakrishnan, su

On 11/10/14 09:59, Renlin Li wrote:
> On 06/11/14 18:07, Renlin Li wrote:
>> On 06/11/14 17:59, Teresa Johnson wrote:
>>> Thanks for fixing the test case. Can you also add the comment I
>>> suggested to the source change?
>>>
>>>> Please add a comment that this is needed due to insane incoming
>>>> frequencies.
>>>>
>>
>> Sorry, I mistakenly add it to the ChangeLog. Should be correct now.
>>
>>
>> x86_64-unknown-linux-gnu bootstrap and regression test have been done,
>> no new issue.
>> aarch64-none-elf toolchain has been test on the model. No new regression.
>>
>> gcc/ChangeLog:
>>
>> 2014-11-06  Renlin Li <Renlin.Li@arm.com>
>>     PR middle-end/61529
>>     * tree-ssa-threadupdate.c (compute_path_counts): Bound path_in_freq.
>>
>> gcc/testsuite/ChangeLog:
>>
>> 2014-11-06  Renlin Li <Renlin.Li@arm.com>
>>     PR middle-end/61529
>>     * gcc.dg/pr61529.c: New.
>
> Hi,
>
> Can anybody help to review or approve this?
Per my message on Nov 4, it's approved with the testcase.
jeff

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2014-11-11 21:43 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-10-29  9:35 [PATCH]Partially fix PR61529, bound basic block frequency Renlin Li
2014-10-29 12:55 ` Teresa Johnson
2014-11-03 15:29   ` Renlin Li
2014-11-04 21:59     ` Jeff Law
2014-11-06 15:09       ` Renlin Li
2014-11-06 15:38         ` Teresa Johnson
2014-11-06 17:54           ` Renlin Li
2014-11-06 17:59             ` Teresa Johnson
2014-11-06 18:07               ` Renlin Li
2014-11-10 17:00                 ` [PING][PATCH]Partially " Renlin Li
2014-11-10 17:10                   ` Teresa Johnson
2014-11-11 21:43                   ` Jeff Law

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).