Re: [PATCH] Windows x64 [repost]

Hello.

The following patches support Windows x64.

1) use intptr_t for Datum and pointer macros. (to support Windows LLP64)
almost the same as that post before.
http://archives.postgresql.org/pgsql-hackers/2009-06/threads.php#01364

2) use appropriate macro and datatypes for Windows API.
enables more than 32bits shared memory.

3) Build scripts for MSVC, this came from
http://archives.postgresql.org/pgsql-hackers/2008-07/msg00440.php
add new parameters to config.pl.
You need define "platform" to "x64" for 64bit programs.

-----

Windows x64 binary that applied patch and build with MSVS2005 can pass
all regression tests (vcregress.bat).

I was checked where the string converted with "%ld" is used.
An especially fatal part is not found excluding one of plperl.

But there is still a possibility that elog messages output a incorrect value.
(I thought it is not fatal, ignored these for the present.)

(eg) src/backend/port/win32_shmem.c, line 167
'size' is 'size_t' = 64bit value.
| ereport(FATAL,
| (errmsg("could not create shared memory segment: %lu", GetLastError()),
| errdetail("Failed system call was CreateFileMapping(size=%lu, name=%s).",
| (unsigned long) size, szShareMem)));

The code that becomes a problem of plperl is the following.
The address is converted into the string, and it is used as hash key.

However, there is really little possibility that two address values
become the same low word, and the problem will not occur.
(Of course, it is necessary to fix though the problem doesn't occur.)

--- src/pl/plperl/plperl.c 2009-11-30 18:56:30.000000000 +0900
+++ /tmp/plperl.c 2009-12-01 18:46:43.000000000 +0900
@@ -95,7 +95,7 @@
**********************************************************************/
typedef struct plperl_query_desc
{
- char qname[sizeof(long) * 2 + 1];
+ char qname[sizeof(void *) * 2 + 1];
void *plan;
int nargs;
Oid *argtypes;
@@ -2343,7 +2343,8 @@
************************************************************/
qdesc = (plperl_query_desc *) malloc(sizeof(plperl_query_desc));
MemSet(qdesc, 0, sizeof(plperl_query_desc));
- snprintf(qdesc->qname, sizeof(qdesc->qname), "%lx", (long) qdesc);
+ /* XXX: for LLP64, use %p or %ll */
+ snprintf(qdesc->qname, sizeof(qdesc->qname), "%p", qdesc);
qdesc->nargs = argc;
qdesc->argtypes = (Oid *) malloc(argc * sizeof(Oid));
qdesc->arginfuncs = (FmgrInfo *) malloc(argc * sizeof(FmgrInfo));

--
Tsutomu Yamada
SRA OSS, Inc. Japan

On Tue, Dec 1, 2009 at 6:25 AM, Tsutomu Yamada <tsutomu(at)sraoss(dot)co(dot)jp> wrote:
> Hello.
>
> The following patches support Windows x64.
>
> 1) use intptr_t for Datum and pointer macros. (to support Windows LLP64)
>   almost the same as that post before.
>   http://archives.postgresql.org/pgsql-hackers/2009-06/threads.php#01364
>
> 2) use appropriate macro and datatypes for Windows API.
>   enables more than 32bits shared memory.
>
> 3) Build scripts for MSVC, this came from
>   http://archives.postgresql.org/pgsql-hackers/2008-07/msg00440.php
>   add new parameters to config.pl.
>   You need define "platform" to "x64" for 64bit programs.

You should add your patch to the currently open commitfest here:

https://commitfest.postgresql.org/action/commitfest_view/open

And perhaps also review the patch submission guidelines here:

http://wiki.postgresql.org/wiki/Submitting_a_Patch

Thanks,

...Robert

Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
> On Tue, Dec 1, 2009 at 6:25 AM, Tsutomu Yamada <tsutomu(at)sraoss(dot)co(dot)jp> wrote:
> > Hello.
> >
> > The following patches support Windows x64.
> >
> > 1) use intptr_t for Datum and pointer macros. (to support Windows LLP64)
> > almost the same as that post before.
> > http://archives.postgresql.org/pgsql-hackers/2009-06/threads.php#01364
> >
> > 2) use appropriate macro and datatypes for Windows API.
> > enables more than 32bits shared memory.
> >
> > 3) Build scripts for MSVC, this came from
> > http://archives.postgresql.org/pgsql-hackers/2008-07/msg00440.php
> > add new parameters to config.pl.
> > You need define "platform" to "x64" for 64bit programs.
>
> You should add your patch to the currently open commitfest here:
>
> https://commitfest.postgresql.org/action/commitfest_view/open
>
> And perhaps also review the patch submission guidelines here:
>
> http://wiki.postgresql.org/wiki/Submitting_a_Patch
>
> Thanks,
>
> ...Robert

Thanks, I add this patch to the open commitfest.

However, archive.postgresql.org has deleted the attachment.
(Why? Email sent to the individual, the attachment is included.)

Is it too large ?
Should I resend them separately or compressing ?
wrong mail format ?
Should I try another mail software ?

--
Tsutomu Yamada
SRA OSS, Inc. Japan

Tsutomu Yamada wrote:
> Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
> > On Tue, Dec 1, 2009 at 6:25 AM, Tsutomu Yamada <tsutomu(at)sraoss(dot)co(dot)jp> wrote:
> > > Hello.
> > >
> > > The following patches support Windows x64.
> > >
> > > 1) use intptr_t for Datum and pointer macros. (to support Windows LLP64)
> > > almost the same as that post before.
> > > http://archives.postgresql.org/pgsql-hackers/2009-06/threads.php#01364
> > >
> > > 2) use appropriate macro and datatypes for Windows API.
> > > enables more than 32bits shared memory.
> > >
> > > 3) Build scripts for MSVC, this came from
> > > http://archives.postgresql.org/pgsql-hackers/2008-07/msg00440.php
> > > add new parameters to config.pl.
> > > You need define "platform" to "x64" for 64bit programs.
> >
> > You should add your patch to the currently open commitfest here:
> >
> > https://commitfest.postgresql.org/action/commitfest_view/open
> >
> > And perhaps also review the patch submission guidelines here:
> >
> > http://wiki.postgresql.org/wiki/Submitting_a_Patch
> >
> > Thanks,
> >
> > ...Robert
>
> Thanks, I add this patch to the open commitfest.
>
> However, archive.postgresql.org has deleted the attachment.
> (Why? Email sent to the individual, the attachment is included.)
>
> Is it too large ?
> Should I resend them separately or compressing ?
> wrong mail format ?
> Should I try another mail software ?

hmm this looks like a bug in the archive interface - might be related to
the fact that it is not looking for attachments after the signature
delimiter or such.

Stefan

Stefan Kaltenbrunner escribió:
> Tsutomu Yamada wrote:

> >However, archive.postgresql.org has deleted the attachment.
> >(Why? Email sent to the individual, the attachment is included.)
> >
> >Is it too large ?
> >Should I resend them separately or compressing ?
> >wrong mail format ?
> >Should I try another mail software ?
>
> hmm this looks like a bug in the archive interface - might be
> related to the fact that it is not looking for attachments after the
> signature delimiter or such.

Hmm, it certainly works in other cases. I think the problem is the part
delimiter, =-=-= in that email; it's probably confusing MHonarc.

--
Alvaro Herrera http://www.CommandPrompt.com/
The PostgreSQL Company - Command Prompt, Inc.

Thanks to suggestion.
I send pathces again by another mailer for the archive.

Sorry to waste resources, below is same content that I send before.

Tsutomu Yamada
SRA OSS, Inc. Japan

#####

The following patches support Windows x64.

1) use intptr_t for Datum and pointer macros. (to support Windows LLP64)
almost the same as that post before.
http://archives.postgresql.org/pgsql-hackers/2009-06/threads.php#01364

2) use appropriate macro and datatypes for Windows API.
enables more than 32bits shared memory.

3) Build scripts for MSVC, this came from
http://archives.postgresql.org/pgsql-hackers/2008-07/msg00440.php
add new parameters to config.pl.
You need define "platform" to "x64" for 64bit programs.

-----

Windows x64 binary that applied patch and build with MSVS2005 can pass
all regression tests (vcregress.bat).

I was checked where the string converted with "%ld" is used.
An especially fatal part is not found excluding one of plperl.

