Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cmd/link: external linking for ppc64le doesn't work #11184

Closed
laboger opened this issue Jun 12, 2015 · 19 comments

Comments

Projects
None yet
6 participants
@laboger
Copy link
Contributor

commented Jun 12, 2015

Currently external linking is not enabled for ppc64le in golang master probably because it doesn't work as is, but there are some existing CLs that mostly provide the support needed to make external linking work with some minor modifications. I'm using Ubuntu 14.04 and 14.10.

Following are the list of errors that occur. I've noted the files from existing CLs that were used to resolve the errors.

  1. is needed to enable external linking with ppc64le, otherwise an error occurs at link time.
  2. is needed to enabled base support for Power relocations, otherwise errors occur concerning undefined relocations
  3. is needed for cgo to work
  4. is needed to correctly handle the runtime.tlsg symbol which appears in tls_ppc64x.s. Otherwise runtime segv's occur when running programs built with external linking.

Here are the files from the CLs with notes on what needs changing.

  1. https://go-review.googlesource.com/#/c/9673/3/src/cmd/9l/obj.go
    Change the compare string from "ppc64le" to "ppc" to enable external linking for ppc64le. Without this external linking on ppc64le is not enabled. File is now in a different directory cmd/link/internal/ppc64.
  2. https://go-review.googlesource.com/#/c/9673/3/src/cmd/9l/asm.go
    Base support for Power specific relocations. This file has been moved to a different directory so the patches need to be applied to the new file in cmd/link/internal/ppc64
  3. https://go-review.googlesource.com/#/c/9676/4/src/runtime/cgo.go
    Needed
  4. https://go-review.googlesource.com/#/c/9677/4/src/cmd/9l/asm.go
    New file asm.go is in cmd/link/internal/ppc64/
    Additional support needed for Tlsg symbol. When the TPREL relocation is used then 0x7000 must be added to the offset since the TPREL relocation subtracts off that amount and the tls_ppc64x.s code also subtracts off 0x7000.

I don't understand the status of these CLs, especially those that cannot be merged anymore because the files are in different directories. Can someone clarify? It was be good if we could get these changes in so external linking for ppc64le can be enabled and made to work.

@ianlancetaylor ianlancetaylor added this to the Go1.5 milestone Jun 12, 2015

@ianlancetaylor

This comment has been minimized.

Copy link
Contributor

commented Jun 12, 2015

@mwhudson

This comment has been minimized.

Copy link
Contributor

commented Jul 14, 2015

FWIW, I have a branch that implements this. It's a bit tangled up with other things but I'll get it submitted in early 1.6 I hope. One detail that isn't noted is that you can't always translate R_ADDRPOWER to a R_PPC_ADDR16_HA / R_PPC_ADDR16_LO pair, sometimes it has to be R_PPC64_ADDR16_LO_DS because it is applied to loads as well as addi, and a few loads (lwa, for example) are DS form.

@laboger

This comment has been minimized.

Copy link
Contributor Author

commented Jul 14, 2015

It would be nice if we could get the changes in that I outlined above. Almost all the changes already exist in various CLs from Minux but were never merged, or in the case of some, are now in files that have moved to other directories. With these changes, external linking for ppc64le at least works in most cases (although I agree there are probably some bugs as noted in the previous comment). Without these changes it doesn't work at all.

@rsc rsc modified the milestones: Go1.6, Go1.5, Go1.6Early Jul 21, 2015

rsc added a commit that referenced this issue Aug 18, 2015

cmd/go: disable TestNoteReading on solaris, linux/ppc64le
Update #11184 (linux/ppc64).
Filed #12178 (solaris) for Go 1.6.

Change-Id: I9e3a456aaccb49590ad4e14b53ddfefca5b0801c
Reviewed-on: https://go-review.googlesource.com/13679
Reviewed-by: Russ Cox <rsc@golang.org>
@laboger

This comment has been minimized.

Copy link
Contributor Author

commented Sep 3, 2015

@mwhudson In you last post you said you hoped to get this in early Go 1.6. Do you know when that might be? There are users who would like to try this out at least on ppc64le. Thanks.

@minux

This comment has been minimized.

Copy link
Member

commented Sep 3, 2015

@laboger

This comment has been minimized.

Copy link
Contributor Author

commented Sep 3, 2015

This issue shows what changes are needed from your original CLs to make it work on ppc64le at least for what I tried. Michael also pointed out a few additional problems with some relocations which I haven't tried to use.

I am willing to help build and test on ppc64le, just let me know when you have patches ready.

Thanks.

