code review 135090043: runtime: allow to call stdcall from Go on windows

runtime: allow to call stdcall from Go on windows
I've started with just one function with 8 arguments,
but stdcall is called from nosplit functions
and 8 args overflow nosplit area.

[dvyukov, 2014-08-28 18:26:31]

Hello golang-codereviews@googlegroups.com (cc: alex.brainman@gmail.com,
khr@golang.org, rsc@golang.org),

I'd like you to review this change to
https://go.googlecode.com/hg/

[dvyukov, 2014-08-28 18:35:02]

Hello golang-codereviews@googlegroups.com (cc: alex.brainman@gmail.com,
golang-codereviews@googlegroups.com, iant@golang.org, khr@golang.org,
rsc@golang.org),

Please take another look.

[aram, 2014-08-28 20:38:15]

LGTM

https://codereview.appspot.com/135090043/diff/140001/src/pkg/runtime/os_windo...
File src/pkg/runtime/os_windows.c (right):

https://codereview.appspot.com/135090043/diff/140001/src/pkg/runtime/os_windo...
src/pkg/runtime/os_windows.c:320: stdcall(void *fn)
Why not rewrite all these in Go, like in 131600043, this is the final goal,
isn't it?

[dvyukov, 2014-08-28 20:46:54]

On 2014/08/28 20:38:15, aram wrote:
> LGTM
> 
>
https://codereview.appspot.com/135090043/diff/140001/src/pkg/runtime/os_windo...
> File src/pkg/runtime/os_windows.c (right):
> 
>
https://codereview.appspot.com/135090043/diff/140001/src/pkg/runtime/os_windo...
> src/pkg/runtime/os_windows.c:320: stdcall(void *fn)
> Why not rewrite all these in Go, like in 131600043, this is the final goal,
> isn't it?

I've tried it and there were new crashes during runtime bootstrap that is very
difficult to debug on windows. Also I figured out that it will be considerably
slower as I will have to call getg many times (windows code is more complex than
solaris), or create several local vars which can overflow nosplit stack.

I agree that it would be positive. But we can live with it for now.

[brainman, 2014-08-28 23:39:24]

LGTM

Alex

https://codereview.appspot.com/135090043/diff/140001/src/pkg/runtime/os_windo...
File src/pkg/runtime/os_windows.c (right):

https://codereview.appspot.com/135090043/diff/140001/src/pkg/runtime/os_windo...
src/pkg/runtime/os_windows.c:339: {
I would implement all these in asm. They all look alike, just n is different.
Load n into AX and JMP. What do you think? I can do it in later CL.

[dfc, 2014-08-29 00:48:29]

nit: please fix the typo in the description.
On 29 Aug 2014 09:39, <alex.brainman@gmail.com> wrote:

> LGTM
>
> Alex
>
>
> https://codereview.appspot.com/135090043/diff/140001/src/
> pkg/runtime/os_windows.c
> File src/pkg/runtime/os_windows.c (right):
>
> https://codereview.appspot.com/135090043/diff/140001/src/
> pkg/runtime/os_windows.c#newcode339
> src/pkg/runtime/os_windows.c:339: {
> I would implement all these in asm. They all look alike, just n is
> different. Load n into AX and JMP. What do you think? I can do it in
> later CL.
>
> https://codereview.appspot.com/135090043/
>
> --
> You received this message because you are subscribed to the Google Groups
> "golang-codereviews" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to golang-codereviews+unsubscribe@googlegroups.com.
> For more options, visit https://groups.google.com/d/optout.
>

[dvyukov, 2014-08-29 08:39:49]

On 2014/08/29 00:48:29, dfc wrote:
> nit: please fix the typo in the description.

done

[dvyukov, 2014-08-29 08:42:18]

On 2014/08/28 23:39:24, brainman wrote:
> LGTM
> 
> Alex
> 
>
https://codereview.appspot.com/135090043/diff/140001/src/pkg/runtime/os_windo...
> File src/pkg/runtime/os_windows.c (right):
> 
>
https://codereview.appspot.com/135090043/diff/140001/src/pkg/runtime/os_windo...
> src/pkg/runtime/os_windows.c:339: {
> I would implement all these in asm. They all look alike, just n is different.
> Load n into AX and JMP. What do you think? I can do it in later CL.


I've started with that. The problem is that the functions return value, and that
value is at different offsets for different functions. So one would need to do
CALL instead of JMP, and then manually calculate ret value offset. Or do some
other hackery 
Not saying about duplication between amd64 and 386.
I've decided to go with C.

[dvyukov, 2014-08-29 08:44:11]

*** Submitted as https://code.google.com/p/go/source/detail?r=4b615799f98d ***

runtime: allow to call stdcall from Go on windows
I've started with just one function with 8 arguments,
but stdcall is called from nosplit functions
and 8 args overflow nosplit area.

LGTM=aram, alex.brainman
R=golang-codereviews, aram, alex.brainman, dave
CC=golang-codereviews, iant, khr, rsc
https://codereview.appspot.com/135090043

[gobot, 2014-08-29 08:57:04]

This CL appears to have broken the netbsd-amd64-bsiegert builder.
See http://build.golang.org/log/5e06fe65b00cae31091ba29d86763a7b254129ee

[brainman, 2014-08-29 10:12:33]

On 2014/08/29 08:42:18, dvyukov wrote:
> ... The problem is that the functions return value, and that
> value is at different offsets for different functions. ...

Yeh, you're right about that. I tried it myself already with runtime.syscall,
but forgotten ...

Alex

[rsc, 2014-08-29 14:18:47]

LGTM