But there is still a possibility that elog messages output a incorrect value.
(I thought it is not fatal, ignored these for the present.)

(eg) src/backend/port/win32_shmem.c, line 167
'size' is 'size_t' = 64bit value.
| ereport(FATAL,
| (errmsg("could not create shared memory segment: %lu", GetLastError()),
| errdetail("Failed system call was CreateFileMapping(size=%lu, name=%s).",
| (unsigned long) size, szShareMem)));

The code that becomes a problem of plperl is the following.
The address is converted into the string, and it is used as hash key.

However, there is really little possibility that two address values
become the same low word, and the problem will not occur.
(Of course, it is necessary to fix though the problem doesn't occur.)

--- src/pl/plperl/plperl.c 2009-11-30 18:56:30.000000000 +0900
+++ /tmp/plperl.c 2009-12-01 18:46:43.000000000 +0900
@@ -95,7 +95,7 @@
**********************************************************************/
typedef struct plperl_query_desc
{
- char qname[sizeof(long) * 2 + 1];
+ char qname[sizeof(void *) * 2 + 1];
void *plan;
int nargs;
Oid *argtypes;
@@ -2343,7 +2343,8 @@
************************************************************/
qdesc = (plperl_query_desc *) malloc(sizeof(plperl_query_desc));
MemSet(qdesc, 0, sizeof(plperl_query_desc));
- snprintf(qdesc->qname, sizeof(qdesc->qname), "%lx", (long) qdesc);
+ /* XXX: for LLP64, use %p or %ll */
+ snprintf(qdesc->qname, sizeof(qdesc->qname), "%p", qdesc);
qdesc->nargs = argc;
qdesc->argtypes = (Oid *) malloc(argc * sizeof(Oid));
qdesc->arginfuncs = (FmgrInfo *) malloc(argc * sizeof(FmgrInfo));

2009/12/4 Tsutomu Yamada <tsutomu(at)sraoss(dot)co(dot)jp>:
> Thanks to suggestion.
> I send pathces again by another mailer for the archive.
>
> Sorry to waste resources, below is same content that I send before.

Just in case anybody was wondering, I've added myself as a reviewer of
this one for next commitfest - I doubt that's very surprising :-)
Others are of course more than welcome to chip in!

--
Magnus Hagander
Me: http://www.hagander.net/
Work: http://www.redpill-linpro.com/

Magnus,

> Just in case anybody was wondering, I've added myself as a reviewer of
> this one for next commitfest - I doubt that's very surprising :-)
> Others are of course more than welcome to chip in!

Greg Smith wrote:
> There are also a couple of patches that for various reasons have yet to
> get a first review done. We've been reassigning for those the last
> couple of days, and I expect that all those will also be looked at by
> Tuesday as well (except for SE-PostgreSQL/Lite, which we all know is a
> bit more complicated). This will leave some time for their authors to
> respond to feedback before we close up here, and of course we still have
> one more CommitFest left for patches that are returned with feedback to
> be resubmitted for.

Now that Greg is going to close the Nov Commit Festa, I think he is
requesting initial reviews for the patches.

Did you have a chance to review the Windows x64 patches?
--
Tatsuo Ishii
SRA OSS, Inc. Japan

Tatsuo Ishii wrote:
> Now that Greg is going to close the Nov Commit Festa, I think he is
> requesting initial reviews for the patches.
>

While Magnus might take a look anyway, in general we'll all be taking a
break from review until January 15th, when the final CommitFest for this
version starts. That way everyone has some time to work on their
development instead of looking at other people's. You should also be
aware that if there are any problems, you may discover the patch just
gets put to the sidelines until the next version. We're trying to
encourage people to submit major patches *before* the final CommitFest,
so that there's time to do a round of feedback on them followed by major
cleanup before they get committed. Unfortunately, something as big was
a new platform port could easily find itself postponed until the next
major version--it's not something development is going to wait for if it
doesn't get finished up before the CommitFest is over. You're basically
at Magnus's mercy though, if he wants to dedicate enough time to get it
done that certainly can happen.

--
Greg Smith 2ndQuadrant Baltimore, MD
PostgreSQL Training, Services and Support
greg(at)2ndQuadrant(dot)com www.2ndQuadrant.com

> Tatsuo Ishii wrote:
> > Now that Greg is going to close the Nov Commit Festa, I think he is
> > requesting initial reviews for the patches.
> >
>
> While Magnus might take a look anyway, in general we'll all be taking a
> break from review until January 15th, when the final CommitFest for this
> version starts. That way everyone has some time to work on their
> development instead of looking at other people's. You should also be
> aware that if there are any problems, you may discover the patch just
> gets put to the sidelines until the next version. We're trying to
> encourage people to submit major patches *before* the final CommitFest,
> so that there's time to do a round of feedback on them followed by major
> cleanup before they get committed. Unfortunately, something as big was
> a new platform port could easily find itself postponed until the next
> major version--it's not something development is going to wait for if it
> doesn't get finished up before the CommitFest is over. You're basically
> at Magnus's mercy though, if he wants to dedicate enough time to get it
> done that certainly can happen.

Ok. Your suggestion is very helpfull. In general Tsutomu will wait for
feedbacks come in, probably until Jan 15th.

BTW, is there anyone who wishes the patches get in 8.5? Apparently
Tstutomu, Magnus and I are counted in the group:-) But I'd like to
know how other people are interested in the patches.
--
Tatsuo Ishii
SRA OSS, Inc. Japan

On Tue, Dec 8, 2009 at 10:30 PM, Tatsuo Ishii <ishii(at)postgresql(dot)org> wrote:
>> Tatsuo Ishii wrote:
>> > Now that Greg is going to close the Nov Commit Festa, I think he is
>> > requesting initial reviews for the patches.
>> >
>>
>> While Magnus might take a look anyway, in general we'll all be taking a
>> break from review until January 15th, when the final CommitFest for this
>> version starts.  That way everyone has some time to work on their
>> development instead of looking at other people's.  You should also be
>> aware that if there are any problems, you may discover the patch just
>> gets put to the sidelines until the next version.  We're trying to
>> encourage people to submit major patches *before* the final CommitFest,
>> so that there's time to do a round of feedback on them followed by major
>> cleanup before they get committed.  Unfortunately, something as big was
>> a new platform port could easily find itself postponed until the next
>> major version--it's not something development is going to wait for if it
>> doesn't get finished up before the CommitFest is over.  You're basically
>> at Magnus's mercy though, if he wants to dedicate enough time to get it
>> done that certainly can happen.
>
> Ok. Your suggestion is very helpfull. In general Tsutomu will wait for
> feedbacks come in, probably until Jan 15th.

Of course there's also no rule that you couldn't review these sooner -
that might help get the ball rolling!

...Robert

> > Ok. Your suggestion is very helpfull. In general Tsutomu will wait for
> > feedbacks come in, probably until Jan 15th.
>
> Of course there's also no rule that you couldn't review these sooner -
> that might help get the ball rolling!

Of course I did before he publishes the patches. (I and he are
working for the same company). However I'm not a Windows programmer
by no means. So my suggestion was mainly for designs...
--
Tatsuo Ishii
SRA OSS, Inc. Japan

On Tue, Dec 8, 2009 at 10:48 PM, Tatsuo Ishii <ishii(at)postgresql(dot)org> wrote:
>> > Ok. Your suggestion is very helpfull. In general Tsutomu will wait for
>> > feedbacks come in, probably until Jan 15th.
>>
>> Of course there's also no rule that you couldn't review these sooner -
>> that might help get the ball rolling!
>
> Of course I did before he publishes the patches.  (I and he are
> working for the same company).  However I'm not a Windows programmer
> by no means. So my suggestion was mainly for designs...

Ah, OK. Makes sense. Sorry, I have not seen too much of your code so
I don't know what areas you specialize in...

...Robert

Tatsuo,

> Ok. Your suggestion is very helpfull. In general Tsutomu will wait for
> feedbacks come in, probably until Jan 15th.
>
> BTW, is there anyone who wishes the patches get in 8.5? Apparently
> Tstutomu, Magnus and I are counted in the group:-) But I'd like to
> know how other people are interested in the patches.

I am very interested. A 64bit-Windows-Version would give a boost
perception-wise

(I know the technical arguments about usefullness or not, but
perception is different and often quite important)

Harald

--
GHUM Harald Massa
persuadere et programmare
Harald Armin Massa
Spielberger Straße 49
70435 Stuttgart
0173/9409607
no fx, no carrier pigeon
-
%s is too gigantic of an industry to bend to the whims of reality

On Wed, Dec 9, 2009 at 8:31 AM, Massa, Harald Armin <chef(at)ghum(dot)de> wrote:
> Tatsuo,
>
>> Ok. Your suggestion is very helpfull. In general Tsutomu will wait for
>> feedbacks come in, probably until Jan 15th.
>>
>> BTW, is there anyone who wishes the patches get in 8.5? Apparently
>> Tstutomu, Magnus and I are counted in the group:-) But I'd like to
>> know how other people are interested in the patches.
>
> I am very interested. A 64bit-Windows-Version would give a boost
> perception-wise

I'm also very interested - despite the fact it'll cause me a boatload
of work to produce a new set of installers for this architecture!

--
Dave Page
EnterpriseDB UK: http://www.enterprisedb.com

> Tatsuo,
>
> > Ok. Your suggestion is very helpfull. In general Tsutomu will wait for
> > feedbacks come in, probably until Jan 15th.
> >
> > BTW, is there anyone who wishes the patches get in 8.5? Apparently
> > Tstutomu, Magnus and I are counted in the group:-) But I'd like to
> > know how other people are interested in the patches.
>
> I am very interested. A 64bit-Windows-Version would give a boost
> perception-wise
>
> (I know the technical arguments about usefullness or not, but
> perception is different and often quite important)

