On 08/02/2017 10:07 AM, Tom de Vries wrote:
> Hi,
>
> I.
>
> for target nvptx we recently ran into PR81442, an ICE in verify_flow_info:
> ...
> error: verify_flow_info: REG_BR_PROB is set but cfg probability is not
> ...
>
> We start out with a jump instruction:
> ...
> (jump_insn 18 17 31 2 (set (pc)
> (if_then_else (ne (reg:BI 83)
> (const_int 0 [0]))
> (label_ref 31)
> (pc))) "reduction-5.c":52 104 {br_true}
> (expr_list:REG_DEAD (reg:BI 83)
> (int_list:REG_BR_PROB 10000 (nil)))
> -> 31)
> ...
>
> The probability is set on the branch edge, but not on the fallthru edge.
>
> After fixup_reorder_chain, the condition is reversed, and the
> probability reg-note update accordingly:
> ...
> (jump_insn 18 17 32 2 (set (pc)
> (if_then_else (eq (reg:BI 83)
> (const_int 0 [0]))
> (label_ref:DI 33)
> (pc))) "reduction-5.c":52 105 {br_false}
> (expr_list:REG_DEAD (reg:BI 83)
> (int_list:REG_BR_PROB 0 (nil)))
> -> 33)
> ...
>
> The branch and fallthru edge are also reversed, which means now the
> probability is set on the fallthru edge, and not on the branch edge.
>
> This is the immediate cause for the verify_flow_info error.
>
>
> II.
>
> We've fixed the ICE in the target by setting the probability on the
> fallthru edge when we introduce it (r250422).
>
> We've noted other places where we were forgetting to set the probability
> (fixed in r250823), but that did not trigger the ICE.
>
>
> 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.
>
> OK for trunk if bootstrap and reg-test on x86_64 succeeds?
Yea, but I'd like to see ongoing work towards full checking.
Jeff
