<html><head><meta http-equiv="Content-Type" content="text/html charset=utf-8"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class="">ping ?<div class=""><br class=""><div><blockquote type="cite" class=""><div class="">On Feb 12, 2015, at 11:52 AM, Frédéric Riss <<a href="mailto:friss@apple.com" class="">friss@apple.com</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><div style="font-family: Menlo-Regular; font-size: 11px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><blockquote type="cite" class=""><div class=""><br class="Apple-interchange-newline">On Feb 6, 2015, at 10:28 AM, David Blaikie <<a href="mailto:dblaikie@gmail.com" class="">dblaikie@gmail.com</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><div dir="ltr" class="">Yeah, I'm punting on the language lawyering to Richard, hopefully… </div></div></blockquote><div class=""><br class=""></div><div class="">Richard,</div><div class=""><br class=""></div><div class="">no need to look at the patch, but we just need you opinion on the following scenario:</div><div class=""><br class=""></div><div class="">What the patch does is allocate all the class DIE objects using a BumpPtrAllocator to</div><div class="">be able to bulk deallocate them. This objects contain a SmallVector that in some rare</div><div class="">cases will resort to heap allocation to store their contents. I added bookkeeping to</div><div class="">keep track of the objects that need their destructors called to free the heap allocation.</div><div class=""><br class=""></div><div class="">Basically the code is</div><div class="">std::vector<DIE *> DIEsToDelete</div><div class="">…</div><div class="">for (auto *DIE : DIEsToDelete)</div><div class=""> DIE->~DIE();</div><div class=""><br class=""></div><div class="">There is a catch though. Some of these objects are of type DIEBlock or DIELoc which</div><div class="">use an identical multiple inheritance scheme:</div><div class=""><br class=""></div><div class="">class DIEBlock : public DIEValue, public DIE {</div><div class="">…</div><div class="">};</div><div class=""><br class=""></div><div class="">I’m storing a pointer to these in the above list and thus invoking just the DIE </div><div class="">destructor on them. The objects aren’t used afterwards as the next step is to</div><div class="">free the memory allocated by the BumpPtrAllocator.</div><div class=""><br class=""></div><div class="">David was worried that this might be UB. Any definitive opinion?</div><div class=""><br class=""></div><div class="">Fred</div><br class=""><blockquote type="cite" class=""><div class=""><div dir="ltr" class=""><div class="gmail_extra"><div class="gmail_quote">On Thu, Feb 5, 2015 at 9:25 AM, Frédéric Riss<span class="Apple-converted-space"> </span><span dir="ltr" class=""><<a href="mailto:friss@apple.com" target="_blank" class="">friss@apple.com</a>></span><span class="Apple-converted-space"> </span>wrote:<br class=""><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 class="" style="word-wrap: break-word;">ping?<div class=""><br class=""><div class=""><blockquote type="cite" class=""><div class=""><div class="h5"><div class="">On Jan 27, 2015, at 6:43 PM, Frédéric Riss <<a href="mailto:friss@apple.com" target="_blank" class="">friss@apple.com</a>> wrote:</div><br class=""></div></div><div class=""><div class=""><div class="h5"><div class="" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px;"><blockquote type="cite" class=""><div class=""><br class="">On Jan 27, 2015, at 4:11 PM, David Blaikie <<a href="mailto:dblaikie@gmail.com" target="_blank" class="">dblaikie@gmail.com</a>> wrote:</div><br class=""><div class=""><br class=""><br class="" style="font-family: Menlo-Regular; font-size: 11px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px;"><div class="gmail_quote" style="font-family: Menlo-Regular; font-size: 11px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px;">On Tue, Jan 27, 2015 at 2:08 PM, Frederic Riss<span class=""> </span><span dir="ltr" class=""><<a href="mailto:friss@apple.com" target="_blank" class="">friss@apple.com</a>></span><span class=""> </span>wrote:<br class=""><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;">This update removes the limitation to a fixed maximum number of<br class="">attributes per DIE. The patch is very similar to the first one that<br class="">I sent for discussion. It only adds a pointer to a vector of DIEs<br class="">as arguement to DIE::addValue(). If a DIE overflows its inline storage<br class="">for attributes, it is added to the vector. The vector is then iterated<br class="">just before the BumpPtrAllocator is destroyed to call the destructors<br class="">of all these DIEs.<br class=""><br class="">This approach gives the most flexibilty as it doesn't impose the<br class="">memory management upon the user (for example DIEHashTest.cpp continues<br class="">to use stack allocated DIEs and it works fine).<br class=""><br class="">I've been playing with this for the last few days, and I have quite<br class="">a few different implementations lying around (for example using<br class="">std::vectors with a custom stateful allocator). They are all more<br class="">complicated and don't perform as well as this patch.<br class=""><br class="">There's one point that I'd like to mention: the patch unifies the<br class="">existing bookkeeping of the DIEBlock and DIELoc objects and treats<br class="">them the same as standard DIEs. This means that we will call ~DIE<br class="">on DIEBlock and DIELoc objects which will result in a 'partial'<br class="">destruction (DIEBlock inherits both from DIEValue and from DIE).<br class="">I think this is fine. It should do exactly what we want and just<br class="">call the destructors of the SmallVectors in the DIE part of the<br class="">object, but I wanted to mention it in case someone thinks its not<br class="">legal to do that.<br class=""></blockquote><div class=""><br class="">Pretty sure that's not valid C++ (not sure quite which cracks the bump allocator objects fall in general in terms of not running dtors, reusing memory, etc - and whether this would be worse than that or not, though).<br class=""></div></div></div></blockquote><div class=""><br class=""></div><div class="">What exactly do you think the issue would be from a language standpoint? Calling a destructor on a ‘partial’ object? I would expect that it is ok to do so. AFAIK, explicit destructor calls follow the same rules as other function calls. I have a pointer to a DIE object, thus I can call the destructor on it. The object stops to ‘exist’ at that point, and thus its enclosing object is in an undefined state, but this doesn’t really matter as it won’t be touched anymore.</div><div class=""><br class=""></div><div class="">I’m very bad at language lawyering though, thus I’d really appreciate a authoritative answer :-)</div><div class=""><br class=""></div><div class="">Fred </div><div class=""><br class=""></div><blockquote type="cite" class=""><div class=""><div class="gmail_quote" style="font-family: Menlo-Regular; font-size: 11px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px;"><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 class=""><div class=""><br class=""><br class=""><a href="http://reviews.llvm.org/D7072" target="_blank" class="">http://reviews.llvm.org/D7072</a><br class=""><br class="">Files:<br class=""> <span class=""> </span>include/llvm/CodeGen/DIE.h<br class=""> <span class=""> </span>lib/CodeGen/AsmPrinter/DIE.cpp<br class=""> <span class=""> </span>lib/CodeGen/AsmPrinter/DIEHash.cpp<br class=""> <span class=""> </span>lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp<br class=""> <span class=""> </span>lib/CodeGen/AsmPrinter/DwarfCompileUnit.h<br class=""> <span class=""> </span>lib/CodeGen/AsmPrinter/DwarfDebug.cpp<br class=""> <span class=""> </span>lib/CodeGen/AsmPrinter/DwarfFile.cpp<br class=""> <span class=""> </span>lib/CodeGen/AsmPrinter/DwarfUnit.cpp<br class=""> <span class=""> </span>lib/CodeGen/AsmPrinter/DwarfUnit.h<br class=""> <span class=""> </span>unittests/CodeGen/DIEHashTest.cpp<br class=""><br class="">EMAIL PREFERENCES<br class=""> <span class=""> </span><a href="http://reviews.llvm.org/settings/panel/emailpreferences/" target="_blank" class="">http://reviews.llvm.org/settings/panel/emailpreferences/</a></div></div></blockquote></div></div></blockquote></div><br class="" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px;"></div></div><span class=""><span class="" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; float: none; display: inline !important;">_______________________________________________</span><br class="" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px;"><span class="" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; float: none; display: inline !important;">llvm-commits mailing list</span><br class="" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px;"><a href="mailto:llvm-commits@cs.uiuc.edu" target="_blank" class="" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px;">llvm-commits@cs.uiuc.edu</a><br class="" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px;"><a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank" class="" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px;">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a></span></div></blockquote></div><br class=""></div></div></blockquote></div><br class=""></div></div></div></blockquote></div><br class="" style="font-family: Menlo-Regular; font-size: 11px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;"><span style="font-family: Menlo-Regular; font-size: 11px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">_______________________________________________</span><br style="font-family: Menlo-Regular; font-size: 11px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Menlo-Regular; font-size: 11px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">llvm-commits mailing list</span><br style="font-family: Menlo-Regular; font-size: 11px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><a href="mailto:llvm-commits@cs.uiuc.edu" style="font-family: Menlo-Regular; font-size: 11px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class="">llvm-commits@cs.uiuc.edu</a><br style="font-family: Menlo-Regular; font-size: 11px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" style="font-family: Menlo-Regular; font-size: 11px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class="">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a></div></blockquote></div><br class=""></div></body></html>