Totally agreed.
--
Tatsuo Ishii
SRA OSS, Inc. Japan

> >> BTW, is there anyone who wishes the patches get in 8.5? Apparently
> >> Tstutomu, Magnus and I are counted in the group:-) But I'd like to
> >> know how other people are interested in the patches.
> >
> > I am very interested. A 64bit-Windows-Version would give a boost
> > perception-wise
>
> I'm also very interested - despite the fact it'll cause me a boatload
> of work to produce a new set of installers for this architecture!

Sorry for this:-)
--
Tatsuo Ishii
SRA OSS, Inc. Japan

2009/12/9 Tatsuo Ishii <ishii(at)postgresql(dot)org>:
>> > Ok. Your suggestion is very helpfull. In general Tsutomu will wait for
>> > feedbacks come in, probably until Jan 15th.
>>
>> Of course there's also no rule that you couldn't review these sooner -
>> that might help get the ball rolling!
>
> Of course I did before he publishes the patches.  (I and he are
> working for the same company).  However I'm not a Windows programmer
> by no means. So my suggestion was mainly for designs...

:-)

As a reference for the future, please let us know when you have done
this before the patch is submitted. I think it's not very common that
just because you are in the same company, you have reviewed it. For
example, I highly doubt that Heikki reviews all the patches Bruce
post, or the other way around :-) And it's very useful to know that
one set of eyes have been on it already.

--
Magnus Hagander
Me: http://www.hagander.net/
Work: http://www.redpill-linpro.com/

> As a reference for the future, please let us know when you have done
> this before the patch is submitted. I think it's not very common that
> just because you are in the same company, you have reviewed it. For
> example, I highly doubt that Heikki reviews all the patches Bruce
> post, or the other way around :-) And it's very useful to know that
> one set of eyes have been on it already.

Ok, next time I will do that.

As I said before, I'm not a good Windows programmer at all and
hesitated to say that "Trust me, I have reviewd his patches":-)
--
Tatsuo Ishii
SRA OSS, Inc. Japan

2009/12/4 Tsutomu Yamada <tsutomu(at)sraoss(dot)co(dot)jp>:
> Thanks to suggestion.
> I send pathces again by another mailer for the archive.
>
> Sorry to waste resources, below is same content that I send before.

I have a couple of comments about the first patch (I'll get to the
others later):

config.win32.h has:
+ #ifdef _MSC_VER
+ /* #undef HAVE_STDINT_H */
+ #else
#define HAVE_STDINT_H 1
+ #endif

Is that really necessary? config.h.win32 is only used on MSVC builds, no?

A bit further down, it has:

+ /* The size of `void *', as computed by sizeof. */
+ #define SIZEOF_VOID_P 4
+

shouldn't that be 8 for win64 platforms?

