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

Jump to bottom

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

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

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

laboger opened this issue Jun 12, 2015 · 19 comments
Labels
FrozenDueToAge
Milestone
Go1.6Early

Comments

@laboger
Copy link
Contributor

laboger 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
Copy link
Contributor

@minux @aclements

@mwhudson
Copy link
Contributor

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
Copy link
Contributor Author

laboger 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
@rsc
cmd/go: disable TestNoteReading on solaris, linux/ppc64le
92c984e 
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
Copy link
Contributor Author

laboger 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
Copy link
Member

minux commented Sep 3, 2015 via email

@laboger
Copy link
Contributor Author

laboger 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
Copy link
Contributor

mwhudson commented Sep 3, 2015

I can post my changes today, I hope.

@mwhudson
Copy link
Contributor

mwhudson 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
Copy link
Contributor Author

laboger 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
Copy link
Contributor Author

laboger 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
Copy link
Contributor

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

@laboger
Copy link
Contributor Author

laboger 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
Copy link
Contributor Author

laboger 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
Copy link
Contributor

mwhudson 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
Copy link
Contributor Author

laboger 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
Copy link
Contributor

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 laboger mentioned this issue Oct 15, 2015
Closed
@laboger
Copy link
Contributor Author

laboger 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
Copy link
Contributor

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

@laboger
Copy link
Contributor Author

laboger commented Nov 12, 2015

This is working as expected now for ppc64le.

@rsc rsc closed this as completed 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.
Assignees
No one assigned
Labels
FrozenDueToAge
Projects
None yet
Milestone
Go1.6Early
Development

No branches or pull requests
6 participants
@rsc @minux @mwhudson @ianlancetaylor @gopherbot @laboger