golang / go

The Go programming language
https://go.dev
BSD 3-Clause "New" or "Revised" License
123.62k stars 17.61k forks source link

all: require POWER8 support for ppc64, to match ppc64le #19074

Closed rsc closed 7 years ago

rsc commented 7 years ago

ppc64le already requires POWER8. In contrast, ppc64 (big-endian) requires only POWER5, because when @minux started the port (as a Google intern) he bought a PowerMac G5, to run Debian and serve as a builder. It served well for that, but now we have access to much faster, more modern POWER8 machines for builders.

@laboger has suggested making big-endian match little-endian, requiring POWER8 for both. This matches current PowerPC 64-bit hardware being built by IBM and be used in general: there are very few people running POWER5 at all, much less running Go on them. I think the PowerMac G5 might be the only readily available such system, and it's really interesting only as a novelty at this point.

I second Lynn's proposal to make the two match, so that the only difference between ppc64 and ppc64le is byte order, not architecture support.

I believe this will mean @minux's PowerMac G5 will no longer be able to run Go, but I think he is the only one with such a system, and it's certainly not his primary system for Go development. I do regret that, but we can't reasonably keep an old architecture going for a single user. (Contrast this with ARMv5 where we pre-announced deprecation and a few companies stepped forward saying they'd like it to stay.)

See original mailing list thread on golang-dev for detail.

minux commented 7 years ago

