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 <p...@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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