@mwhudson

This comment has been minimized.

Copy link
Contributor

commented Sep 3, 2015

I can post my changes today, I hope.

@mwhudson

This comment has been minimized.

Copy link
Contributor

commented Sep 3, 2015

I've just pushed up a series of changes

remote: New Changes:
remote:   https://go-review.googlesource.com/14233 cmd/internal/obj, cmd/link, runtime: handle TLS more like a platform linker ...
remote:   https://go-review.googlesource.com/14234 cmd/internal/obj, cmd/link: replace R_ADDRPOWER with a pair of relocations
remote:   https://go-review.googlesource.com/14235 cmd/internal/obj, cmd/link: handle the fact that a few store/loads on ppc64 ...
remote:   https://go-review.googlesource.com/14236 cmd/link: enable external linking on ppc64
remote:   https://go-review.googlesource.com/14237 runtime/cgo: something that is apparently needed for external linking on ppc64
remote:   https://go-review.googlesource.com/14238 cmd/dist: run more cgo tests on ppc64x

They pass ./all.bash on ppc64le.

@laboger

This comment has been minimized.

Copy link
Contributor Author

commented Sep 9, 2015

I was able to build this, and tried a few simple cases of external linking on ppc64le, static and dynamic and they worked as I expected.

@laboger

This comment has been minimized.

Copy link
Contributor Author

commented Sep 21, 2015

What is the outlook on this? If ppc64 for big endian is still an issue, can we at least get this support merged for ppc64le?

@mwhudson

This comment has been minimized.

Copy link
Contributor

commented Sep 22, 2015

The branches are just waiting for reviewer time, I think. The simple ones have been approved.

@laboger

This comment has been minimized.

Copy link
Contributor Author

commented Sep 23, 2015

Thanks for the response.

Please note that the dynamic linker for ppc64le (ABI v2) is ld64.so.2, not ld64.so.1 as is for ppc64.

@laboger

This comment has been minimized.

Copy link
Contributor Author

commented Oct 2, 2015

I might have made a mistake in my previous testing, I am no longer able to pull your CLs and build a toolchain that works for ppc64le. There are multiple dependent CLs and I'm not sure how to get all the right code. Any suggestions on how to do that would be helpful, or maybe they need to be rebased.

These CLs haven't been looked at for over two weeks.

@mwhudson

This comment has been minimized.

Copy link
Contributor

commented Oct 6, 2015

On 3 October 2015 at 01:36, laboger notifications@github.com wrote:

I might have made a mistake in my previous testing, I am no longer able to
pull your CLs and build a toolchain that works for ppc64le. There are
multiple dependent CLs and I'm not sure how to get all the right code. Any
suggestions on how to do that would be helpful, or maybe they need to be
rebased.

I did rework some things, but you should be able to get the latest version
by going to the final CL in the series,
https://go-review.googlesource.com/#/c/14238/, clicking download in the top
right and using the "checkout" option (which is currently "git fetch
https://go.googlesource.com/go refs/changes/38/14238/6 && git checkout
FETCH_HEAD" but that will change if I upload a new version).

These CLs haven't been looked at for over two weeks.

Yep :/

@laboger

This comment has been minimized.

Copy link
Contributor Author

commented Oct 12, 2015

I have built and tested with these CLs on ppc64le and they seem to work as expected. Thanks! I will try ppc64 as well.

@mwhudson

This comment has been minimized.

Copy link
Contributor

commented Oct 12, 2015

Good to hear! I have no way of testing ppc64 and wasn't thinking at all
about that as I did this, so I really don't know if that will work.

On 13 October 2015 at 01:40, laboger notifications@github.com wrote:

I have built and tested with these CLs on ppc64le and they seem to work as
expected. Thanks! I will try ppc64 as well.


Reply to this email directly or view it on GitHub
#11184 (comment).

@laboger

This comment has been minimized.

Copy link
Contributor Author

commented Oct 21, 2015

Rebuilt with your latest rebase and all continued to work for ppc64le.
I think support for pcp64le needs to get upstream first then an issue against ppc64 big endian can be written.

@mwhudson

This comment has been minimized.

Copy link
Contributor

commented Nov 10, 2015

I've just submitted the changes that should fix this (somehow I failed to notice Russ had reviewed them)

@laboger

This comment has been minimized.

Copy link
Contributor Author

commented Nov 12, 2015

This is working as expected now for ppc64le.

@rsc rsc closed this Nov 12, 2015

@golang golang locked and limited conversation to collaborators Nov 16, 2016

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
You can’t perform that action at this time.