[prev in list] [next in list] [prev in thread] [next in thread]
List: git
Subject: Re: pack bitmap woes on Windows
From: Johannes Sixt <j.sixt () viscovery ! net>
Date: 2014-02-13 8:07:05
Message-ID: 52FC7D29.5070900 () viscovery ! net
[Download RAW message or body]
Am 2/12/2014 17:48, schrieb Jeff King:
> 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
>
> -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);
>
>
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic