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

Open
rsc opened this Issue Feb 14, 2017 · 9 comments

Projects

None yet

3 participants

@rsc
Contributor
rsc commented Feb 14, 2017

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
Member
minux commented Feb 14, 2017
@rsc
Contributor
rsc commented Feb 14, 2017

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
Contributor
laboger commented Feb 14, 2017

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
Member
minux commented Feb 15, 2017
@rsc
Contributor
rsc commented Feb 15, 2017

@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
Contributor
laboger commented Feb 21, 2017

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
Contributor
laboger commented Feb 22, 2017

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
Contributor
rsc commented Feb 22, 2017

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

@rsc rsc closed this Feb 22, 2017
@rsc rsc changed the title from proposal: all: drop POWER5 support for ppc64, to match ppc64le to all: drop POWER5 support for ppc64, to match ppc64le Feb 22, 2017
@rsc rsc added this to the Go1.9Early milestone Feb 22, 2017
@rsc rsc reopened this Feb 22, 2017
@rsc
Contributor
rsc commented Feb 22, 2017

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

@rsc rsc changed the title from all: drop POWER5 support for ppc64, to match ppc64le to all: require POWER8 support for ppc64, to match ppc64le Feb 22, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment