{"thread":{"id":"24178","subject":"[PATCH 2/4] msvc: Select the \"fast\" definition of the {get,put}_be32() macros","startedAt":"2010-06-23T19:47:02Z","lastAt":"2010-06-30T19:53:59Z","messageCount":5,"participants":["Ramsay Jones","Jonathan Nieder"],"isPatch":true,"patchVersion":1,"patchTotal":4},"messages":[{"id":"144100","messageId":"4C2264B6.5010906@ramsay1.demon.co.uk","threadId":"24178","inReplyTo":null,"subject":"[PATCH 2/4] msvc: Select the \"fast\" definition of the {get,put}_be32() macros","fromName":"Ramsay Jones","fromEmail":"ramsay@ramsay1.demon.co.uk","sentAt":"2010-06-23T19:47:02Z","receivedAt":"2010-06-23T19:47:02Z","isPatch":true,"sender":{"key":"ramsay@ramsayjones.plus.com","avatar":"https://avatars.githubusercontent.com/u/33702710?v=4"},"body":"\nOn Intel machines, the msvc compiler defines the CPU architecture\nmacros _M_IX86 and _M_X64 (equivalent to __i386__ and __x86_64__\nrespectively). Use these macros in the pre-processor expression\nto select the \"fast\" definition of the {get,put}_be32() macros.\n\nSigned-off-by: Ramsay Jones \n---\n\nI did consider adding an additional guard, like so:\n\n (defined(_MSC_VER) && (defined(_M_IX86) || defined(_M_X64))) || \\\n\nHowever, I decided that the extra paranoia was not needed ...\n\n\n block-sha1/sha1.c | 1 +\n 1 files changed, 1 insertions(+), 0 deletions(-)\n\ndiff --git a/block-sha1/sha1.c b/block-sha1/sha1.c\nindex d893475..e102856 100644\n--- a/block-sha1/sha1.c\n+++ b/block-sha1/sha1.c\n@@ -70,6 +70,7 @@\n */\n \n #if defined(__i386__) || defined(__x86_64__) || \\\n+ defined(_M_IX86) || defined(_M_X64) || \\\n defined(__ppc__) || defined(__ppc64__) || \\\n defined(__powerpc__) || defined(__powerpc64__) || \\\n defined(__s390__) || defined(__s390x__)\n-- \n1.7.1\n"},{"id":"144140","messageId":"20100624112148.GA12485@burratino","threadId":"24178","inReplyTo":"4C2264B6.5010906@ramsay1.demon.co.uk","subject":"Re: [PATCH 2/4] msvc: Select the \"fast\" definition of the {get,put}_be32() macros","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-06-24T11:21:48Z","receivedAt":"2010-06-24T11:21:48Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Ramsay Jones wrote:\n\n> diff --git a/block-sha1/sha1.c b/block-sha1/sha1.c\n> index d893475..e102856 100644\n> --- a/block-sha1/sha1.c\n> +++ b/block-sha1/sha1.c\n> @@ -70,6 +70,7 @@\n> */\n> \n> #if defined(__i386__) || defined(__x86_64__) || \\\n> + defined(_M_IX86) || defined(_M_X64) || \\\n> defined(__ppc__) || defined(__ppc64__) || \\\n> defined(__powerpc__) || defined(__powerpc64__) || \\\n> defined(__s390__) || defined(__s390x__)\n\nLooks good to me, for what it’s worth. You might want a similar\nchange on line 57:\n\n \n-#if defined(__i386__) || defined(__x86_64__)\n+#if defined(__i386__) || defined(__x86_64__) || \\\n+\tdefined(_M_IX86) || defined(_M_X64)\n #define setW(x, val) (*(volatile unsigned int *)&W(x) = (val))\n\nOr alternatively, it might make sense to add something like the\nfollowing to compat/mingw.h or git-compat-util.h to fix this once.\n\n #if defined(_M_IX86) && !defined(__i386__)\n # define __i386__\n #endif\n #if defined(_M_X64) && !defined (__x86_64__)\n # define __x86_64__\n #endif\n\nThanks,\nJonathan\n"},{"id":"144321","messageId":"4C250DD2.2030801@ramsay1.demon.co.uk","threadId":"24178","inReplyTo":"20100624112148.GA12485@burratino","subject":"Re: [PATCH 2/4] msvc: Select the \"fast\" definition of the {get,put}_be32() macros","fromName":"Ramsay Jones","fromEmail":"ramsay@ramsay1.demon.co.uk","sentAt":"2010-06-25T20:13:06Z","receivedAt":"2010-06-25T20:13:06Z","isPatch":true,"sender":{"key":"ramsay@ramsayjones.plus.com","avatar":"https://avatars.githubusercontent.com/u/33702710?v=4"},"body":"Jonathan Nieder wrote:\n> Ramsay Jones wrote:\n> \n>> diff --git a/block-sha1/sha1.c b/block-sha1/sha1.c\n>> index d893475..e102856 100644\n>> --- a/block-sha1/sha1.c\n>> +++ b/block-sha1/sha1.c\n>> @@ -70,6 +70,7 @@\n>> */\n>> \n>> #if defined(__i386__) || defined(__x86_64__) || \\\n>> + defined(_M_IX86) || defined(_M_X64) || \\\n>> defined(__ppc__) || defined(__ppc64__) || \\\n>> defined(__powerpc__) || defined(__powerpc64__) || \\\n>> defined(__s390__) || defined(__s390x__)\n> \n> Looks good to me, for what it’s worth. You might want a similar\n> change on line 57:\n> \n> \n> -#if defined(__i386__) || defined(__x86_64__)\n> +#if defined(__i386__) || defined(__x86_64__) || \\\n> +\tdefined(_M_IX86) || defined(_M_X64)\n> #define setW(x, val) (*(volatile unsigned int *)&W(x) = (val))\n\nI looked at this, but decided not to make this change (while adding\nan item to my TODO list to investigate further).\n\nAfter reading the large comment before line 57, and with a vague\nrecollection of the mailing list discussion, I was left with the\nimpression that this was aimed specifically at a quirk of the gcc\ncode generator. In other words, maybe line 57 should read:\n\n#if defined(__GNUC__) && (defined(__i386__) || defined(__x86_64__))\n\nI don't know ... anyway I suspect that msvc has a different set of\ncode generation quirks! :-P\n\nIt should probably be investigated at some point, but I don't think\nit's an urgent issue (and the msvc build will be no worse off ;-).\n\nATB,\nRamsay Jones\n"},{"id":"144327","messageId":"20100626183111.GB13070@burratino","threadId":"24178","inReplyTo":"4C250DD2.2030801@ramsay1.demon.co.uk","subject":"Re: [PATCH 2/4] msvc: Select the \"fast\" definition of the {get,put}_be32() macros","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-06-26T18:31:12Z","receivedAt":"2010-06-26T18:31:12Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Ramsay Jones wrote:\n> Jonathan Nieder wrote:\n\n>> -#if defined(__i386__) || defined(__x86_64__)\n>> +#if defined(__i386__) || defined(__x86_64__) || \\\n>> +\tdefined(_M_IX86) || defined(_M_X64)\n>> #define setW(x, val) (*(volatile unsigned int *)&W(x) = (val))\n>\n> I looked at this, but decided not to make this change (while adding\n> an item to my TODO list to investigate further).\n\nMmm, my only complaint is that leaving out this change makes the\ncode appear to do something other than it does. Maybe a comment\nwould help.\n\n> After reading the large comment before line 57, and with a vague\n> recollection of the mailing list discussion, I was left with the\n> impression that this was aimed specifically at a quirk of the gcc\n> code generator.\n\nSort of, but sort of not. Using volatile here is saying “I really\nwant to do these stores in this order”. And that is probably the\nright thing to do for _any_ code generator on x86, unless it is very\nsmart.\n\n> In other words, maybe line 57 should read:\n>\n> #if defined(__GNUC__) && (defined(__i386__) || defined(__x86_64__))\n\nThat would exclude icc etc so I’d prefer to avoid it without\nempirical evidence.\n\n> It should probably be investigated at some point, but I don't think\n> it's an urgent issue (and the msvc build will be no worse off ;-).\n\nRight, I agree with this. :) So for what it’s worth:\n\n Acked-by: Jonathan Nieder \n\nI am referring to your original patch $gmane/149542 here.\n\nThanks.\n"},{"id":"144547","messageId":"4C2BA0D7.2070903@ramsay1.demon.co.uk","threadId":"24178","inReplyTo":"20100626183111.GB13070@burratino","subject":"Re: [PATCH 2/4] msvc: Select the \"fast\" definition of the {get,put}_be32() macros","fromName":"Ramsay Jones","fromEmail":"ramsay@ramsay1.demon.co.uk","sentAt":"2010-06-30T19:53:59Z","receivedAt":"2010-06-30T19:53:59Z","isPatch":true,"sender":{"key":"ramsay@ramsayjones.plus.com","avatar":"https://avatars.githubusercontent.com/u/33702710?v=4"},"body":"Jonathan Nieder wrote:\n>> It should probably be investigated at some point, but I don't think\n>> it's an urgent issue (and the msvc build will be no worse off ;-).\n> \n> Right, I agree with this. :) So for what it’s worth:\n> \n> Acked-by: Jonathan Nieder \n\nThanks!\n\nATB,\nRamsay Jones\n"}]}