This patch also needs autoconf support for other platforms, but I've
bugged Bruce about that and have some code to get that done. Just
wanted these questions settled before I move on.

--
Magnus Hagander
Me: http://www.hagander.net/
Work: http://www.redpill-linpro.com/

2009/12/31 Magnus Hagander <magnus(at)hagander(dot)net>:
> 2009/12/4 Tsutomu Yamada <tsutomu(at)sraoss(dot)co(dot)jp>:
> A bit further down, it has:
>
> + /* The size of `void *', as computed by sizeof. */
> + #define SIZEOF_VOID_P 4
> +
>
>
> shouldn't that be 8 for win64 platforms?

Nevermind this second comment. Now that it's no longer 1AM, I see that
this is included in the *second* patch...

--
Magnus Hagander
Me: http://www.hagander.net/
Work: http://www.redpill-linpro.com/

On Fri, Dec 4, 2009 at 11:42, Tsutomu Yamada <tsutomu(at)sraoss(dot)co(dot)jp> wrote:
>
> 2) use appropriate macro and datatypes for Windows API.
>   enables more than 32bits shared memory.

Are you sure this one should use __noop, and not __nop?

__noop: http://msdn.microsoft.com/en-us/library/s6btaxcs.aspx
__nop: http://msdn.microsoft.com/en-us/library/aa983381.aspx

I think __nop is what we want?

Also, that turns it into "nop" and not "rep nop", no?

Should we perhaps instead use __yield, per:
http://msdn.microsoft.com/en-us/library/2b2h26kx.aspx

Hopefully someone who knows this better than me can comment :-)

--
Magnus Hagander
Me: http://www.hagander.net/
Work: http://www.redpill-linpro.com/

On Fri, Jan 1, 2010 at 20:45, Magnus Hagander <magnus(at)hagander(dot)net> wrote:
> On Fri, Dec 4, 2009 at 11:42, Tsutomu Yamada <tsutomu(at)sraoss(dot)co(dot)jp> wrote:
>>
>> 2) use appropriate macro and datatypes for Windows API.
>>   enables more than 32bits shared memory.
>
> Are you sure this one should use __noop, and not __nop?
>
> __noop: http://msdn.microsoft.com/en-us/library/s6btaxcs.aspx
> __nop: http://msdn.microsoft.com/en-us/library/aa983381.aspx
>
> I think __nop is what we want?
>
> Also, that turns it into "nop" and not "rep nop", no?

I did some more research, and __nop() is at least closer than
__noop(), but it's still not the same.

> Should we perhaps instead use __yield, per:
> http://msdn.microsoft.com/en-us/library/2b2h26kx.aspx

On further reading, __yield() is only available on Itanium.

--
Magnus Hagander
Me: http://www.hagander.net/
Work: http://www.redpill-linpro.com/

Magnus Hagander <magnus(at)hagander(dot)net> wrote:
> On Fri, Jan 1, 2010 at 20:45, Magnus Hagander <magnus(at)hagander(dot)net> wrote:
> > On Fri, Dec 4, 2009 at 11:42, Tsutomu Yamada <tsutomu(at)sraoss(dot)co(dot)jp> wrote:
> >>
> >> 2) use appropriate macro and datatypes for Windows API.
> >> enables more than 32bits shared memory.
> >
> > Are you sure this one should use __noop, and not __nop?
> >
> > __noop: http://msdn.microsoft.com/en-us/library/s6btaxcs.aspx
> > __nop: http://msdn.microsoft.com/en-us/library/aa983381.aspx
> >
> > I think __nop is what we want?
> >
> > Also, that turns it into "nop" and not "rep nop", no?
>
> I did some more research, and __nop() is at least closer than
> __noop(), but it's still not the same.
>
>
> > Should we perhaps instead use __yield, per:
> > http://msdn.microsoft.com/en-us/library/2b2h26kx.aspx
>
> On further reading, __yield() is only available on Itanium.

This spinlock code was came from below.
http://archives.postgresql.org/pgsql-hackers/2008-07/msg00307.php

Sorry, I didn't care which macro was better.

I found 'YieldProcessor' in MSDN.
http://msdn.microsoft.com/en-us/library/ms687419%28VS.85%29.aspx

YieldProcessor was defined in "winnt.h".
The definition changes depending on architecture and compiler version.

