On 08/04/2017 11:15 AM, Jan Hubicka wrote:
III.
I've written this patch to check for the missing probability more
consistently. I'm not certain if we can require that the probability
should always be set, so I'm just requiring that if it is set on one
outgoing edge, it needs to be set on all outgoing edges.
Sofar I've build a c-only x86_64 non-bootstrap compiler and ran dg.exp.
The only problem I ran into was in attr-simd{,-2,-4}.c. I've written a
tentative patch for that, and will submit it as follow-up.
Is this check a good idea?
I think the additional checking is a good idea. Ideally we'd verify
that all edges have a probability. Until then I think you need some
kind of rationale in a comment for why the checking is limited.
[ Not done yet. ]
I've extended the patch to skip over EH edges. I've limited the check
further (as shown in attached patch) to only check if 'EDGE_COUNT
(bb->succs) == 2'. That way I don't run into the more complicated cases
like a switch with default edge probability set to never, and all other
edges not set.
I've committed two patches for expand_oacc_for that trigger the check.
OK for trunk if bootstrap and reg-test on x86_64 succeeds?
Yea, but I'd like to see ongoing work towards full checking.
I have full checking in my tree for some time. At x86-64 bootstrap there
is one remaining offender in simd_clone_adjust which was not fixed yet
https://gcc.gnu.org/ml/gcc-patches/2017-07/msg00219.html
Jakub did not tell me what would be a reasonable guess :)
After that I plan to enable full checking after checking arm/ppc.
So I hope we will converge to full checking really soon. But having
additional check is fine.
To make sure I understand correctly: are you saying that you have in a
local tree:
- patches that add missing edge probabilities, and
- a patch that adds a check that all edges have probabilities,
and that the state of that local tree is that for x86_64 bootstrap and
reg-test there only one regression site left?
If that is the case, I'd better stop fixing sites that trigger the check
in my patch.
Thanks,
- Tom
Verify edge probability consistency in verify_flow_info
2017-08-02 Tom de Vries <t...@codesourcery.com>
* cfghooks.c (verify_flow_info): Verify that if one outgoing edge has
probability set, all outgoing edges have probability set.
---
gcc/cfghooks.c | 16 ++++++++++++++++
1 file changed, 16 insertions(+)
diff --git a/gcc/cfghooks.c b/gcc/cfghooks.c
index 18dc49a..a9af41f 100644
--- a/gcc/cfghooks.c
+++ b/gcc/cfghooks.c
@@ -152,6 +152,8 @@ verify_flow_info (void)
bb->index, bb->frequency);
err = 1;
}
+ bool has_prob_uninit_edges = false;
+ bool has_prob_init_edges = false;
FOR_EACH_EDGE (e, ei, bb->succs)
{
if (last_visited [e->dest->index] == bb)
@@ -166,6 +168,13 @@ verify_flow_info (void)
e->src->index, e->dest->index);
err = 1;
}
+ if ((e->flags & EDGE_EH) == 0)
+ {
+ if (e->probability.initialized_p ())
+ has_prob_init_edges = true;
+ else
+ has_prob_uninit_edges = true;
+ }
if (!e->count.verify ())
{
error ("verify_flow_info: Wrong count of edge %i->%i",
@@ -197,6 +206,13 @@ verify_flow_info (void)
error ("wrong amount of branch edges after unconditional jump %i", bb->index);
err = 1;
}
+ if (has_prob_uninit_edges && has_prob_init_edges
+ && EDGE_COUNT (bb->succs) == 2)
+ {
+ error ("Missing edge probability after unconditional jump in bb %i",
+ bb->index);
+ err = 1;
+ }
FOR_EACH_EDGE (e, ei, bb->preds)
{
