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

[Xinliang David Li via llvm-commits]

On Wed, Jan 25, 2017 at 6:33 PM, Chandler Carruth <chandlerc at gmail.com>
wrote:

> 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.
>

BP and layout are highly correlated, but I agree this is not the thread
discussing that.


>
> 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.
>

Right.  I was referring to this particular case as well.

thanks,

David

>
> 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/20170125/e78452f6/attachment.html>