Fix PR 53743 and other -freorder-blocks-and-partition failures (issue6823047)

[Steven Bosscher]

On Fri, May 10, 2013 at 12:57 AM, Xinliang David Li wrote:
> On Thu, May 9, 2013 at 3:40 PM, Steven Bosscher wrote:

>> This patch mixes up things badly from the point of
>> what-depends-on-what, the whole approach looks wrong to me.
>
>
> Do you mean the 'source file dependency' or 'logical dependency'?
>
> If the former, the code can be easily refactored to remove the
> dependency. I don't see how the latter can be avoided as bb-partition
> etc does change cfg states and leads to different actions in cfg
> layout finalize.

I mean logical dependency. cfglayout is just a representation of the
CFG, and bb-partition is a code transformation. By making
fixup_reorder_chain emit the note, you' ve put part of the
transformation into out-of-cfglayout which is just bogus. You also
don't put GCSE or loop unrolling in out-of-cfglayout, and this change
is IMHO in the same category: mixing transformations into internal
representations. That may be a short-term fix but it is a long-term
maintenance/cleanups nightmare.

Although I've only skimmed the patch, I have noted several issues with it:

* 3 different changes put into a single patch: the
crtl->has_bb_partition change (which looks good to me), the
verification stuff, and various fixes. The patch should be submitted
in 3 parts to make/testing review easier.

* Emitting barriers in cfglayout mode. That's non-sense.

* Fixup redirected edges that did not cross partitions before but
apparently do after redirection. This is not supposed to happen in the
first place, so fixing up any of this just papers over an error
elsewhere in the compiler.

* The fixup_reorder_chain changes I've mentioned above.


Ciao!
Steven