From: Ramsay Jones <ramsay@ramsay1.demon.co.uk>
Date: Thu, 14 Nov 2013 23:09:40 GMT
Subject: Re: [PATCH v3 0/21] pack bitmaps
Message-ID: <52855834.3080608@ramsay1.demon.co.uk>
In-Reply-To: <20131114213320.GA16466@sigill.intra.peff.net>

On 14/11/13 21:33, Jeff King wrote:
> On Thu, Nov 14, 2013 at 07:19:38PM +0000, Ramsay Jones wrote:
> 
>> Unfortunately, I didn't find time this weekend to finish the msvc build
>> fixes. However, after a quick squint at these patches, I think you have
>> almost done it for me! :-D
>>
>> I must have misunderstood the previous discussion, because my patch was
>> written on the assumption that the ewah directory wouldn't be "git-ified"
>> (e.g. #include git-compat-util.h).
> 
> I think it was up for debate at some point, but we did decide to go
> ahead and git-ify. Please feel free to submit further fixups if you need
> them.

Yep, will do; at present it looks like that one-liner.

> 
>>>   - the ewah code used gcc's __builtin_ctzll, but did not provide a
>>>     suitable fallback. We now provide a fallback in C.
>>
>> ... here.
>>
>> I was messing around with several implementations (including the use of
>> msvc compiler intrinsics) with the intention of doing some timing tests
>> etc. [I suspected my C fallback function (a different implementation to
>> yours) would be slightly faster.]
> 
> Yeah, I looked around for several implementations, and ultimately wrote
> one that was the most readable to me. The one I found shortest and most
> inscrutable was:
> 
>   return popcount((x & -x) - 1);
> 
> the details of which I still haven't worked through in my head. ;)

Yeah, I stumbled over that one too!

> I do think on most platforms that intrinsics or inline assembler are the
> way to go. My main goal was to get something correct that would let it
> compile everywhere, and then people can use that as a base for
> optimizing. Patches welcome. :)

Indeed, I can happily leave that to another day (or to someone else
more motivated ;-)

ATB,
Ramsay Jones