<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=us-ascii">
</head>
<body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;">
<br>
<div>
<blockquote type="cite">
<div dir="ltr">
<div class="gmail_extra">
<div class="gmail_quote">On Thu, Jul 21, 2016 at 6:13 AM, Igor Laevsky <span dir="ltr">
<<a href="mailto:igor@azulsystems.com" target="_blank">igor@azulsystems.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin: 0px 0px 0px 0.8ex; border-left-width: 1px; border-left-color: rgb(204, 204, 204); border-left-style: solid; padding-left: 1ex; position: static; z-index: auto;">
<div style="word-wrap:break-word"><br>
<div><span class="">
<blockquote type="cite">
<div dir="ltr">How about some stats about the number of eraseBB calls from JumpThreading?</div>
</blockquote>
<div><br>
</div>
</span>
<div>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.</div>
</div>
</div>
</blockquote>
<div><br>
</div>
<div><br>
</div>
<div>Understand, but it is still theoretical. </div>
<div><br>
</div>
<div>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.</div>
<div><br>
</div>
<div>I suggest postponing this patch until we see a real compile time regression due to eraseBB in the wild.</div>
</div>
</div>
</div>
</blockquote>
<div><br>
</div>
<div>Ok, makes sense</div>
<br>
<blockquote type="cite">
<div dir="ltr">
<div class="gmail_extra">
<div class="gmail_quote">
<div><br>
</div>
<div> </div>
<div>David</div>
<div><br>
</div>
<blockquote class="gmail_quote" style="margin: 0px 0px 0px 0.8ex; border-left-width: 1px; border-left-color: rgb(204, 204, 204); border-left-style: solid; padding-left: 1ex; position: static; z-index: auto;">
<div style="word-wrap:break-word">
<div><span class=""><br>
<blockquote type="cite">
<div dir="ltr">
<div>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.</div>
</div>
</blockquote>
<div><br>
</div>
</span>
<div>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 (<a href="https://reviews.llvm.org/D20957" target="_blank">https://reviews.llvm.org/D20957</a>)</div>
<span class=""><br>
<blockquote type="cite">
<div dir="ltr">
<div><br>
</div>
<div>David</div>
</div>
<div class="gmail_extra"><br>
<div class="gmail_quote">On Wed, Jul 20, 2016 at 9:51 AM, Igor Laevsky <span dir="ltr">
<<a href="mailto:igor@azulsystems.com" target="_blank">igor@azulsystems.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin: 0px 0px 0px 0.8ex; border-left-width: 1px; border-left-color: rgb(204, 204, 204); border-left-style: solid; padding-left: 1ex; position: static; z-index: auto;">
igor-laevsky added a comment.<br>
<br>
Hi,<br>
<br>
I don't have specific build times. This is mostly motivated by the review comments in the
<a href="https://reviews.llvm.org/D20957" rel="noreferrer" target="_blank">https://reviews.llvm.org/D20957</a>. 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.<br>
<br>
<br>
<a href="https://reviews.llvm.org/D22414" rel="noreferrer" target="_blank">https://reviews.llvm.org/D22414</a><br>
<br>
<br>
<br>
</blockquote>
</div>
<br>
</div>
</blockquote>
</span></div>
<br>
</div>
</blockquote>
</div>
<br>
</div>
</div>
</blockquote>
</div>
<br>
</body>
</html>