<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=""><br class=""><div><blockquote type="cite" class=""><div class="">On Mar 9, 2015, at 2:56 PM, David Blaikie <<a href="mailto:dblaikie@gmail.com" class="">dblaikie@gmail.com</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><br class="Apple-interchange-newline"><br style="font-family: Helvetica; font-size: 12px; 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=""><div class="gmail_quote" style="font-family: Helvetica; font-size: 12px; 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;">On Mon, Mar 9, 2015 at 2:45 PM, 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 style="word-wrap: break-word;" class=""><br class=""><div class=""><div class=""><div class="h5"><blockquote type="cite" class=""><div class="">On Mar 9, 2015, at 2:21 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 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;" class=""><div class="gmail_quote" 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;">On Mon, Mar 9, 2015 at 2:09 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;">Author: friss<br class="">Date: Mon Mar 9 16:09:50 2015<br class="">New Revision: 231701<br class=""><br class="">URL:<span class=""> </span><a href="http://llvm.org/viewvc/llvm-project?rev=231701&view=rev" target="_blank" class="">http://llvm.org/viewvc/llvm-project?rev=231701&view=rev</a><br class="">Log:<br class="">DwarfAccelTable: fix obvious typo.<br class=""><br class="">I have a test for that issue, but I didn't include it in the commit as it's<br class="">a 200KB file for a pretty minor issue. (The reason the file is so big is<br class="">that it needs > 1024 variables/functions to trigger and that with debug<br class="">information.<br class=""><br class="">The issue/fix on the other side is totally trivial. If poeple want the test<br class="">commited, I can do that. It just didn't seem worth it to me.<br class=""><br class="">Modified:<br class=""> <span class=""> </span>llvm/trunk/lib/CodeGen/AsmPrinter/DwarfAccelTable.cpp<br class=""><br class="">Modified: llvm/trunk/lib/CodeGen/AsmPrinter/DwarfAccelTable.cpp<br class="">URL:<span class=""> </span><a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/AsmPrinter/DwarfAccelTable.cpp?rev=231701&r1=231700&r2=231701&view=diff" target="_blank" class="">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/AsmPrinter/DwarfAccelTable.cpp?rev=231701&r1=231700&r2=231701&view=diff</a><br class="">==============================================================================<br class="">--- llvm/trunk/lib/CodeGen/AsmPrinter/DwarfAccelTable.cpp (original)<br class="">+++ llvm/trunk/lib/CodeGen/AsmPrinter/DwarfAccelTable.cpp Mon Mar 9 16:09:50 2015<br class="">@@ -54,7 +54,7 @@ void DwarfAccelTable::ComputeBucketCount<br class=""> // Then compute the bucket size, minimum of 1 bucket.<br class=""> if (num > 1024)<br class=""> Header.bucket_count = num / 4;<br class="">- if (num > 16)<br class="">+ else if (num > 16)<br class=""> Header.bucket_count = num / 2;<br class=""></blockquote><div class=""><br class="">Should this whole if/else if chain just be rewritten in terms of a continuous function from num to bucket count? (which could perhaps be unit tested - perhaps the accel table in general could be unit tested & exercise this case reasonably) - looks something like "num / ((log2 num + 1) / 2)" or some such?<br class=""></div></div></div></blockquote><div class=""><br class=""></div></div></div><div class="">I found that typo because I’m using this code to emit the accelerator tables in llvm-dsymutil and comparing them to the output of the ‘reference’ implementation which is in Darwin’s dsymutil.</div></div></div></blockquote><div class=""><br class="">Worth throwing some kind of C fuzzer at the two tools & searching for cases where the output is different? (if there's an easy way to constrain the input to produce exactly matching output between the two) Dunno - just a thought.<br class=""></div></div></div></blockquote><div><br class=""></div><div>Once I upstreamed all of llvm-dsymutil, we’ll be doing bit-for-bit comparison over large codebases (everything that’s easily testable internally), it’s not fuzzing but still pretty intensive validation. For the record, I found that and the hash collision issues by testing only on the clang binaries.</div><br class=""><blockquote type="cite" class=""><div class="gmail_quote" style="font-family: Helvetica; font-size: 12px; 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;"><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 style="word-wrap: break-word;" class=""><div class=""><div class="">Both implementations had the same logic for computing the number of buckets (except for the typo in this code obviously), thus I think the intention was really to limit the division factor to 4.</div><span class=""><br class=""><blockquote type="cite" class=""><div class="gmail_quote" 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 class="">What was the failure mode?<br class=""></div></div></blockquote><div class=""><br class=""></div></span><div class="">The resulting table was functional[*], you’d just end up with twice the expected bucket count.</div></div></div></blockquote><div class=""><br class="">Thanks for the background. (well, as you go, maybe you'll find that some different/better forms of testing could be helpful - given an accelerator table should be somewhat ADT-like, I'd lean towards some unit testing, perhaps)<br class=""></div></div></blockquote><div><br class=""></div><div>I tend to agree with you, the main issue is that the output is done through the MC layer, so it is quite hard to test. Even if we’d have the ability to stream it to a buffer, it wouldn’t be easy to test. We could maybe rewrite all the logic to go to an intermediate form that would be programmatically testable… I’m really not sure it’s worth it.</div><div><br class=""></div><div>Fred</div><br class=""><blockquote type="cite" class=""><div class="gmail_quote" style="font-family: Helvetica; font-size: 12px; 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;"><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 style="word-wrap: break-word;" class=""><div class=""><div class=""><br class=""></div><div class="">Fred</div><div class=""><br class=""></div><div class="">*: generally functional. In fact the implementation is totally broken when there are hash collisions, which I’m going to address next.</div></div></div></blockquote><div class=""><br class="">Fun.<br class=""> </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 style="word-wrap: break-word;" class=""><div class=""><span class=""><br class=""><blockquote type="cite" class=""><div class="gmail_quote" 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 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;"> else<br class=""> Header.bucket_count = num > 0 ? num : 1;<br class=""><br class=""><br class="">_______________________________________________<br class="">llvm-commits mailing list<br class=""><a href="mailto:llvm-commits@cs.uiuc.edu" target="_blank" class="">llvm-commits@cs.uiuc.edu</a><br class=""><a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank" class="">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a></blockquote></div></blockquote></span></div></div></blockquote></div></blockquote></div><br class=""></body></html>