Re: [PATCH 2/4] msvc: Select the "fast" definition of the {get,put}_be32() macros

From: Ramsay Jones <ramsay@ramsay1.demon.co.uk>
Date: Jun 25, 2010, 20:13 UTC
Message-ID: <4C250DD2.2030801@ramsay1.demon.co.uk>
In-Reply-To: <20100624112148.GA12485@burratino>

Jonathan Nieder wrote:

Show 23 quoted lines

> Ramsay Jones wrote:
> 
>> diff --git a/block-sha1/sha1.c b/block-sha1/sha1.c
>> index d893475..e102856 100644
>> --- a/block-sha1/sha1.c
>> +++ b/block-sha1/sha1.c
>> @@ -70,6 +70,7 @@
>>   */
>>  
>>  #if defined(__i386__) || defined(__x86_64__) || \
>> +    defined(_M_IX86) || defined(_M_X64) || \
>>      defined(__ppc__) || defined(__ppc64__) || \
>>      defined(__powerpc__) || defined(__powerpc64__) || \
>>      defined(__s390__) || defined(__s390x__)
> 
> 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))

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