patch, 4 partsmsvc: Select the "fast" definition of the {get,put}_be32() macros

On Intel machines, the msvc compiler defines the CPU architecture macros _M_IX86 and _M_X64 (equivalent to __i386__ and __x86_64__ respectively). Use these macros in the pre-processor expression to select the "fast" definition of the {get,put}_be32() macros.

Signed-off-by: Ramsay Jones <ramsay@ramsay1.demon.co.uk>
---

I did consider adding an additional guard, like so:

    (defined(_MSC_VER) && (defined(_M_IX86) || defined(_M_X64))) || \

However, I decided that the extra paranoia was not needed ...

 block-sha1/sha1.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

Ramsay Jones wrote:

Looks good to me, for what it’s worth. You might want a similar change on line 57:

 
-#if defined(__i386__) || defined(__x86_64__)
+#if defined(__i386__) || defined(__x86_64__) || \
+	defined(_M_IX86) || defined(_M_X64)
   #define setW(x, val) (*(volatile unsigned int *)&W(x) = (val))

Or alternatively, it might make sense to add something like the following to compat/mingw.h or git-compat-util.h to fix this once.

 #if defined(_M_IX86) && !defined(__i386__)
 # define __i386__
 #endif
 #if defined(_M_X64) && !defined (__x86_64__)
 # define __x86_64__
 #endif

Thanks, Jonathan

Jonathan Nieder wrote:

I looked at this, but decided not to make this change (while adding an item to my TODO list to investigate further).

After reading the large comment before line 57, and with a vague recollection of the mailing list discussion, I was left with the impression that this was aimed specifically at a quirk of the gcc code generator. In other words, maybe line 57 should read:

#if defined(__GNUC__) && (defined(__i386__) || defined(__x86_64__))

I don't know ... anyway I suspect that msvc has a different set of code generation quirks! :-P

It should probably be investigated at some point, but I don't think it's an urgent issue (and the msvc build will be no worse off ;-).

ATB, Ramsay Jones

Ramsay Jones wrote:

Mmm, my only complaint is that leaving out this change makes the code appear to do something other than it does. Maybe a comment would help.

Sort of, but sort of not. Using volatile here is saying “I really want to do these stores in this order”. And that is probably the right thing to do for _any_ code generator on x86, unless it is very smart.

That would exclude icc etc so I’d prefer to avoid it without empirical evidence.

Right, I agree with this. :)  So for what it’s worth:

  Acked-by: Jonathan Nieder <jrnieder@gmail.com>

I am referring to your original patch $gmane/149542 here.

Thanks.

Jonathan Nieder wrote:

Thanks!

ATB, Ramsay Jones