git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] Use faster byte swapping when compiling with MSVC
@ 2009-10-19 16:37 Sebastian Schuberth
  2009-10-20  7:04 ` Junio C Hamano
  2009-10-20 12:20 ` Ludvig Strigeus
  0 siblings, 2 replies; 7+ messages in thread
From: Sebastian Schuberth @ 2009-10-19 16:37 UTC (permalink / raw)
  To: git

When compiling with MSVC on x86-compatible, use an intrinsic for byte swapping.
In contrast to the GCC path, we do not prefer inline assembly here as it is not
supported for the x64 platform.

Signed-off-by: Sebastian Schuberth <sschuberth@gmail.com>
---
 compat/bswap.h |   10 ++++++++++
 1 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/compat/bswap.h b/compat/bswap.h
index 5cc4acb..279e0b4 100644
--- a/compat/bswap.h
+++ b/compat/bswap.h
@@ -28,6 +28,16 @@ static inline uint32_t default_swab32(uint32_t val)
 	} \
 	__res; })
 
+#elif defined(_MSC_VER) && (defined(_M_IX86) || defined(_M_X64))
+
+#include <stdlib.h>
+
+#define bswap32(x) _byteswap_ulong(x)
+
+#endif
+
+#ifdef bswap32
+
 #undef ntohl
 #undef htonl
 #define ntohl(x) bswap32(x)
-- 
1.6.5.rc2.13.g1be2

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH] Use faster byte swapping when compiling with MSVC
  2009-10-19 16:37 [PATCH] Use faster byte swapping when compiling with MSVC Sebastian Schuberth
@ 2009-10-20  7:04 ` Junio C Hamano
  2009-10-20  8:56   ` Sebastian Schuberth
  2009-10-20 12:20 ` Ludvig Strigeus
  1 sibling, 1 reply; 7+ messages in thread
From: Junio C Hamano @ 2009-10-20  7:04 UTC (permalink / raw)
  To: Sebastian Schuberth
  Cc: git, Marius Storm-Olsen, Johannes Sixt, Johannes Schindelin

Sebastian Schuberth <sschuberth@gmail.com> writes:

> When compiling with MSVC on x86-compatible, use an intrinsic for byte swapping.
> In contrast to the GCC path, we do not prefer inline assembly here as it is not
> supported for the x64 platform.
>
> Signed-off-by: Sebastian Schuberth <sschuberth@gmail.com>

Unlike the other one this is not Acked by Marius, Dscho, or J6t; should I
pick this up myself, or should I wait to be fed by one of msysgit people?


> ---
>  compat/bswap.h |   10 ++++++++++
>  1 files changed, 10 insertions(+), 0 deletions(-)
>
> diff --git a/compat/bswap.h b/compat/bswap.h
> index 5cc4acb..279e0b4 100644
> --- a/compat/bswap.h
> +++ b/compat/bswap.h
> @@ -28,6 +28,16 @@ static inline uint32_t default_swab32(uint32_t val)
>  	} \
>  	__res; })
>  
> +#elif defined(_MSC_VER) && (defined(_M_IX86) || defined(_M_X64))
> +
> +#include <stdlib.h>
> +
> +#define bswap32(x) _byteswap_ulong(x)
> +
> +#endif
> +
> +#ifdef bswap32
> +
>  #undef ntohl
>  #undef htonl
>  #define ntohl(x) bswap32(x)
> -- 
> 1.6.5.rc2.13.g1be2

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] Use faster byte swapping when compiling with MSVC
  2009-10-20  7:04 ` Junio C Hamano
@ 2009-10-20  8:56   ` Sebastian Schuberth
  2009-10-20  9:53     ` Johannes Schindelin
  0 siblings, 1 reply; 7+ messages in thread
From: Sebastian Schuberth @ 2009-10-20  8:56 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: git, Marius Storm-Olsen, Johannes Sixt, Johannes Schindelin

On Tue, Oct 20, 2009 at 09:04, Junio C Hamano <gitster@pobox.com> wrote:

>> When compiling with MSVC on x86-compatible, use an intrinsic for byte swapping.
>> In contrast to the GCC path, we do not prefer inline assembly here as it is not
>> supported for the x64 platform.
>>
>> Signed-off-by: Sebastian Schuberth <sschuberth@gmail.com>
>
> Unlike the other one this is not Acked by Marius, Dscho, or J6t; should I
> pick this up myself, or should I wait to be fed by one of msysgit people?

Well, in fact I am one of the msysgit poeple, although I mostly worked
on the installer until now. In general, I like my patches to be
reviewed, but this one is rather uncritical, I guess. So it's up to
you, Junio, I'm perfectly OK with waiting for an ACK.

