git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: pack bitmap woes on Windows

From
Johannes Sixt <j.sixt@viscovery.net>
Date
Feb 13, 2014, 08:07 UTC
Message-ID
<52FC7D29.5070900@viscovery.net>
In-Reply-To
<20140212164828.GA24484@sigill.intra.peff.net>
Am 2/12/2014 17:48, schrieb Jeff King:
Show 22 quoted lines
> On Wed, Feb 12, 2014 at 03:49:24PM +0100, Erik Faye-Lund wrote:
> 
>> It seems the author of a201c20b41a2f0725977bcb89a2a66135d776ba2
>> assumes __BYTE_ORDER was guaranteed to always be defined, probably
>> fooled by the following check:
>>
>> #if !defined(__BYTE_ORDER)
>> # error "Cannot determine endianness"
>> #endif
>>
>> However, that check is only performed if we're NOT building with GCC
>> on x86/x64 nor MSVC (which I don't think defined __BYTE_ORDER either)
>>
>> The following would make __BYTE_ORDER a guarantee, and show that MinGW
>> does not define it:
> 
> Yes, that is the problem. Sorry, this got brought up earlier[1], but the
> discussion went on and I did not notice that Brian's patch did not get
> applied.
> 
> After re-reading the discussion, I think the patch below is the best
> solution. Can you confirm that it fixes the problem for you?
Yes, it fixes the problem! t5310-pack-bitmaps passes all tests with the patch.

Thanks, -- Hannes

Show 70 quoted lines
> 
> -Peff
> 
> [1] http://thread.gmane.org/gmane.comp.version-control.git/241278
> 
> -- >8 --
> Subject: ewah: unconditionally ntohll ewah data
> 
> Commit a201c20 tried to optimize out a loop like:
> 
>   for (i = 0; i < len; i++)
> 	  data[i] = ntohll(data[i]);
> 
> in the big-endian case, because we know that ntohll is a
> noop, and we do not need to pay the cost of the loop at all.
> However, it mistakenly assumed that __BYTE_ORDER was always
> defined, whereas it may not be on systems which do not
> define it by default, and where we did not need to define it
> to set up the ntohll macro. This includes OS X and Windows.
> 
> We could muck with the ordering in compat/bswap.h to make
> sure it is defined unconditionally, but it is simpler to
> still to just execute the loop unconditionally. That avoids
> the application code knowing anything about these magic
> macros, and lets it depend only on having ntohll defined.
> 
> And since the resulting loop looks like (on a big-endian
> system):
> 
>   for (i = 0; i < len; i++)
> 	  data[i] = data[i];
> 
> any decent compiler can probably optimize it out.
> 
> Original report and analysis by Brian Gernhardt.
> 
> Signed-off-by: Jeff King <peff@peff.net>
> ---
>  ewah/ewah_io.c | 10 +++-------
>  1 file changed, 3 insertions(+), 7 deletions(-)
> 
> diff --git a/ewah/ewah_io.c b/ewah/ewah_io.c
> index 4a7fae6..f7f700e 100644
> --- a/ewah/ewah_io.c
> +++ b/ewah/ewah_io.c
> @@ -113,6 +113,7 @@ int ewah_serialize(struct ewah_bitmap *self, int fd)
>  int ewah_read_mmap(struct ewah_bitmap *self, void *map, size_t len)
>  {
>  	uint8_t *ptr = map;
> +	size_t i;
>  
>  	self->bit_size = get_be32(ptr);
>  	ptr += sizeof(uint32_t);
> @@ -135,13 +136,8 @@ int ewah_read_mmap(struct ewah_bitmap *self, void *map, size_t len)
>  	memcpy(self->buffer, ptr, self->buffer_size * sizeof(uint64_t));
>  	ptr += self->buffer_size * sizeof(uint64_t);
>  
> -#if __BYTE_ORDER != __BIG_ENDIAN
> -	{
> -		size_t i;
> -		for (i = 0; i < self->buffer_size; ++i)
> -			self->buffer[i] = ntohll(self->buffer[i]);
> -	}
> -#endif
> +	for (i = 0; i < self->buffer_size; ++i)
> +		self->buffer[i] = ntohll(self->buffer[i]);
>  
>  	self->rlw = self->buffer + get_be32(ptr);
>  
> 
Previous: Jeff King
Message 11 of 11 in “pack bitmap woes on Windows”
  1. Johannes SixtFeb 12, 2014
  2. Erik Faye-LundFeb 12, 2014
  3. Johannes SixtFeb 12, 2014
  4. David KastrupFeb 12, 2014
  5. Johannes SixtFeb 12, 2014
  6. David KastrupFeb 12, 2014
  7. Duy NguyenFeb 12, 2014
  8. Erik Faye-LundFeb 12, 2014
  9. Erik Faye-LundFeb 12, 2014
  10. Jeff KingFeb 12, 2014
  11. Johannes SixtFeb 13, 2014

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.