__asm { rep nop };
__mm_pause();
__yield();

YieldProcessor become "__mm_pause()" in _AMD64_.
So "__mm_pause()" is better?

// test program
#include <windows.h>

main()
{
YieldProcessor();
}
// end
// cl /E test.c > out.i
// tail out.i
// # I recommend redirecting to file, The output become large.

Tsutomu Yamada
SRA OSS, Inc. Japan

On Tue, Jan 5, 2010 at 09:14, Tsutomu Yamada <tsutomu(at)sraoss(dot)co(dot)jp> wrote:
> Magnus Hagander <magnus(at)hagander(dot)net> wrote:
>  > On Fri, Jan 1, 2010 at 20:45, Magnus Hagander <magnus(at)hagander(dot)net> wrote:
>  > > On Fri, Dec 4, 2009 at 11:42, Tsutomu Yamada <tsutomu(at)sraoss(dot)co(dot)jp> wrote:
>  > >>
>  > >> 2) use appropriate macro and datatypes for Windows API.
>  > >>   enables more than 32bits shared memory.
>  > >
>  > > Are you sure this one should use __noop, and not __nop?
>  > >
>  > > __noop: http://msdn.microsoft.com/en-us/library/s6btaxcs.aspx
>  > > __nop: http://msdn.microsoft.com/en-us/library/aa983381.aspx
>  > >
>  > > I think __nop is what we want?
>  > >
>  > > Also, that turns it into "nop" and not "rep nop", no?
>  >
>  > I did some more research, and __nop() is at least closer than
>  > __noop(), but it's still not the same.
>  >
>  >
>  > > Should we perhaps instead use __yield, per:
>  > > http://msdn.microsoft.com/en-us/library/2b2h26kx.aspx
>  >
>  > On further reading, __yield() is only available on Itanium.
>
>
> This spinlock code was came from below.
> http://archives.postgresql.org/pgsql-hackers/2008-07/msg00307.php
>
> Sorry, I didn't care which macro was better.
>
> I found 'YieldProcessor' in MSDN.
> http://msdn.microsoft.com/en-us/library/ms687419%28VS.85%29.aspx
>
> YieldProcessor was defined in "winnt.h".
> The definition changes depending on architecture and compiler version.
>
> __asm { rep nop };
> __mm_pause();
> __yield();
>
> YieldProcessor become "__mm_pause()" in _AMD64_.
> So "__mm_pause()" is better?

Yeah, that seems right. I'll change it to that.

--
Magnus Hagander
Me: http://www.hagander.net/
Work: http://www.redpill-linpro.com/

Magnus Hagander <magnus(at)hagander(dot)net> wrote:

> 2009/12/4 Tsutomu Yamada <tsutomu(at)sraoss(dot)co(dot)jp>:
> > Thanks to suggestion.
> > I send pathces again by another mailer for the archive.
> >
> > Sorry to waste resources, below is same content that I send before.
>
> I have a couple of comments about the first patch (I'll get to the
> others later):
>
> config.win32.h has:
> + #ifdef _MSC_VER
> + /* #undef HAVE_STDINT_H */
> + #else
> #define HAVE_STDINT_H 1
> + #endif
>
> Is that really necessary? config.h.win32 is only used on MSVC builds, no?

I also think it is unnecessary.

But pg_config.h.win32 was referred from "src/bcc32.mak".
Is Borland C++ still supported?
It is an unnecessary code if not becoming a problem to Borland.

--
Tsutomu Yamada
SRA OSS, Inc. Japan

On Tue, Jan 5, 2010 at 12:58, Tsutomu Yamada <tsutomu(at)sraoss(dot)co(dot)jp> wrote:
> Magnus Hagander <magnus(at)hagander(dot)net> wrote:
>
>  > 2009/12/4 Tsutomu Yamada <tsutomu(at)sraoss(dot)co(dot)jp>:
>  > > Thanks to suggestion.
>  > > I send pathces again by another mailer for the archive.
>  > >
>  > > Sorry to waste resources, below is same content that I send before.
>  >
>  > I have a couple of comments about the first patch (I'll get to the
>  > others later):
>  >
>  > config.win32.h has:
>  > + #ifdef _MSC_VER
>  > + /* #undef HAVE_STDINT_H */
>  > + #else
>  >   #define HAVE_STDINT_H 1
>  > + #endif
>  >
>  > Is that really necessary? config.h.win32 is only used on MSVC builds, no?
>
> I also think it is unnecessary.
>
> But pg_config.h.win32 was referred from "src/bcc32.mak".
> Is Borland C++ still supported?
> It is an unnecessary code if not becoming a problem to Borland.

