[llvm] r231701 - DwarfAccelTable: fix obvious typo.

[Frédéric Riss]

> On Mar 9, 2015, at 2:21 PM, David Blaikie <dblaikie at gmail.com> wrote:
> 
> 
> 
> On Mon, Mar 9, 2015 at 2:09 PM, Frederic Riss <friss at apple.com <mailto:friss at apple.com>> wrote:
> Author: friss
> Date: Mon Mar  9 16:09:50 2015
> New Revision: 231701
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=231701&view=rev <http://llvm.org/viewvc/llvm-project?rev=231701&view=rev>
> Log:
> DwarfAccelTable: fix obvious typo.
> 
> I have a test for that issue, but I didn't include it in the commit as it's
> a 200KB file for a pretty minor issue. (The reason the file is so big is
> that it needs > 1024 variables/functions to trigger and that with debug
> information.
> 
> The issue/fix on the other side is totally trivial. If poeple want the test
> commited, I can do that. It just didn't seem worth it to me.
> 
> Modified:
>     llvm/trunk/lib/CodeGen/AsmPrinter/DwarfAccelTable.cpp
> 
> Modified: llvm/trunk/lib/CodeGen/AsmPrinter/DwarfAccelTable.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/AsmPrinter/DwarfAccelTable.cpp?rev=231701&r1=231700&r2=231701&view=diff <http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/AsmPrinter/DwarfAccelTable.cpp?rev=231701&r1=231700&r2=231701&view=diff>
> ==============================================================================
> --- llvm/trunk/lib/CodeGen/AsmPrinter/DwarfAccelTable.cpp (original)
> +++ llvm/trunk/lib/CodeGen/AsmPrinter/DwarfAccelTable.cpp Mon Mar  9 16:09:50 2015
> @@ -54,7 +54,7 @@ void DwarfAccelTable::ComputeBucketCount
>    // Then compute the bucket size, minimum of 1 bucket.
>    if (num > 1024)
>      Header.bucket_count = num / 4;
> -  if (num > 16)
> +  else if (num > 16)
>      Header.bucket_count = num / 2;
> 
> 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?

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. 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.

> What was the failure mode?

The resulting table was functional[*], you’d just end up with twice the expected bucket count.

Fred

*: generally functional. In fact the implementation is totally broken when there are hash collisions, which I’m going to address next.

>    else
>      Header.bucket_count = num > 0 ? num : 1;
> 
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu <mailto:llvm-commits at cs.uiuc.edu>
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits <http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150309/0cd4fdc7/attachment.html>