On 10/26/2017 02:55 PM, Eric Botcazou wrote:
> The attached Ada testcase triggers an ICE at -O3 when compiled for x86:
>
> eric@polaris:~/build/gcc/native> gcc/xgcc -Bprev-gcc -S opt68.adb -O3 -m32
> opt68.adb: In function 'Opt68.Copy':
> opt68.adb:51:6: error: multiple hot/cold transitions found (bb 34)
> opt68.adb:51:6: error: multiple hot/cold transitions found (bb 64)
> +===========================GNAT BUG DETECTED==============================+
> | 8.0.0 20171026 (experimental) [trunk revision 254096] (x86_64-suse-linux)
> GCC error:|
> | verify_flow_info failed |
>
> It's the RTL basic block reordering pass confusing itself, more precisely:
>
> /* If the best destination has multiple predecessors, and can be
> duplicated cheaper than a jump, don't allow it to be added
> to a trace. We'll duplicate it when connecting traces. */
> if (best_edge && EDGE_COUNT (best_edge->dest->preds) >= 2
> && copy_bb_p (best_edge->dest, 0))
> best_edge = NULL;
>
> Now, if you're in the first round (reordering of the hot paritition) and the
> basic block has 2 predecessors, a regular one (corresponding to best_edge)
> and
> another coming from the cold partition (i.e. EDGE_CROSSING), the duplication
> will redirect best_edge and leave the block with only one predecessor coming
> from the cold partition, which means that the block will eventually become
> cold at the end of the pass if fixup_partitions is invoked. But that's too
> late since
>
> /* Signal that rtl_verify_flow_info_1 can now verify that there
> is at most one switch between hot/cold sections. */
> crtl->bb_reorder_complete = true;
>
> So we would need to re-run the pass in order to move the block from the hot
> partition to the cold partition (re-running only a subpass wouldn't work
> since
> the above decision is made by find_traces but the duplication is triggered by
> connect_traces), which seems overkill.
>
> Therefore the attached patch detects this problematic case and doesn't reset
> best_edge upon finding it. This means that the destination will be added to
> the current trace in the hot partition, which looks OK since all its other
> predecessors are in the cold partition. It also contains a fix for an off-by-
> one bug in the dump file and removes a redundant check from better_edge_p
> (the
> same check on the BB_PARTITION of the blocks is done in find_traces_1_round).
>
> Bootstrapped/regtested on x86_64-suse-linux. Any objection?
>
>
> 2017-10-26 Eric Botcazou <ebotca...@adacore.com>
>
> * bb-reorder.c (find_traces_1_round): Fix off-by-one index.
> Move around comment. Do not reset best_edge for a copiable
> destination if the copy would cause a partition change.
> (better_edge_p): Remove redundant check.
>
>
> 2017-10-26 Eric Botcazou <ebotca...@adacore.com>
>
> * gnat.dg/opt68.ad[sb]: New test.
OK.
jeff