Hmm. Yeah, that's a good question. I'd say it's supported on 8.4,
because it's in the docs, and I think it works. As for 8.5, it will
only be supported if somebody steps up and does the testing around RC
time.

Having accidentally configured that thing wrong for no change for
libpq (I think it was), I'm not convinced it wil actually break
Borland. So let's leave it this way unless someone confirms it's
broken.

--
Magnus Hagander
Me: http://www.hagander.net/
Work: http://www.redpill-linpro.com/

On Fri, Dec 4, 2009 at 11:42, Tsutomu Yamada <tsutomu(at)sraoss(dot)co(dot)jp> wrote:
> The following patches support Windows x64.
>
> 1) use intptr_t for Datum and pointer macros. (to support Windows LLP64)
>   almost the same as that post before.
>   http://archives.postgresql.org/pgsql-hackers/2009-06/threads.php#01364
>
> 2) use appropriate macro and datatypes for Windows API.
>   enables more than 32bits shared memory.
>
> 3) Build scripts for MSVC, this came from
>   http://archives.postgresql.org/pgsql-hackers/2008-07/msg00440.php
>   add new parameters to config.pl.
>   You need define "platform" to "x64" for 64bit programs.

I have now applied this, with some extensive refactoring and
extension. But all parts in this one should be in now, so if you are
missing something, please let me know :-)

> I was checked where the string converted with "%ld" is used.
> An especially fatal part is not found excluding one of plperl.

I have not looked at the plperl stuff yet. I'd appreciate a comment
from someone who knows plperl :-) Andrew, maybe?

--
Magnus Hagander
Me: http://www.hagander.net/
Work: http://www.redpill-linpro.com/

Magnus Hagander <magnus(at)hagander(dot)net> writes:
> On Fri, Dec 4, 2009 at 11:42, Tsutomu Yamada <tsutomu(at)sraoss(dot)co(dot)jp> wrote:
>> I was checked where the string converted with "%ld" is used.
>> An especially fatal part is not found excluding one of plperl.

> I have not looked at the plperl stuff yet. I'd appreciate a comment
> from someone who knows plperl :-) Andrew, maybe?

I think I dealt with that already, if Tsutomu-san is speaking of the
prepared-query hash table keys in plperl and pltcl.

regards, tom lane

Thank you very much for refining patch.
I thought there is no missing part.

Magnus Hagander <magnus(at)hagander(dot)net> wrote:
> On Fri, Dec 4, 2009 at 11:42, Tsutomu Yamada <tsutomu(at)sraoss(dot)co(dot)jp> wrote:
> > The following patches support Windows x64.
> >
> > 1) use intptr_t for Datum and pointer macros. (to support Windows LLP64)
> > almost the same as that post before.
> > http://archives.postgresql.org/pgsql-hackers/2009-06/threads.php#01364
> >
> > 2) use appropriate macro and datatypes for Windows API.
> > enables more than 32bits shared memory.
> >
> > 3) Build scripts for MSVC, this came from
> > http://archives.postgresql.org/pgsql-hackers/2008-07/msg00440.php
> > add new parameters to config.pl.
> > You need define "platform" to "x64" for 64bit programs.
>
> I have now applied this, with some extensive refactoring and
> extension. But all parts in this one should be in now, so if you are
> missing something, please let me know :-)
>
> > I was checked where the string converted with "%ld" is used.
> > An especially fatal part is not found excluding one of plperl.
>
> I have not looked at the plperl stuff yet. I'd appreciate a comment
> from someone who knows plperl :-) Andrew, maybe?

It has already been dealt as followed.
# Thanks, I dit not notice the part of pltcl.

Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> I think I dealt with that already, if Tsutomu-san is speaking of the
> prepared-query hash table keys in plperl and pltcl.

--
Tsutomu Yamada
SRA OSS, Inc. Japan