<div dir="ltr">and can you check the existing uses?<div><br></div></div><div class="gmail_extra"><br><div class="gmail_quote">On Mon, Mar 14, 2016 at 12:55 PM, Justin Bogner via llvm-commits <span dir="ltr"><<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">escha <<a href="mailto:escha@apple.com">escha@apple.com</a>> writes:<br>
> escha created this revision.<br>
> escha added reviewers: bogner, resistor.<br>
> escha added a subscriber: llvm-commits.<br>
> escha set the repository for this revision to rL LLVM.<br>
><br>
> In some places, like InstCombine, we resize a DenseMap to fit the<br>
> elements we intend to put in it, then insert those elements (to avoid<br>
> continual reallocations as it grows). But .resize(foo) doesn't<br>
> actually do what people think; it resizes to foo buckets (which is<br>
> really an implementation detail the user of DenseMap probably<br>
> shouldn't care about), not the space required to fit foo<br>
> elements. DenseMap grows if 3/4 of its buckets are full, so this<br>
> actually causes one forced reallocation every time instead of avoiding<br>
> a reallocation.<br>
><br>
> This patch makes .resize(foo) do the intuitive thing: it grows to the<br>
> size necessary to fit foo elements without new allocations.<br>
<br>
</span>Could you add a unittest for this please? You can probably find<br>
something similar to base it off of in unittests/ADT/DenseMapTest.cpp.<br>
<span class=""><br>
> Repository:<br>
>   rL LLVM<br>
><br>
> <a href="http://reviews.llvm.org/D18154" rel="noreferrer" target="_blank">http://reviews.llvm.org/D18154</a><br>
><br>
> Files:<br>
>   include/llvm/ADT/DenseMap.h<br>
><br>
> Index: include/llvm/ADT/DenseMap.h<br>
> ===================================================================<br>
> --- include/llvm/ADT/DenseMap.h<br>
> +++ include/llvm/ADT/DenseMap.h<br>
> @@ -81,8 +81,11 @@<br>
>    }<br>
>    unsigned size() const { return getNumEntries(); }<br>
><br>
> -  /// Grow the densemap so that it has at least Size buckets. Does not shrink<br>
> +  /// Grow the densemap so that it can contain at least Size items before<br>
> +  /// resizing again. This means somewhat more than Size buckets because<br>
> +  /// densemap resizes upon reaching 3/4 full.<br>
>    void resize(size_type Size) {<br>
> +    Size = (Size * 4 + 2) / 3;<br>
<br>
</span>Why +2? This probably deserves a comment.<br>
<span class=""><br>
>      incrementEpoch();<br>
>      if (Size > getNumBuckets())<br>
>        grow(Size);<br>
</span>_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@lists.llvm.org">llvm-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a><br>
</blockquote></div><br></div>