The reason to only use ppc970 instructions for the ppc64 port is not (only) because I have the G5. It's because that's the only easily available platform to run Go on big-endian ppc64 machines. (i.e. you don't need to spend thousands of USD to get one.)

As I mentioned in the email thread, the bigger problem is the general supporting different ISA revisions. We have the same problem for ARM, and we tried the simplest solution of just bumping the required ISA version in

17082, but received many oppositions.

Now I realized that simply dropping the support for older systems is not the best way forward, because we obviously can't keep requiring newer and newer Power processors (we otherwise succeeded in requiring the bare minimum for the 386 and amd64 port). I doubt people using Power systems upgrade once every year. Therefore, at some point, we will have to support multiple Power ISA versions as well.

It's better that we design a way to enable that support now, instead of being lazy and bumping the requirement in the hope that we don't need to support multiple ISA versions.

The new SSA backend actually makes this simpler, because we can have both kind of rewrite rules that use different instructions, but condition on arch version.

My whole point is that, with proper design, supporting both older and newer instruction set version should be essentially free (except having a builder.)

I also want to hear concrete reason why keeping supporting ppc970 is hard. I listed a few reason in #17082 for dropping pre-ARMv6K systems. Note that nearly all new instructions introduced in later ISA versions are intended for specialized packages, and are not interesting to a compiler backend. The only thing I I'm currently aware of is that Power 8 provides dedicated instructions to do integer/FP conversions, so we no longer need to go through memory.

rsc commented 7 years ago

The availability of hardware is not what matters. The use of the hardware is what matters. I am unaware of anyone using Go on POWER5 except you, Minux, and I suspect even you don't use it very often. The machine is too slow!

In contrast, we heard from multiple companies using ARMv5.

laboger commented 7 years ago

Minux, are you asking compatibility with power5 or ppc970 because they aren't the same. We agreed to compatibility with power5 at one time and it was documented that way and I believe some instructions were added last year that were compatible with power5 but not ppc970. This is part of my point. The power5 compatibility is not even being verified and I'm not sure if that is even what you want.

You used the word hard, but my point is that it requires extra work and requires extra "clutter" in the code through #ifdefs and extra files and more complicated if statements. Here are some examples. That is why I am asking this question now. Let's decide this so we know how to proceed in Go 1.9. If we knew of other users I'd have no concern with continuing to maintain it but I don't know of anyone else.

Example 1: the And8 and Or8 atomic functions. On ppc64le they use the LBAR and STBCC because that is really what the code should be doing: get the reservation on the byte, not the word. But because in power5 those instructions don't exist we have to do some extra loads and shifts and get the reservations on the entire word (4 bytes) for ppc64. Now I want these to be intrinsics. I really only care if ppc64le performs better, so I'd like to say don't make these intrinsics, but the way intrinsics are determined in the gc/ssa.go code is based on the family, which is the same for ppc64 and ppc64le so is not specific enough in this case. Yes I can add an extra check to see if this is only ppc64le but in the past I've gotten comments from others in the community about making the code too cluttered when I try to do stuff like this. If we allow these to be intrinsics for both then I have to implement separate code sequences for ppc64le and ppc64. Not hard, just extra work to do and test.

Example 2: Many, many crypto, math, and math/big functions can be improved by using VMX and VSX instructions not available in power5. Most of these can be done through their own ppc64le.s file but there will be some that could be improved by having the compiler generate the instructions directly.

Example 3: I have a better implementation of IndexByte that is not compatible with power5. I could use basically the same implementation for ppc64le and ppc64 (this one does require 2 #ifdefs in the function for endianness differences) if they could use the same instruction set. Otherwise they have to be totally different. There are other byte functions that can be improved that will have the same situation.

Example 4: There is also a separate code sequence in ppc64/ssa.go for the isel instruction because that one is not in power5.

minux commented 7 years ago

I think hardware availability matters a lot.

The reason why ppc64 and ppc64le port is not widely used is precisely because hardware are not easily available. (Compare that to x86 or ARM and you will see the problem. You can get an ARM or ARM64 system capable of running Go applications for less than 30 dollars even, but how much money do you need to pay to get a Power system capable of running Go?) And we will make the problem worse by raising the requirement (and thus make suitable hardware even less available.)

If you say Go is just for commercial vendors, then that's wrong direction for an open source project.

rsc commented 7 years ago

@minux, as far as commercial vs open source, I think you have it backwards. The Go open source project does not aim to make every port freely or cheaply usable by all users, including individuals. Instead, the Go open source project aims to be useful on whatever hardware is commonly used by its users, whether those users are individuals or companies. It is true that individuals have easy access to x86 and arm and arm64, but not to some of the other ports (ppc64le, s390x). That doesn't make those ports less useful or somehow "the wrong direction for an open source project". It's OK to support expensive hardware that individuals would probably not choose to or be able to buy.

Here is a thought experiment. Suppose that we had ppc64 and ppc64le meaning POWER8+ already and that those ports were not reusable for the PowerMac G5, and then suppose someone came to us and said they wanted to add a new port to the PowerMac G5. We'd naturally ask how many users would benefit from such a port. If the answer was "maybe one person", we'd almost certainly say no, it's not worth the maintenance burden.

From my point of view, that's the situation we are in. The team at IBM that is maintaining the ppc64 and ppc64le ports is held back by support for this old system that maybe one person uses (and that might actually already be broken and unreported, per @laboger's comment above). I've been asking for a few days, and there is basically no evidence that this is an important system to even a significant minority of our potential user base. You're the only one who wants this.

You do have a point about architecture variation, but that point doesn't counter the "why are we spending effort to maintain a system that only one person actually uses?" point. We can wait to address architecture variation when we have both an older system that is important to keep working and a newer system that is important to optimize for. Right now we only have the latter.

Finally, if "hardware availability matters a lot", then that's an argument against keeping the PowerMac G5 working too, since it's not generally available in a warrantied, supported form. It was discontinued over ten years ago and the only way to get one is to buy a 10+ year-old system on eBay, install a different OS, and hope it works.

For all these reasons, I am inclined to accept the proposal to make POWER8 the new requirement for the ppc64 port.

laboger commented 7 years ago

I just noticed that according to this page, the ppc64 builder is a power7. I'm not sure if this is current, because it was my understanding that both ppc64 and ppc64le builders were power8.

https://github.com/golang/go/wiki/DashboardBuilders

laboger commented 7 years ago

Brad verified that the builders are both power8 and the wiki has been updated. It does say on this wiki that the ppc64 builder on Minux' ppc970 machine no longer works.

rsc commented 7 years ago

Thanks for investigating. Proposal accepted, for the reasons I gave in my comment 7 days ago.

rsc commented 7 years ago

Should have left open - there's probably documentation or other work to be done.

gopherbot commented 7 years ago

CL https://golang.org/cl/38404 mentions this issue.

gopherbot commented 7 years ago

CL https://golang.org/cl/38406 mentions this issue.

gopherbot commented 7 years ago

CL https://golang.org/cl/39010 mentions this issue.

bradfitz commented 7 years ago

This seems to have happened.

There might be more to do, but it's not a blocker to use POWER8 features in all POWER assembly before marking this closed. It's already noted for the release notes. So, closing.

glaubitz commented 7 years ago

In case some other POWER5 user is disgruntled as I am, getting golang-1.9 to build on POWER5 again is a matter of reverting 189053aee2705a16ed660b7e036e9b6d825c0e9b, c644a76e1f215d4ae2b210e7c9134b2291077ad3, 4a1140472b3093edf9cd653666242393d7f4c2bd. and 475f02cbec3da0a192a20ade170fb7e71542a552 (in that order). Just successfully built golang-1.9 on Debian unstable ppc64 using these changes.

Full patch available in: http://paste.debian.net/984459/

awilfox commented 6 years ago

I appreciate your work @glaubitz; unfortunately, this really does only work for POWER5. I'm trying to package Go for our distribution, Adélie Linux, which is based on the musl libc. We only have a few dozen users right now, but we already have a few vocal users of Go that would love to use it on their Adélie machines. Our PPC64 packages are built with -mcpu=970 and I personally use a Power Mac G5 as my primary workstation, as do a few of our users. (Another is running a business off of an IBM POWER5 blade.) With the patch provided in the last comment, I am still seeing SIGILL in Atomic.Or8 when trying to bootstrap a crossed build from x86_64.

We – users of the 970 and other older-than-P8 PPC64 machines – definitely do still exist, and it is readily apparent to me that there is a non-zero amount of people who use the 970 and want to use Go language on it. Is this at all something that can be put back in Go? If not, perhaps @glaubitz and I could do a deeper dive on finding a way to make it build and run on the 970 in addition to the POWER5 ISA, and I could maintain the patches locally with our Go supporter/volunteer.

Thank you.

glaubitz commented 6 years ago

Our PPC64 packages are built with -mcpu=970 and I personally use a Power Mac G5 as my primary workstation, as do a few of our users.

That's actually the baseline we use in Debian as well, we have recently even disabled AltiVec by default because many users run Debian ppc64 on Freescale E5500 machines.

With the patch provided in the last comment, I am still seeing SIGILL in Atomic.Or8 when trying to bootstrap a crossed build from x86_64.

Can you run the binary in gdb and run "x/i" with the address of the SIGILL instruction? Then we can figure out what instruction caused the problem and I can look which patch I need to revert.

We – users of the 970 and other older-than-P8 PPC64 machines – definitely do still exist, and it is readily apparent to me that there is a non-zero amount of people who use the 970 and want to use Go language on it. Is this at all something that can be put back in Go?

I suggest you make a post to golang-dev and I'll join the discussion.

I also still don't understand why the baseline for ppc64be was raised to POWER8 because there is no operating system according to my best knowledge which runs in POWER8 big-endian mode. Virtually everyone using POWER8 hardware is running a little-endian operating system.

I have the impression that IBM folk want to forcefully deprecate older POWER machines to drive their POWER8/9 sales. I have also seen similar attempts from IBM folk in other projects where they tried to dismiss patches for supporting older POWER versions by claiming they were deprecated as if IBM is the only manufacturer of POWER hardware, ignoring companies like Freescale.

Luckily, other projects like Rust-Lang and OpenJDK have a better attitude than Golang in this regard and there IBM folk hasn't yet managed to force older POWER hardware into deprecation.

laboger commented 6 years ago

It is probably the change in go 1.8 for frim, friz, and frip that forced the power5 requirement. Reverting that change along with the other changes you already have in your patch should provide compatibility with the 970 instruction set.

On Mon, Nov 27, 2017 at 4:14 AM, John Paul Adrian Glaubitz < notifications@github.com> wrote:

Our PPC64 packages are built with -mcpu=970 and I personally use a Power Mac G5 as my primary workstation, as do a few of our users.

That's actually the baseline we use in Debian as well, we have recently even disabled AltiVec by default because many users run Debian ppc64 on Freescale E5500 machines.

With the patch provided in the last comment, I am still seeing SIGILL in Atomic.Or8 when trying to bootstrap a crossed build from x86_64.

@awilfox https://github.com/awilfox Can you run the binary in gdb and run "x/i" with the address of the SIGILL instruction? Then we can figure out what instruction caused the problem and I can look which patch I need to revert.

We – users of the 970 and other older-than-P8 PPC64 machines – definitely do still exist, and it is readily apparent to me that there is a non-zero amount of people who use the 970 and want to use Go language on it. Is this at all something that can be put back in Go?

I suggest you make a post to golang-dev and I'll join the discussion.

I also still don't understand why the baseline for ppc64be was raised to POWER8 because there is no operating system according to my best knowledge which runs in POWER8 big-endian mode. Virtually everyone using POWER8 hardware is running a little-endian operating system.

I have the impression that IBM folk wants to forcefully deprecate older POWER machines to drive their POWER8/9 sales. I have also seen similar attempts from IBM folk in other projects where they tried to dismiss patches for supporting older POWER versions by claiming they were deprecated as if IBM is the only manufacturer of POWER hardware, ignoring companies like Freescale.

Luckily, other projects like Rust-Lang and OpenJDK have a better attitude than Golang in this regard and there IBM folk hasn't yet managed to force older POWER hardware into deprecation.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/golang/go/issues/19074#issuecomment-347136757, or mute the thread https://github.com/notifications/unsubscribe-auth/AI_wjLXljC0ygnKNR__WW5iF8Q7Nbehiks5s6ouNgaJpZM4L_7Qi .

awilfox commented 6 years ago

Thanks @laboger, I've reverted 3311275c. Still seeing SIGILL in atomic.Or8:

Program received signal SIGILL, Illegal instruction.
0x0000000000048b1c in runtime.check () at /usr/src/aports/community/go/src/go/src/runtime/runtime1.go:257
257             atomic.Or8(&m[1], 0xf0)
(gdb) x/8i ($pc - 8)
   0x48b14 <runtime.check+516>: li      r5,-16
   0x48b18 <runtime.check+520>: hwsync
=> 0x48b1c <runtime.check+524>: lbarx   r31,0,r4
   0x48b20 <runtime.check+528>: or      r31,r31,r5
   0x48b24 <runtime.check+532>: stbcx.  r31,0,r4
   0x48b28 <runtime.check+536>: bne     0x48b1c <runtime.check+524>
   0x48b2c <runtime.check+540>: isync
   0x48b30 <runtime.check+544>: lbz     r5,72(r1)

LBARX is POWER8. This should also crash e5500 as it wasn't added until e6500. Then again, I don't see LI in the old or new version of atomic.Or8. Anyway, this should have been fixed with the reversion of 189053a, so I'm not sure why it's still there. Perhaps I need to dig deeper and ensure that the Go toolchain that generates this toolchain has the patch applied properly.

glaubitz commented 6 years ago

@awilfox Thanks for the heads-up. I will revert https://github.com/golang/go/commit/3311275ce8eef87a64b78589e0da0bf115f9be07 and https://github.com/golang/go/commit/189053aee2705a16ed660b7e036e9b6d825c0e9b in my tree.

I also wasn't implying that it works on e5500. Debian's ppc64 buildds are actually POWER7 and POWER8 which why the two missing reverts slipped through and why the builds failed only sporadically.

glaubitz commented 6 years ago

@awilfox After revertung 3311275, I'm now running into:

# bootstrap/cmd/compile/internal/ssa
/home/glaubitz/go-linux-ppc64-bootstrap/src/cmd/compile/internal/ssa/opGen.go:18125:17: error: reference to undefined identifier ‘ppc64.AFRIM’
   asm:    ppc64.AFRIM,
                 ^
/home/glaubitz/go-linux-ppc64-bootstrap/src/cmd/compile/internal/ssa/opGen.go:18138:17: error: reference to undefined identifier ‘ppc64.AFRIP’
   asm:    ppc64.AFRIP,
                 ^
/home/glaubitz/go-linux-ppc64-bootstrap/src/cmd/compile/internal/ssa/opGen.go:18151:17: error: reference to undefined identifier ‘ppc64.AFRIZ’
   asm:    ppc64.AFRIZ,
                 ^
go tool dist: FAILED: /usr/bin/go install -gcflags=-l -tags=math_big_pure_go compiler_bootstrap bootstrap/cmd/...: exit status 2

I have done a git blame which leads to 0f19e24da7f which can't be easily reverted though.

I assume you're working with golang-1.9 instead of git master, correct?

laboger commented 6 years ago

Try this.

On Tue, Nov 28, 2017 at 6:14 AM, John Paul Adrian Glaubitz < notifications@github.com> wrote:

@awilfox https://github.com/awilfox After revertung 3311275 https://github.com/golang/go/commit/3311275ce8eef87a64b78589e0da0bf115f9be07, I'm now running into:

bootstrap/cmd/compile/internal/ssa

/home/glaubitz/go-linux-ppc64-bootstrap/src/cmd/compile/internal/ssa/opGen.go:18125:17: error: reference to undefined identifier ‘ppc64.AFRIM’ asm: ppc64.AFRIM, ^ /home/glaubitz/go-linux-ppc64-bootstrap/src/cmd/compile/internal/ssa/opGen.go:18138:17: error: reference to undefined identifier ‘ppc64.AFRIP’ asm: ppc64.AFRIP, ^ /home/glaubitz/go-linux-ppc64-bootstrap/src/cmd/compile/internal/ssa/opGen.go:18151:17: error: reference to undefined identifier ‘ppc64.AFRIZ’ asm: ppc64.AFRIZ, ^ go tool dist: FAILED: /usr/bin/go install -gcflags=-l -tags=math_big_pure_go compiler_bootstrap bootstrap/cmd/...: exit status 2

I have done a git blame which leads to 0f19e24 https://github.com/golang/go/commit/0f19e24da7fa564af3fa4e831463951d5715211a which can't be easily reverted though.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/golang/go/issues/19074#issuecomment-347504784, or mute the thread https://github.com/notifications/unsubscribe-auth/AI_wjLlglho9jcIVs9Rzf_utltePaas9ks5s6_kRgaJpZM4L_7Qi .

index 0bd5cea..633bdc2 100644
--- a/src/cmd/compile/internal/gc/ssa.go
+++ b/src/cmd/compile/internal/gc/ssa.go
@@ -2808,17 +2808,35 @@ func init() {
        func(s *state, n *Node, args []*ssa.Value) *ssa.Value {
            return s.newValue1(ssa.OpTrunc, types.Types[TFLOAT64], args[0])
        },
-       sys.PPC64, sys.S390X)
+       sys.S390X)
    addF("math", "Ceil",
        func(s *state, n *Node, args []*ssa.Value) *ssa.Value {
            return s.newValue1(ssa.OpCeil, types.Types[TFLOAT64], args[0])
        },
-       sys.PPC64, sys.S390X)
+       sys.S390X)
    addF("math", "Floor",
        func(s *state, n *Node, args []*ssa.Value) *ssa.Value {
            return s.newValue1(ssa.OpFloor, types.Types[TFLOAT64], args[0])
        },
-       sys.PPC64, sys.S390X)
+       sys.S390X)
+   add("math", "Trunc",
+       func(s *state, n *Node, args []*ssa.Value) *ssa.Value {
+           return s.newValue1(ssa.OpTrunc, types.Types[TFLOAT64], args[0])
+       },
+       sys.ArchPPC64LE)
+
+   add("math", "Ceil",
+       func(s *state, n *Node, args []*ssa.Value) *ssa.Value {
+           return s.newValue1(ssa.OpCeil, types.Types[TFLOAT64], args[0])
+       },
+       sys.ArchPPC64LE)
+
+   add("math", "Floor",
+       func(s *state, n *Node, args []*ssa.Value) *ssa.Value {
+           return s.newValue1(ssa.OpFloor, types.Types[TFLOAT64], args[0])
+       },
+       sys.ArchPPC64LE)
+
    addF("math", "Round",
        func(s *state, n *Node, args []*ssa.Value) *ssa.Value {
            return s.newValue1(ssa.OpRound, types.Types[TFLOAT64], args[0])
diff --git a/src/math/floor_ppc64x.s b/src/math/floor_ppc64x.s
deleted file mode 100644
index 2ab011d..0000000
--- a/src/math/floor_ppc64x.s
+++ /dev/null
@@ -1,25 +0,0 @@
-// Copyright 2016 The Go Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style
-// license that can be found in the LICENSE file.
-
-// +build ppc64 ppc64le
-
-#include "textflag.h"
-
-TEXT ·Floor(SB),NOSPLIT,$0
-   FMOVD   x+0(FP), F0
-   FRIM    F0, F0
-   FMOVD   F0, ret+8(FP)
-   RET
-
-TEXT ·Ceil(SB),NOSPLIT,$0
-   FMOVD   x+0(FP), F0
-   FRIP    F0, F0
-   FMOVD   F0, ret+8(FP)
-   RET
-
-TEXT ·Trunc(SB),NOSPLIT,$0
-   FMOVD   x+0(FP), F0
-   FRIZ    F0, F0
-   FMOVD   F0, ret+8(FP)
-   RET
diff --git a/src/math/stubs_ppc64x.s b/src/math/stubs_ppc64x.s
index 30c51dd..273bca4 100644
--- a/src/math/stubs_ppc64x.s
+++ b/src/math/stubs_ppc64x.s
@@ -57,6 +57,15 @@ TEXT ·Frexp(SB),NOSPLIT,$0
 TEXT ·Hypot(SB),NOSPLIT,$0
    BR ·hypot(SB)

+TEXT ·Floor(SB),NOSPLIT,$0
+   BR ·floor(SB)
+
+TEXT ·Ceil(SB),NOSPLIT,$0
+   BR ·ceil(SB)
+
+TEXT ·Trunc(SB),NOSPLIT,$0
+   BR ·trunc(SB)
+
 TEXT ·Ldexp(SB),NOSPLIT,$0
    BR ·ldexp(SB)
laboger commented 6 years ago

Make sure the old pkg and bin directories are removed before you do a build from source.

On Tue, Nov 28, 2017 at 1:08 AM, A. Wilcox notifications@github.com wrote:

Thanks @laboger https://github.com/laboger, I've reverted 3311275 https://github.com/golang/go/commit/3311275ce8eef87a64b78589e0da0bf115f9be07. Still seeing SIGILL in atomic.Or8:

Program received signal SIGILL, Illegal instruction. 0x0000000000048b1c in runtime.check () at /usr/src/aports/community/go/src/go/src/runtime/runtime1.go:257 257 atomic.Or8(&m[1], 0xf0) (gdb) x/8i ($pc - 8) 0x48b14 <runtime.check+516>: li r5,-16 0x48b18 <runtime.check+520>: hwsync => 0x48b1c <runtime.check+524>: lbarx r31,0,r4 0x48b20 <runtime.check+528>: or r31,r31,r5 0x48b24 <runtime.check+532>: stbcx. r31,0,r4 0x48b28 <runtime.check+536>: bne 0x48b1c <runtime.check+524> 0x48b2c <runtime.check+540>: isync 0x48b30 <runtime.check+544>: lbz r5,72(r1)

LBARX is POWER8. This should also crash e5500 as it wasn't added until e6500. Then again, I don't see LI in the old or new version of atomic.Or8. Anyway, this should have been fixed with the reversion of 189053a https://github.com/golang/go/commit/189053aee2705a16ed660b7e036e9b6d825c0e9b, so I'm not sure why it's still there. Perhaps I need to dig deeper and ensure that the Go toolchain that generates this toolchain has the patch applied properly.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/golang/go/issues/19074#issuecomment-347433269, or mute the thread https://github.com/notifications/unsubscribe-auth/AI_wjIfyrRX1PILhhXsYTFmYuz4EhJWzks5s67GBgaJpZM4L_7Qi .

glaubitz commented 6 years ago

@laboger Could you put your diff inside triple backticks by editing your comment? I cannot copy the patch otherwise as github messes up the formatting. Thanks!

laboger commented 6 years ago

golang-legacy.diff.txt

glaubitz commented 6 years ago

Doesn't help, unfortunately. The references to these functions are still present:

# bootstrap/cmd/compile/internal/ssa
/home/glaubitz/go-linux-ppc64-bootstrap/src/cmd/compile/internal/ssa/opGen.go:18125:17: error: reference to undefined identifier ‘ppc64.AFRIM’
   asm:    ppc64.AFRIM,
                 ^
/home/glaubitz/go-linux-ppc64-bootstrap/src/cmd/compile/internal/ssa/opGen.go:18138:17: error: reference to undefined identifier ‘ppc64.AFRIP’
   asm:    ppc64.AFRIP,
                 ^
/home/glaubitz/go-linux-ppc64-bootstrap/src/cmd/compile/internal/ssa/opGen.go:18151:17: error: reference to undefined identifier ‘ppc64.AFRIZ’
   asm:    ppc64.AFRIZ,
                 ^
go tool dist: FAILED: /usr/bin/go install -gcflags=-l -tags=math_big_pure_go compiler_bootstrap bootstrap/cmd/...: exit status 2
laboger commented 6 years ago

Sorry I wasn't clear on this. Don't revert 3311275, which added the FRIP, FRIM, and FRIZ, since they will still be needed for ppc64le. Just apply this patch instead, and that will leave the instructions available but prevent them from being generated for ppc64 big endian binaries. If you are getting these errors that means you reverted (removed) their definitions from a.out.go or asm9.go.

I was able to do a successful build on ppc64 power8 with this patch (I don't have access to any hardware older than power7).

glaubitz commented 6 years ago

Yes, this works. Thanks a lot! Will push this into my POWER5 repo.

glaubitz commented 6 years ago

@awilfox I have pushed Lynn's patch to my rebase power5 repo now, can you give it a try?

See: https://github.com/glaubitz/go/tree/power5

laboger commented 6 years ago

I appreciate your work @glaubitz; unfortunately, this really does only work for POWER5. I'm trying to package Go for our distribution, Adélie Linux, which is based on the musl libc. We only have a few dozen users right now, but we already have a few vocal users of Go that would love to use it on their Adélie machines. Our PPC64 packages are built with -mcpu=970 and I personally use a Power Mac G5 as my primary workstation, as do a few of our users. (Another is running a business off of an IBM POWER5 blade.) With the patch provided in the last comment, I am still seeing SIGILL in Atomic.Or8 when trying to bootstrap a crossed build from x86_64.

One note on support for ppc64 big endian. Just because you are able to generate code with the right instruction set doesn't mean you will have full support for golang on ppc64 big endian. For one thing, full linker support was never added for ppc64 big endian, which means that you can't use the external linker no matter what instruction set or distro you use. It is my understanding that Alpine expects to be able to link using -pie, which requires the external linker on ppc64, and the support to use the external linker on ppc64 big endian was never added.

glaubitz commented 6 years ago

@laboger Thanks for the heads-up!

So far, golang doesn't seem to use an external linker on Debian as far as I know and the compiler works well enough on ppc64be.

The main reason why we still need ppc64be support in golang is because gccgo - according to Debian's Go maintainers - has still too many issues that they trust it to compile any Go packages. From my current knowledge, lots of packages built with gccgo will just crash or not build at all while they work just fine when using golang as the compiler. Thus, the Debian Go maintainers are building all their packages exclusively with golang and use gccgo solely for bootstrapping the golang compiler.

This is why I was very unhappy with the fact that golang upstream dropped ppc64be because gccgo currently is not a usable replacement for it, unfortunately.

laboger commented 6 years ago

If the problems with gccgo are not reported, then they can't be fixed. Just saying.

Send me an example of one of the packages that crashes when built with gccgo on ppc64 big endian, I'd like to know what those problems are. I know golang is preferred by the communities because it is easier to build, etc., but AFAIK as a compiler gccgo should work as well or better than golang on ppc64 big endian. I keep hearing rumors that it doesn't but no one reports those problems or provides specific examples of why it is unusable.

On Thu, Nov 30, 2017 at 8:31 AM, John Paul Adrian Glaubitz < notifications@github.com> wrote:

@laboger https://github.com/laboger Thanks for the heads-up!

So far, golang doesn't seem to use an external linker on Debian as far as I know and the compiler works well enough on ppc64be.

The main reason why we still need ppc64be support in golang is because gccgo - according to Debian's Go maintainers - has still too many issues that they trust it to compile any Go packages. From my current knowledge, lots of packages built with gccgo will just crash or not build at all while they work just fine when using golang as the compiler. Thus, the Debian Go maintainers are building all their packages exclusively with golang and use gccgo solely for bootstrapping the golang compiler.

This is why I was very unhappy with the fact that golang upstream dropped ppc64be because gccgo currently is not a usable replacement for it, unfortunately.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/golang/go/issues/19074#issuecomment-348203818, or mute the thread https://github.com/notifications/unsubscribe-auth/AI_wjJDMuJAjFDl0_EeWSdwnucx7OAmrks5s7rwngaJpZM4L_7Qi .

glaubitz commented 6 years ago

Yes, I will report these issues once I find the time for it.

I'm not one of Debian's Go maintainers, so I don't know which packages are affected. I am just repeating what they told me. It's also not specific to ppc64 but apparently gccgo is inferior to golang in general and has these issues. I think the main problem is that 99% of all Go developers write and test their code using golang and there is no continuous integration running for gccgo anywhere.

I'm myself just a porter for various architectures and my main focus is on fixing issues once they arise. I have recently been very busy with Rust and OpenJDK on various architectures, but I will certainly get back to Go at some point. There is still the Linux/sparc64 port of Golang I would like to finish.

I will get in touch Debian's Go maintainers and ask them though.

Xe commented 6 years ago

I have recently acquired POWER5 test hardware and would like to help with POWER5 maintenance. I do not currently have a working linux install on it, but as soon as I do I will run a full bisect of Go from the last version I can get working to go 1.9.

ianlancetaylor commented 6 years ago

For what it's worth, gccgo tests are routinely run and reported on the gcc-testresults mailing list. Here are some recent results on ppc64be: https://gcc.gnu.org/ml/gcc-testresults/2017-12/msg00048.html .

glaubitz commented 6 years ago

@Xe Cool! You might might interested in Debian for ppc64 then. It is an unofficial port of Debian for ppc64 and supported by the Debian Ports people. We're hanging out in #debian-ports on OFTC.

As for golang-1.9, if I understand correctly, my current fork should work fine on POWER5 (or even POWER4 which corresponds to Apple's PowerPC 970 found in the G5 machines). I haven't found the time yet though to test compile the fork on my Apple PowerMac G5 which is currently not set up.

@ianlancetaylor It's not so much about gccgo being unreliable on ppc64be but rather gccgo in general not ready for prime-time, at least according to Debian's Go maintainers. But I said, I need to set up a test environment and see how many of the ~800 packages will build.

ceseo commented 6 years ago

@glaubitz your go1.9 fork should work if you did not pick up my changes from the time when we deprecated code older than POWER8. Otherwise, there are plenty of new implementations in runtime/asm_ppc64x.s and math/big/arith_ppc64x.s that use newer instructions and will crash on POWER5.

Xe commented 6 years ago

Is this still a problem?

awilfox commented 6 years ago

As of 1.10 and #24358, yes, it is.

This is still blocking Go being packaged in Adélie, and this is still preventing others on RHEL7, Debian, etc from using the Go packages that are available for their ppc64 systems (if the CPU is < POWER8 or < e6500).

glaubitz commented 6 years ago

@awilfox I have switched the default Go compiler for "ppc64" on Debian to gccgo now to work around this issue. Most of the packages build fine, although there are some that cause problems when being built with gccgo instead of golang. I am planning to report those packages to gccgo upstream as Ian Lance Taylor asked me to. But I am currently busy with other projects.

@Xe As for golang, I have just rebased my POWER5 branch and resolved all the merge conflicts. Let's see if it still builds. Overall, there is not much effort necessary to get golang to build fine on POWER5.

NattyNarwhal commented 6 years ago

Another +1 for a problem.

I don't see much purpose in having ppc64 be POWER8 only; the pool of possible users is small as:

It seems that making big endian ppc64 require POWER8 is a de facto deprecation without actually doing a deprecation, as the platform support matrices are in a way that makes the user base for big endian POWER8 Linux extremely small. Your best routes are to consider allowing ppc64be tolerate pre-POWER8 or just send the port to a farm upstate.

awilfox commented 6 years ago

Ping. This is still blocking us. Will there be any resolution? gccgo doesn't work properly on musl due to ucontext issues, so that didn't end up being a solution.

We're still blocking on this, a full year later, and it's actually prevented us from doing things like using GitLab CI (which requires Go) or writing some internal projects using Go…

ianlancetaylor commented 6 years ago

@awilfox This issue is closed, so pinging here won't help. I'm not aware of any plans to change this decision.

If you have problems using gccgo, please report them.

DariuszOstolski commented 5 years ago

I was investigating whether golang can be used on embedded platforms like e5500 and was supposed to leave this without comment but I decided to express my opinion so community voice has been heard.

I don't understand why such decision was made. I had plans to make an application for embedded platform and this is a blocker for me and I think it is for everybody who is investigating whether golang can be used.

In my opinion this is classical egg and chicken problem: we won't support because there aren't too many users and there aren't too many users because a given platform is not well supported.

I can't afford to support a custom branch just to be able to build it has to be officially supported.

bradfitz commented 5 years ago

@DariuszOstolski, you didn't say why gccgo doesn't work for you.

glaubitz commented 5 years ago

@bradfitz gccgo doesn't build all packages that golang builds. I know it's supposed to be compatible, but it isn't. I can provide a list of Debian packages which don't build.

See, for example, etcd: https://buildd.debian.org/status/fetch.php?pkg=etcd&arch=ppc64&ver=3.2.18%2Bdfsg-1&stamp=1539989343&raw=0

Fails with:

# github.com/coreos/etcd/pkg/cpuutil
src/github.com/coreos/etcd/pkg/cpuutil/endian.go:31:13: error: array bound is not constant
  if v := (*[intWidth]byte)(unsafe.Pointer(&i)); v[0] == 0 {
             ^
src/github.com/coreos/etcd/pkg/cpuutil/endian.go:31:51: error: array index out of bounds
  if v := (*[intWidth]byte)(unsafe.Pointer(&i)); v[0] == 0 {
                                                   ^

Builds fine with golang: https://buildd.debian.org/status/fetch.php?pkg=etcd&arch=ppc64el&ver=3.2.18%2Bdfsg-1&stamp=1523405069&raw=0

bradfitz commented 5 years ago

@glaubitz, anytime you find a case where any two Go compilers have different behavior, please file a bug. We consider that a big problem and try to fix those bugs quickly and will add new tests to make sure it doesn't recur in the future.

@ianlancetaylor et al will fix gccgo bugs quickly. That's a more fruitful avenue for getting old ppc64 support, because it's not coming back to the golang.org compiler.

bradfitz commented 5 years ago

I filed https://github.com/golang/go/issues/28601 for the gccgo issue.

awilfox commented 5 years ago

old ppc64 support

You realise POWER7 was the current generation from 2010-2014, right? You have deprecated something that is newer than Intel Sandy Bridge, and was the current POWER CPU when Haswell came out.

The golang Go compiler also supports ARM going back to ARMv6 because of niche uses like smartphones and the RasPi 1. But you won't support a CPU more than 4 years old that has a larger (and obviously more vocal) userbase – ranging from hobbyists with Apple G5, newer computers based on the AmigaOne designs, people with 4 year old IBM servers, etc?

I would have let this thread die, but calling anything pre-POWER8 "old" is downright insulting to the community.

I'll also note that we can't get any support for gccgo bugs because none of the versions newer than 6 branch work for our distribution. GCC 7 has horrifying regressions on basically "anything that isn't x86"; GCC 8 uses 3-5× the RAM over GCC 6 for the same workloads, which means less packages being built at one time because our builders have to have less jobs running at once. Go ahead, tell me to go complain to the GCC maintainers. Even when 6 was a supported branch they didn't care about us, and they won't even fix bugs present in 7 and 8. So when we have issues with gccgo such as "segfaulting on ppc64" (and only ppc64), we are shown the door.

At this point, we're probably going to have to drop Go from our repositories.

DariuszOstolski commented 5 years ago

In embedded world those devices will have to be supported for at least 20 years and e5500 is a very powerful processor even now.

friedkiwi commented 5 years ago

Also - another reason to support POWER5-levels is a (potential) port to AIX and IBM i. Doing this would cover the other operating systems in the POWER ecosystem. IBM i is binary compatible with AIX, however, it SIGILLs on anything above POWER5.

The argument wrt. cost of hardware also remains valid - even POWER7 machines still sell for thousands of USD in the second hand market, and POWER8 machines are unobtainium. I have several customers who are running P5 and P6 in prod and will be for years to come because of the price point of P8 and P9 and that there is no entry level market for their needs.