-- 
Sebastian Schuberth

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] Use faster byte swapping when compiling with MSVC
  2009-10-20  8:56   ` Sebastian Schuberth
@ 2009-10-20  9:53     ` Johannes Schindelin
  2009-10-20 16:55       ` Junio C Hamano
  0 siblings, 1 reply; 7+ messages in thread
From: Johannes Schindelin @ 2009-10-20  9:53 UTC (permalink / raw)
  To: Sebastian Schuberth
  Cc: Junio C Hamano, git, Marius Storm-Olsen, Johannes Sixt

Hi,

On Tue, 20 Oct 2009, Sebastian Schuberth wrote:

> On Tue, Oct 20, 2009 at 09:04, Junio C Hamano <gitster@pobox.com> wrote:
> 
> >> When compiling with MSVC on x86-compatible, use an intrinsic for byte 
> >> swapping. In contrast to the GCC path, we do not prefer inline 
> >> assembly here as it is not supported for the x64 platform.
> >>
> >> Signed-off-by: Sebastian Schuberth <sschuberth@gmail.com>
> >
> > Unlike the other one this is not Acked by Marius, Dscho, or J6t; 
> > should I pick this up myself, or should I wait to be fed by one of 
> > msysgit people?
> 
> Well, in fact I am one of the msysgit poeple, although I mostly worked 
> on the installer until now. In general, I like my patches to be 
> reviewed, but this one is rather uncritical, I guess. So it's up to you, 
> Junio, I'm perfectly OK with waiting for an ACK.

Apart from the fact that I do not have MSVC (and I don't want it, either), 
there is another strong reason why I think Sebastian does not need ACKs or 
SOBs on MSVC patches: he has plenty of experience as a maintainer of a 
rather big (commercial) software that has to compile on Windows, MacOSX 
and several Unix-type OSes (and it is known that Sebastian is a Windows 
guy).

So I would trust Sebastian's patches (at least when it comes to MSVC) 
without even reviewing them.

Ciao,
Dscho

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] Use faster byte swapping when compiling with MSVC
  2009-10-19 16:37 [PATCH] Use faster byte swapping when compiling with MSVC Sebastian Schuberth
  2009-10-20  7:04 ` Junio C Hamano
@ 2009-10-20 12:20 ` Ludvig Strigeus
  2009-10-20 12:44   ` Erik Faye-Lund
  1 sibling, 1 reply; 7+ messages in thread
From: Ludvig Strigeus @ 2009-10-20 12:20 UTC (permalink / raw)
  To: git

Sebastian Schuberth <sschuberth <at> gmail.com> writes:
> 
> When compiling with MSVC on x86-compatible, use an intrinsic for byte swapping.
> In contrast to the GCC path, we do not prefer inline assembly here as it is not
> supported for the x64 platform.
> 
>  
> +#elif defined(_MSC_VER) && (defined(_M_IX86) || defined(_M_X64))
> +
> +#include <stdlib.h>
> +
> +#define bswap32(x) _byteswap_ulong(x)
> +
> +#endif

unsigned long (as used by _byteswap_ulong) is 64 bits on x64, right? Then it
doesn't make sense to use _byteswap_ulong to swap 32-bit quantities (assuming
that's what bswap32 does) would it?

/Ludde

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] Use faster byte swapping when compiling with MSVC
  2009-10-20 12:20 ` Ludvig Strigeus
@ 2009-10-20 12:44   ` Erik Faye-Lund
  0 siblings, 0 replies; 7+ messages in thread
From: Erik Faye-Lund @ 2009-10-20 12:44 UTC (permalink / raw)
  To: Ludvig Strigeus; +Cc: git

On Tue, Oct 20, 2009 at 2:20 PM, Ludvig Strigeus <ludde@spotify.com> wrote:
> unsigned long (as used by _byteswap_ulong) is 64 bits on x64, right? Then it
> doesn't make sense to use _byteswap_ulong to swap 32-bit quantities (assuming
> that's what bswap32 does) would it?

No, unsigned long is 32bit on x64 on Windows.

--->8---
#include <stdio.h>
#include <stdlib.h>

int main(int argc, char *argv[])
{
  printf("sizeof(void *) = %d\n", sizeof(void *));
  printf("sizeof(unsigned long) = %d\n", sizeof(unsigned long));
  return 0;
}
--->8---
sizeof(void *) = 8
sizeof(unsigned long) = 4

-- 
Erik "kusma" Faye-Lund

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] Use faster byte swapping when compiling with MSVC
  2009-10-20  9:53     ` Johannes Schindelin
@ 2009-10-20 16:55       ` Junio C Hamano
  0 siblings, 0 replies; 7+ messages in thread
From: Junio C Hamano @ 2009-10-20 16:55 UTC (permalink / raw)
  To: Johannes Schindelin
  Cc: Sebastian Schuberth, Junio C Hamano, git, Marius Storm-Olsen,
	Johannes Sixt

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

>> Well, in fact I am one of the msysgit poeple, although I mostly worked 
>> on the installer until now. In general, I like my patches to be 
>> reviewed, but this one is rather uncritical, I guess. So it's up to you, 
>> Junio, I'm perfectly OK with waiting for an ACK.
>
> Apart from the fact that I do not have MSVC (and I don't want it, either), 
> there is another strong reason why I think Sebastian does not need ACKs or 
> SOBs on MSVC patches: he has plenty of experience as a maintainer of a 
> rather big (commercial) software that has to compile on Windows, MacOSX 
> and several Unix-type OSes (and it is known that Sebastian is a Windows 
> guy).
>
> So I would trust Sebastian's patches (at least when it comes to MSVC) 
> without even reviewing them.

I very appreciate a strong Ack in a specific area like this.  I do skim
msysgit list from time to time, and in retrospect I realize I _could_ have
recognized Sebastian's name but somehow it didn't click.

I guess I should apply both patches to 'master', then.  Thanks.

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2009-10-20 16:56 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-10-19 16:37 [PATCH] Use faster byte swapping when compiling with MSVC Sebastian Schuberth
2009-10-20  7:04 ` Junio C Hamano
2009-10-20  8:56   ` Sebastian Schuberth
2009-10-20  9:53     ` Johannes Schindelin
2009-10-20 16:55       ` Junio C Hamano
2009-10-20 12:20 ` Ludvig Strigeus
2009-10-20 12:44   ` Erik Faye-Lund

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).