<div dir="ltr"><div class="gmail_extra"><br><div class="gmail_quote">On Thu, Jan 8, 2015 at 4:38 PM, Chandler Carruth <span dir="ltr"><<a href="mailto:chandlerc@google.com" target="_blank">chandlerc@google.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"><div dir="ltr"><div class="gmail_extra"><span class=""><br><div class="gmail_quote">On Thu, Jan 8, 2015 at 3:59 PM, Duncan P. N. Exon Smith <span dir="ltr"><<a href="mailto:dexonsmith@apple.com" target="_blank">dexonsmith@apple.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"><span>> On 2015-Jan-08, at 15:56, Chandler Carruth <<a href="mailto:chandlerc@google.com" target="_blank">chandlerc@google.com</a>> wrote:<br>
><br>
> After arguing about this for like an hour with Richard, David, and Alexey, I think we have a better path forward...<br>
><br>
> Much as you suggest, the key is nuking the upcast in the asserts build. But we can fix a number of other things in the process. For example, it is really annoying that we construct ValueHandle objects for the empty and tombstone keys. Instead, it would be better if we could not construct an object at all, and it turns out there is a nice way to do this.<br>
<br>
</span>Colour me interested...<br>
<span><br>
> Unfortunately, it requires some refactoring to DenseMap and DenseMapInfo to expose this freedom. I'm going to do that refactoring (which seems like generally good cleanup), and then we can look at a more direct way of fixing the bug here.<br>
<br>
</span>SGTM!</blockquote></div><br></span>Bleh. Never mind. Doesn't work. Removing the PointerIntPair still helps, and I'll submit the obvious patch to do that shortly.</div><div class="gmail_extra"><br></div><div class="gmail_extra">We add a private constructor to AssertingVH that accepts a raw Value* (and in NDEBUG, store a raw Value*), sinking all the casting into methods. We ensure the ValueHandleBase can also be constructed for a raw Value* with no casting. Then we hand in DenseMapInfo<Value *>::getEmptyKey() and DenseMapInfo<Value *>::getTombstoneKey(). No need to do any other funny business I think.</div></div></blockquote><div><br></div><div>Alright, see <a href="http://reviews.llvm.org/D6903">http://reviews.llvm.org/D6903</a></div><div> </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"><div dir="ltr"><div class="gmail_extra"><br></div><div class="gmail_extra">At some point, I would really like to also get rid of the shifts in DenseMapInfo<T*>, but I don't think we need to solve that problem today.</div></div>
<br>_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
<br></blockquote></div><br><br clear="all"><div><br></div>-- <br><div class="gmail_signature"><div dir="ltr">Alexey Samsonov<br><a href="mailto:vonosmas@gmail.com" target="_blank">vonosmas@gmail.com</a></div></div>
</div></div>