[PATCH] D22414: [BranchProbabilityInfo] Change analysis result storage to simplify basic block removal

[Xinliang David Li via llvm-commits]

On Thu, Jul 21, 2016 at 6:13 AM, Igor Laevsky <igor at azulsystems.com> wrote:

>
> How about some stats about the number of eraseBB calls from JumpThreading?
>
>
> I think this number will greatly depend on the nature of the workload.
> Theoretically, JumpThreading calls eraseBlock O(number of bb) times. When
> eraseBlock has linear complexity we will get O(n^2) overall operations.
>


Understand, but it is still theoretical.

Another concern with this change is that it can actually increase the
overhead in normal paths:  single level map becomes two level -- the lookup
time may increase. Also the value type for the DenseMap becomes IndexedMap
which is based on SmallVector. It increases the densesMap keyvalue pair
size a lot. Note that densemap is an open addressing hash table so
increased value type size can result in more wasted space. The overhead of
resizing will also increase.

I suggest postponing this patch until we see a real compile time regression
due to eraseBB in the wild.


David


> An alternative way of fixing the problem which requires slight change in
> interface -- the erase interface need to pass in number of successors of
> the 'BB' before it is erased. in BPI, you can form the 'Edge' keys from
> 'BB' and index and erase them.
>
>
> At the time when we remove basic block it can have different number of
> successors (sometimes zero). But we want to remove all references to the
> basic block from the analysis map. Otherwise we will end up having same
> dangling pointer problem (https://reviews.llvm.org/D20957)
>
>
> David
>
> On Wed, Jul 20, 2016 at 9:51 AM, Igor Laevsky <igor at azulsystems.com>
> wrote:
>
>> igor-laevsky added a comment.
>>
>> Hi,
>>
>> I don't have specific build times. This is mostly motivated by the review
>> comments in the https://reviews.llvm.org/D20957. Evidence for importance
>> of this is that LVI also had linear time EraseBlock and it caused problems
>> there. BPI will call EraseBlock exactly as frequently as LVI did, so it's
>> natural to assume that performance bottleneck will be in the same place.
>>
>>
>> https://reviews.llvm.org/D22414
>>
>>
>>
>>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160721/6a5ce24e/attachment.html>