[PATCH] D28583: CodeGen: Allow small copyable blocks to "break" the CFG.

[Chandler Carruth via llvm-commits]

On Wed, Jan 25, 2017 at 6:18 PM Xinliang David Li <davidxl at google.com>
wrote:

> On Wed, Jan 25, 2017 at 5:33 PM, Chandler Carruth <chandlerc at gmail.com>
> wrote:
>
> Just to explicitly chime in here...
>
> On Wed, Jan 25, 2017 at 4:32 PM Kyle Butt <iteratee at google.com> wrote:
>
> Why is it important to track that ?
>
>
> Chandler can back me up here, but front ends and users rely on source
> order of branches being a slight hint in the absence of other evidence.
> Going through in a second pass the only way to preserve that is to somehow
> record the order of the first pass. Stable sort just doesn't keep enough
> information.
>
>
> The idea when Andy and I started the original discussion around MBP was as
> follows:
>
> In some cases we won't have good profile signals or good CFG structure
> signals. In those cases, we should preserve source order of code for three
> reasons in decreasing order of importance:
>
>
> Static prediction heuristics use many different signals, but I am not
> aware of any using source locations. If there is such heuristic which
> proves to be useful, it
>
> 1) should be considered in branch probablity analysis
> 2) use the actual debug source location instead of CFG iterator order as
> used in Kyle's code.
>

So, I'm not really trying to re-litigate this design principle, certainly
not in the bottom of this thread. But I equally don't think this patch is
the time to change behavior. Note that the original idea was specific to
code layout, not other probability-based optimizations. So I wouldn't
expect it to impact branch probability.

But CFG iteration order is *not* what we wanted to preserve. I've not read
the patch really, sorry for that. But your other email mentioned that this
is more the order of the successor iterators, and I agree that this order
isn't interesting in any direction.

The idea was to use the order of basic blocks in the incoming machine
function as the "basis ordering". So, when tie breaking, try to pick a
block that is *already* sequenced as a successor, or pick the "nearest"
block. That was all.


>
> 1) Principle of least surprise when debugging, performance analyzing, etc.
> It is very useful if boring C-like code remains in the same basic structure
> it came in as when the compiler has no good reason to reorder it.
>
>
> For optimized build, debuggability should not be the guiding principle.
>

This is only when we have *no other signal*.


>
> 2) Reduce surprising churn of order. Basic source code edits that don't
> re-arrange the order of code shouldn't rearrange the order of generated
> code. Otherwise, innocuous changes trigger performance regressions that
> aren't interesting -- the old version got lucky and the new version gets
> unlucky. There isn't anything to do, but users dislike the surprising noise
> in their benchmarks.
>
>
> This is not necessarily true already. If a branch code condition is
> changed, it will likely trigger branch prediction to produce very different
> result.
>

Sure, but again, when we *have no signal* we don't have to make this any
worse.

All of this was about: what do we do when we don't have a *reason* to
layout the code a particular way. Clearly, if we have some reason
everything you say holds.

>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170126/82481db5/attachment.html>