Re: [gofrontend-dev] [PATCH 1/2, libgo] Add reflection support to gccgo for ppc64, ppc64le in gcc 4.9
Yes, I have other options, I just wanted to verify that this was still your answer. Thanks. On 01/14/2015 09:24 AM, Ian Lance Taylor wrote: On Wed, Jan 14, 2015 at 5:49 AM, Lynn A. Boger labo...@linux.vnet.ibm.com wrote: I have an updated patch for the reflection functions in gcc 4.9 for ppc64 ppc64le which allows the recover.go test to pass and the updated version of libgo/go/reflect/all_tests.go to pass once all testcases are enabled for ppc64 ppc64le. Based on the discussion we've had on this patch, has the decision changed concerning whether or not this patch might be accepted into the gcc 4.9 branch? If so, I will post the updated patch. To me it is not appropriate to add brand new code, code that has never been in mainline, to an existing release branch. That is not what release branches are for. Release branches are for backports of bug fixes, not for new code. I assume that the people who want this code are not going to sit on their hands waiting for the 4.9.3 release, which won't happen for several months even in the best case. Why not provide this code as a separate patch on top of the 4.9 releases? Or manage your own branch if necessary? Ian
Re: [gofrontend-dev] [PATCH 1/2, libgo] Add reflection support to gccgo for ppc64, ppc64le in gcc 4.9
On Wed, Jan 14, 2015 at 5:49 AM, Lynn A. Boger labo...@linux.vnet.ibm.com wrote: I have an updated patch for the reflection functions in gcc 4.9 for ppc64 ppc64le which allows the recover.go test to pass and the updated version of libgo/go/reflect/all_tests.go to pass once all testcases are enabled for ppc64 ppc64le. Based on the discussion we've had on this patch, has the decision changed concerning whether or not this patch might be accepted into the gcc 4.9 branch? If so, I will post the updated patch. To me it is not appropriate to add brand new code, code that has never been in mainline, to an existing release branch. That is not what release branches are for. Release branches are for backports of bug fixes, not for new code. I assume that the people who want this code are not going to sit on their hands waiting for the 4.9.3 release, which won't happen for several months even in the best case. Why not provide this code as a separate patch on top of the 4.9 releases? Or manage your own branch if necessary? Ian
Re: [gofrontend-dev] [PATCH 1/2, libgo] Add reflection support to gccgo for ppc64, ppc64le in gcc 4.9
I have an updated patch for the reflection functions in gcc 4.9 for ppc64 ppc64le which allows the recover.go test to pass and the updated version of libgo/go/reflect/all_tests.go to pass once all testcases are enabled for ppc64 ppc64le. Based on the discussion we've had on this patch, has the decision changed concerning whether or not this patch might be accepted into the gcc 4.9 branch? If so, I will post the updated patch. On 01/08/2015 08:53 AM, Ian Lance Taylor wrote: On Thu, Jan 8, 2015 at 5:44 AM, Lynn A. Boger labo...@linux.vnet.ibm.com wrote: Also as was noted below, some level of reflection support is needed to build Docker. Sorry to be so pedantic, but I'm just trying to clarify that you keep saying reflection support, but reflection support is there. What is missing is a miniscule rarely used part of the reflect package, namely support for reflect.MakeFunc, and, it's true, calling reflect.ValueOf, then Method or MethodByName, then Interface. Ian
Re: [gofrontend-dev] [PATCH 1/2, libgo] Add reflection support to gccgo for ppc64, ppc64le in gcc 4.9
If this change is not included, in the gcc go testsuite, the recover.go
testcase fails with this error:
panic: reflect.makeMethodValue not implemented for ppc64le
when running this test:
func test9reflect1() {
f := reflect.ValueOf(T1{}).Method(0).Interface().(func())
defer f()
panic(9)
}
Also as was noted below, some level of reflection support is needed to
build Docker. I was given this patch by someone in IBM trying to build
Docker on Power who told me it was needed (but didn't really know the
details). There is interest in trying to get Docker to build with gcc
4.9 because 5.0 won't be available in distros for a while yet. I was
told that the minimal reflection support provided by this patch was
enough to get it to build. But if a simple workaround in the Docker
source will make it work that might be sufficient. I'll find out. Thanks.
On 01/07/2015 07:20 PM, Ian Lance Taylor wrote:
On Wed, Jan 7, 2015 at 4:39 PM, Michael Hudson-Doyle
michael.hud...@canonical.com wrote:
Ian Lance Taylor i...@golang.org writes:
On Wed, Jan 7, 2015 at 9:26 AM, Lynn A. Boger
labo...@linux.vnet.ibm.com wrote:
In libgo/go/reflect/makefunc.go, calls to MakeFunc, makeMethodValue and
makeValueMethod will panic if called when GOARCH is ppc64 or ppc64le.
Right, I'm just saying that almost no code actually does that. I just
tried a web search and found no uses other than examples of how to use
it. I'm sure there are a few, but not many.
There is a somewhat hidden one in Docker:
https://github.com/docker/docker/blob/master/api/client/cli.go#L64
(it is also possible to patch docker to do this differently, of course).
Ah, so that is why it matters.
Perhaps you could talk Docker into replace the return statement with
return func(a ...string) error {
return
method.CallSlice([]reflect.Value{reflect.ValueOf(a)})[0].Interface().(error)
}, true
It would probably be just as efficient.
Ian
Re: [gofrontend-dev] [PATCH 1/2, libgo] Add reflection support to gccgo for ppc64, ppc64le in gcc 4.9
On Thu, Jan 8, 2015 at 5:44 AM, Lynn A. Boger labo...@linux.vnet.ibm.com wrote: Also as was noted below, some level of reflection support is needed to build Docker. Sorry to be so pedantic, but I'm just trying to clarify that you keep saying reflection support, but reflection support is there. What is missing is a miniscule rarely used part of the reflect package, namely support for reflect.MakeFunc, and, it's true, calling reflect.ValueOf, then Method or MethodByName, then Interface. Ian
Re: [gofrontend-dev] [PATCH 1/2, libgo] Add reflection support to gccgo for ppc64, ppc64le in gcc 4.9
The only bugs I am aware of when using reflection on ppc64 ppc64le in gcc 4.9 are in the gccgo testcase recover.go and what is needed to build Docker. When I run the gccgo testsuite and see a failing testcase with a panic message saying that a function is not implemented it makes me think some major pieces are missing so that is why I've used that phrase. Also I noticed that the libgo/go/reflect/all_test.go testcase is not running some tests when GOARCH is ppc64 or ppc64le, and if those are enabled they fail. So some might call those bugs or unimplemented features or missing support. To me, a message saying that something is not implemented is considered missing support even though it might be minor. The information I received from those who had attempted to build Docker on ppc64le in gcc 4.9 said that it needed reflection and it wasn't there and this patch was used to make it work. I hope that answers your question? I have not done a thorough testing/investigation of reflection in gccgo 4.9 on Power and my statement refers only on the above information. On 01/08/2015 08:53 AM, Ian Lance Taylor wrote: On Thu, Jan 8, 2015 at 5:44 AM, Lynn A. Boger labo...@linux.vnet.ibm.com wrote:I Also as was noted below, some level of reflection support is needed to build Docker. Sorry to be so pedantic, but I'm just trying to clarify that you keep saying reflection support, but reflection support is there. What is missing is a miniscule rarely used part of the reflect package, namely support for reflect.MakeFunc, and, it's true, calling reflect.ValueOf, then Method or MethodByName, then Interface. Ian
Re: [gofrontend-dev] [PATCH 1/2, libgo] Add reflection support to gccgo for ppc64, ppc64le in gcc 4.9
Ian Lance Taylor i...@golang.org writes: On Wed, Jan 7, 2015 at 9:26 AM, Lynn A. Boger labo...@linux.vnet.ibm.com wrote: In libgo/go/reflect/makefunc.go, calls to MakeFunc, makeMethodValue and makeValueMethod will panic if called when GOARCH is ppc64 or ppc64le. Right, I'm just saying that almost no code actually does that. I just tried a web search and found no uses other than examples of how to use it. I'm sure there are a few, but not many. There is a somewhat hidden one in Docker: https://github.com/docker/docker/blob/master/api/client/cli.go#L64 (it is also possible to patch docker to do this differently, of course). Cheers, mwh pgpdSphHFvTaL.pgp Description: PGP signature
Re: [gofrontend-dev] [PATCH 1/2, libgo] Add reflection support to gccgo for ppc64, ppc64le in gcc 4.9
On Wed, Jan 7, 2015 at 4:39 PM, Michael Hudson-Doyle
michael.hud...@canonical.com wrote:
Ian Lance Taylor i...@golang.org writes:
On Wed, Jan 7, 2015 at 9:26 AM, Lynn A. Boger
labo...@linux.vnet.ibm.com wrote:
In libgo/go/reflect/makefunc.go, calls to MakeFunc, makeMethodValue and
makeValueMethod will panic if called when GOARCH is ppc64 or ppc64le.
Right, I'm just saying that almost no code actually does that. I just
tried a web search and found no uses other than examples of how to use
it. I'm sure there are a few, but not many.
There is a somewhat hidden one in Docker:
https://github.com/docker/docker/blob/master/api/client/cli.go#L64
(it is also possible to patch docker to do this differently, of course).
Ah, so that is why it matters.
Perhaps you could talk Docker into replace the return statement with
return func(a ...string) error {
return
method.CallSlice([]reflect.Value{reflect.ValueOf(a)})[0].Interface().(error)
}, true
It would probably be just as efficient.
Ian
Re: [gofrontend-dev] [PATCH 1/2, libgo] Add reflection support to gccgo for ppc64, ppc64le in gcc 4.9
On Wed, Jan 7, 2015 at 8:43 AM, Lynn A. Boger labo...@linux.vnet.ibm.com wrote: I thought that since this only affects ppc64 and ppc64le, and provides reflection support that doesnt exist in gcc 4.9 for Power (you get a panic if you try to use reflection on ppc64, ppc64le), it would be OK. I understand that the code doesn't work today in 4.9, but a release branch is for releases and backports of bug fixes, not for any sort of new development. (As far as I knoew reflection works in general for PPC in 4.9, it's just reflect.MakeFunc, a rarely used feature, that is broken.) Ian
Re: [gofrontend-dev] [PATCH 1/2, libgo] Add reflection support to gccgo for ppc64, ppc64le in gcc 4.9
On Tue, Jan 6, 2015 at 7:37 AM, Lynn A. Boger labo...@linux.vnet.ibm.com wrote: Add support for reflection for gccgo in gcc 4.9. This is not a backport because reflection support in gcc trunk is done using FFI. Bootstrap built and tested on ppc64, ppc64le. Sorry, I didn't see at first that this is not a backport. This is a complex patch that has never been tested on trunk. We don't this kind of change on release branches. For this kind of work you'll need to maintain your own branch. Sorry. Ian
Re: [gofrontend-dev] [PATCH 1/2, libgo] Add reflection support to gccgo for ppc64, ppc64le in gcc 4.9
I thought that since this only affects ppc64 and ppc64le, and provides reflection support that doesnt exist in gcc 4.9 for Power (you get a panic if you try to use reflection on ppc64, ppc64le), it would be OK. We'll look at putting it in our own branch. On 01/07/2015 10:06 AM, Ian Lance Taylor wrote: On Tue, Jan 6, 2015 at 7:37 AM, Lynn A. Boger labo...@linux.vnet.ibm.com wrote: Add support for reflection for gccgo in gcc 4.9. This is not a backport because reflection support in gcc trunk is done using FFI. Bootstrap built and tested on ppc64, ppc64le. Sorry, I didn't see at first that this is not a backport. This is a complex patch that has never been tested on trunk. We don't this kind of change on release branches. For this kind of work you'll need to maintain your own branch. Sorry. Ian
Re: [gofrontend-dev] [PATCH 1/2, libgo] Add reflection support to gccgo for ppc64, ppc64le in gcc 4.9
On Wed, Jan 7, 2015 at 9:26 AM, Lynn A. Boger labo...@linux.vnet.ibm.com wrote: In libgo/go/reflect/makefunc.go, calls to MakeFunc, makeMethodValue and makeValueMethod will panic if called when GOARCH is ppc64 or ppc64le. Right, I'm just saying that almost no code actually does that. I just tried a web search and found no uses other than examples of how to use it. I'm sure there are a few, but not many. Ian
Re: [gofrontend-dev] [PATCH 1/2, libgo] Add reflection support to gccgo for ppc64, ppc64le in gcc 4.9
In libgo/go/reflect/makefunc.go, calls to MakeFunc, makeMethodValue and makeValueMethod will panic if called when GOARCH is ppc64 or ppc64le. I understand your point about what you allow into a release branch and since it has known bugs with an existing testcase then I agree it shouldn't be accepted the way it is. I will work on fixing that and then figure out what branch is best to include it in. On 01/07/2015 10:54 AM, Ian Lance Taylor wrote: On Wed, Jan 7, 2015 at 8:43 AM, Lynn A. Boger labo...@linux.vnet.ibm.com wrote: I thought that since this only affects ppc64 and ppc64le, and provides reflection support that doesnt exist in gcc 4.9 for Power (you get a panic if you try to use reflection on ppc64, ppc64le), it would be OK. I understand that the code doesn't work today in 4.9, but a release branch is for releases and backports of bug fixes, not for any sort of new development. (As far as I knoew reflection works in general for PPC in 4.9, it's just reflect.MakeFunc, a rarely used feature, that is broken.) Ian
