<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Mon, Mar 9, 2015 at 2:45 PM, Frédéric Riss <span dir="ltr"><<a href="mailto:friss@apple.com" target="_blank">friss@apple.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word"><br><div><div><div class="h5"><blockquote type="cite"><div>On Mar 9, 2015, at 2:21 PM, David Blaikie <<a href="mailto:dblaikie@gmail.com" target="_blank">dblaikie@gmail.com</a>> wrote:</div><br><div><br><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"><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> </span><span dir="ltr"><<a href="mailto:friss@apple.com" target="_blank">friss@apple.com</a>></span><span> </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">Author: friss<br>Date: Mon Mar  9 16:09:50 2015<br>New Revision: 231701<br><br>URL:<span> </span><a href="http://llvm.org/viewvc/llvm-project?rev=231701&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=231701&view=rev</a><br>Log:<br>DwarfAccelTable: fix obvious typo.<br><br>I have a test for that issue, but I didn't include it in the commit as it's<br>a 200KB file for a pretty minor issue. (The reason the file is so big is<br>that it needs > 1024 variables/functions to trigger and that with debug<br>information.<br><br>The issue/fix on the other side is totally trivial. If poeple want the test<br>commited, I can do that. It just didn't seem worth it to me.<br><br>Modified:<br>   <span> </span>llvm/trunk/lib/CodeGen/AsmPrinter/DwarfAccelTable.cpp<br><br>Modified: llvm/trunk/lib/CodeGen/AsmPrinter/DwarfAccelTable.cpp<br>URL:<span> </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">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/AsmPrinter/DwarfAccelTable.cpp?rev=231701&r1=231700&r2=231701&view=diff</a><br>==============================================================================<br>--- llvm/trunk/lib/CodeGen/AsmPrinter/DwarfAccelTable.cpp (original)<br>+++ llvm/trunk/lib/CodeGen/AsmPrinter/DwarfAccelTable.cpp Mon Mar  9 16:09:50 2015<br>@@ -54,7 +54,7 @@ void DwarfAccelTable::ComputeBucketCount<br>   // Then compute the bucket size, minimum of 1 bucket.<br>   if (num > 1024)<br>     Header.bucket_count = num / 4;<br>-  if (num > 16)<br>+  else if (num > 16)<br>     Header.bucket_count = num / 2;<br></blockquote><div><br>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></div></div></div></blockquote><div><br></div></div></div><div>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><br>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> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word"><div><div> 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><blockquote type="cite"><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>What was the failure mode?<br></div></div></blockquote><div><br></div></span><div>The resulting table was functional[*], you’d just end up with twice the expected bucket count.</div></div></div></blockquote><div><br>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> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word"><div><div><br></div><div>Fred</div><div><br></div><div>*: 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><br>Fun.<br> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word"><div><span class=""><br><blockquote type="cite"><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>     Header.bucket_count = num > 0 ? num : 1;<br><br><br>_______________________________________________<br>llvm-commits mailing list<br><a href="mailto:llvm-commits@cs.uiuc.edu" target="_blank">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></blockquote></div></blockquote></span></div><br></div></blockquote></div><br></div></div>