| Message ID | 545B8F35.9070603@arm.com |
|---|---|
| State | New |
| Headers | show |
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
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