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

all: port to linux/loong64 #46229

Open
XiaodongLoong opened this issue May 18, 2021 · 87 comments
Open

all: port to linux/loong64 #46229

XiaodongLoong opened this issue May 18, 2021 · 87 comments

Comments

@XiaodongLoong
Copy link

@XiaodongLoong XiaodongLoong commented May 18, 2021

Hi community,

I am from Loongson company (R & D CPU), and we have developed a new RISC CPU instruction architecture named LoongArch.

We have successfully ported linux/loongarch64 in Go version 1.15.6, and want to submit some patches to the Go community to support this port.

To support linux/loongarch64 port, we have a plan:
(1) provide a builder machine (Loongson 3A5000/2.2GHz,debian-buster / 4.19 linux-kernel) remotely.
(2) submit the patches in August (after Go 1.17 release).
(3) complete the submission and review of all patches by November and be stable state.
(4) maintain linux/loongarch64 port and builder machine for a long time.

@XiaodongLoong
Copy link
Author

@XiaodongLoong XiaodongLoong commented May 18, 2021

@dmitshur dmitshur added this to the Proposal milestone May 18, 2021
@dmitshur dmitshur changed the title all: port to linux/loongarch64 proposal: all: port to linux/loongarch64 May 18, 2021
@dmitshur
Copy link
Contributor

@dmitshur dmitshur commented May 18, 2021

@mengzhuo
Copy link
Contributor

@mengzhuo mengzhuo commented May 19, 2021

I think maybe "loong64"/"la64" is more appropriate since "loongarch64" is simply too loong ( pun :)

@ianlancetaylor ianlancetaylor added this to Incoming in Proposals May 19, 2021
@ianlancetaylor
Copy link
Contributor

@ianlancetaylor ianlancetaylor commented May 19, 2021

This seems fine to me. Can you provide links to a description of the architecture?

I take it that you are suggesting a GOARCH value of loongarch64.

@dr2chase
Copy link
Contributor

@dr2chase dr2chase commented May 19, 2021

I prefer not "la64" because it might be confused with "IA64" which is the usual acronym for Intel's Itanium.
I think "loong64" is good; a GOARCH value that itself contains the substring "arch" is redundant, we don't do that for any other.

@rsc
Copy link
Contributor

@rsc rsc commented May 19, 2021

This proposal has been added to the active column of the proposals project
and will now be reviewed at the weekly proposal review meetings.
— rsc for the proposal review group

@rsc rsc moved this from Incoming to Active in Proposals May 19, 2021
@XiaodongLoong
Copy link
Author

@XiaodongLoong XiaodongLoong commented May 20, 2021

@ianlancetaylor We will provide an architecture instruction manual in the near future and I will update a link on this issue. As you said we use GOARCH value of loongarch64 in our Go version 1.15.6.

@mengzhuo @dr2chase Thanks for your suggestion, we will consider it.

Thank you all.

@tklauser
Copy link
Member

@tklauser tklauser commented May 20, 2021

It seems like the instruction is MIPS64 compatible? How does Loongson differ from MIPS64?

I see that e.g. in GCC, in binutils and in the Linux kernel the architecture is handled as a variant of the MIPS/MIPS64 architecture. Could we do this in a similar way for Go and avoid having to introduce a separate GOARCH?

@XiaodongLoong
Copy link
Author

@XiaodongLoong XiaodongLoong commented May 20, 2021

@tklauser The Loongson you said is not the LoongArch. Loongson is an extension of MIPS64 instruction set, but LoongArch is not. LoongArch is a new RISC CPU instruction architecture.

@tklauser
Copy link
Member

@tklauser tklauser commented May 20, 2021

@XiaodongLoong thanks for clarifying

@XiaodongLoong
Copy link
Author

@XiaodongLoong XiaodongLoong commented May 21, 2021

The description of LoongArch architecture: https://github.com/loongson/LoongArch-Documentation/releases

@rsc
Copy link
Contributor

@rsc rsc commented May 26, 2021

Are people happy with GOARCH=loong64?

If the builder will be at Loongson in China, do you know whether it will be able to connect to our build infrastructure running in Google data centers? The remote builder connects to Google's servers; Google's servers never dial back in the other direction.

@mengzhuo
Copy link
Contributor

@mengzhuo mengzhuo commented May 27, 2021

Are people happy with GOARCH=loong64?

If the builder will be at Loongson in China, do you know whether it will be able to connect to our build infrastructure running in Google data centers? The remote builder connects to Google's servers; Google's servers never dial back in the other direction.

GOARCH=loong64 sounds good to me.

There are three reverse builders that in China up and running for about 2 years:

  • host-linux-amd64-wsl (2)
  • host-linux-mipsle-mengzhuo

@xen0n
Copy link
Contributor

@xen0n xen0n commented May 27, 2021

I second the loong64 suggestion, considering e.g. we already use arm64 for that architecture officially named aarch64.

@XiaodongLoong
Copy link
Author

@XiaodongLoong XiaodongLoong commented May 31, 2021

The proposal on the GOARCH=loong64 is entirely feasible in a technology perspective. But this is related to the business brands, intellectual property and so on. We have launched an internal discussion on this issues and I will update the discussion results as soon as possible.

Thanks.

@mengzhuo
Copy link
Contributor

@mengzhuo mengzhuo commented May 31, 2021

I don't see why GOARCH=loong64 is related to IP or brand. What it matter is conciseness and correctness ( el vs le)

Intel x86_64 = amd64
Arm v8 (aarch64) = arm64

Microprocessor without Interlocked Pipelined Stages = MIPS -> mips
Performance Optimization With Enhanced RISC – Performance Computing = PPC -> ppc

Since Loongarch64 is short for "Loongson architecture version 64bit ", I think GOARCH=loong64 is good enough for this very new RISC architecture.

@rsc
Copy link
Contributor

@rsc rsc commented Jun 2, 2021

Thanks @XiaodongLoong. We will wait for your response.

@XiaodongLoong
Copy link
Author

@XiaodongLoong XiaodongLoong commented Jun 5, 2021

@rsc After a week of internal discussion, we combine the community's proposal with our reality, and finally produced discussion results with difficulty. Considering brand promotion, registration of trademark and the unity of naming, we want to continue to use GOARCH=loongarch64.

@mengzhuo @xen0n Thank you for your advice, sincerely.

@tduslost
Copy link

@tduslost tduslost commented Jun 5, 2021

happy with GOARCH=loong64

@dongzerun
Copy link

@dongzerun dongzerun commented Jun 5, 2021

happy with GOARCH=loong64 too

@bigwhite
Copy link

@bigwhite bigwhite commented Jun 5, 2021

prefer GOARCH=loong64

@xen0n
Copy link
Contributor

@xen0n xen0n commented Jun 5, 2021

I'm sorry but the "brand promotion" argument is just way off limits. Do you really consider "impressing" people with the 9-letter word loongarch appearing verbatim everywhere a good marketing strategy? Or that even an enum value never used in front of the general public should be trademarked? While we're at it, in order to better highlight the brand and to comply with trademark law, do we optionally accept GOARCH=LoongArch™️64 too?

Non-technical points aside, the point @dr2chase brought forward earlier is valid, and the "discussion result" fails to provide justification as to why the current convention should absolutely be broken. The Go language and community has a reputation for being rather opinionated than most others; for GOARCH values, apart from aarch64 -> arm64 we also have mipsel -> mipsle, x86_64 -> amd64 for example. Go never decides on a value just because the GNU or LLVM triple says so, much less a vendor's unilateral statement.

Finally, remember most community members not part of the full-time Go team are volunteers, devoting their free time to Go, and in this thread, LoongArch, without rewards. Appearing grateful to community input and contributions but in fact pushing one's own agenda without changes or clear communication is not good, and the softer part of our hearts can certainly feel that. Not placing your trademark in GOARCH is perfectly fine, people will still know of the brand, and a lot of typing would be saved. 😂

@willliu
Copy link

@willliu willliu commented Jun 6, 2021

One shortcoming to use loong64 instead of loongarch64 is the potential future confusion, since loongarch64 seems intended used for ISA name, linux kernel, java runtime, compiler and everything.

@xen0n
Copy link
Contributor

@xen0n xen0n commented Jun 6, 2021

One shortcoming to use loong64 instead of loongarch64 is the potential future confusion, since loongarch64 seems intended used for ISA name, linux kernel, java runtime, compiler and everything.

And no one seems to mistake arm64 or amd64; users are smart enough to figure out things. Go, or any other project not using target triples, is not obliged to follow each other; what's important is following respective established conventions. IOW, “入乡随俗”——“when in Rome, do as Romans do”.

The argument against GOARCH=loongarch64 is never meant to be delibrate opposition or personal attack; instead it's purely technical and about convention and consistency.

Also, a kind reminder to @mengzhuo : maybe calm down a bit and don't start to call names? We have no hard evidence that @willliu is actually @XiaodongLoong , so it's probably not time to make this kind of statement.

However, I agree the account is suspicious, because the LoongArch ports of Linux and Java are never sent upstream, and the toolchain sources are withdrawn after only a short time, I think.

@gopherbot
Copy link

@gopherbot gopherbot commented Aug 15, 2021

Change https://golang.org/cl/342325 mentions this issue: misc, test: port to linux/loong64

@xen0n
Copy link
Contributor

@xen0n xen0n commented Aug 15, 2021

@XiaodongLoong What's these CLs about? I think we've communicated privately about the cooperation details as to who does what component, and I've already put out an initial revision of cmd/internal/obj and assembler port. And now a half-heartedly done MIPS-like port is submitted, neglecting all the previous work we did, and is of inferior quality?

@XiaodongLoong
Copy link
Author

@XiaodongLoong XiaodongLoong commented Aug 16, 2021

Per the request #46229 (comment), we have rebased from go1.15.6 to master branch and submitted patches to gerrit.
Unfortunately, a few test cases executed by all.bash can not pass, but we will continue to fix that.
We look forward to see your code review comments.

Thanks!

@xen0n
Copy link
Contributor

@xen0n xen0n commented Aug 16, 2021

Per the request #46229 (comment), we have rebased from go1.15.6 to master branch and submitted patches to gerrit.
Unfortunately, a few test cases executed by all.bash can not pass, but we will continue to fix that.
We look forward to see your code review comments.

Thanks!

tl;dr: I've already reviewed the code, and the conclusion was that a rewrite is needed to meet upstream requirements, but you didn't listen and posted the CLs nevertheless. I strongly object to basing the whole port on the original MIPS code, and carrying MIPS-isms around (like the assembly mnemonics and so on), so I won't comment on the CLs individually.

We communicated privately about the expected quality standards, and I think we all agreed that a MIPS-based port is not acceptable; so I put forward a proposal that I implement the assembler/linker side. After getting the cmd/internal/obj interface reviewed and merged, the SSA and runtime work can then begin on Loongson's side. You and your leader expressed concern about "punctuality" issues, but @mengzhuo and I both think it's impossible to sneak loong64 into go1.18 this cycle; but after voicing our opinions, no replies from Loongson side were ever heard. I think there's something wrong with the cooperation mechanism; maybe you should review my earlier CLs too, to push things forward responsibly?

@mengzhuo
Copy link
Contributor

@mengzhuo mengzhuo commented Aug 16, 2021

@XiaodongLoong I think you might want to check the Porting Policy instead of submitting those CLs without testes and reviews.

All CLs necessary to run all.bash successfully must have been sent for review. Typically this will be a handful of CLs split by the part of the tree they change.
Once those conditions are satisfied, the Go team can accept the port and begin to merge the CLs. Once the CLs are all submitted, all.bash must pass, so that the builder reports "ok" in the dashboard.

p.s. Is any qualified developer want to maintain this port? @xen0n is it ok for you?

@xen0n
Copy link
Contributor

@xen0n xen0n commented Aug 16, 2021

@XiaodongLoong I think you might want to check the Porting Policy instead of submitting those CLs without testes and reviews.

All CLs necessary to run all.bash successfully must have been sent for review. Typically this will be a handful of CLs split by the part of the tree they change.
Once those conditions are satisfied, the Go team can accept the port and begin to merge the CLs. Once the CLs are all submitted, all.bash must pass, so that the builder reports "ok" in the dashboard.

p.s. Is any qualified developer want to maintain this port? @xen0n is it ok for you?

I think if it's my code that eventually gets merged, then I should take part in maintaining it, obviously. Currently no substantial loong64 code is in, so I'm holding up my CL for component ownership. Let's finish the code first.

@XiaodongLoong
Copy link
Author

@XiaodongLoong XiaodongLoong commented Aug 17, 2021

@mengzhuo Thank you for your reminding. We will obey the policies of the community, and continue to complete the rest of work.

@xen0n
(1)We just discussed the possibility of refactoring, but we did not reach a unified consensus.
(2)There is a precedent for the new port based on the existing port, such as mips based on ppc64.
(3)We have done a lot of work on MIPS. For example, we support buildmode=c-shared on linux/mips64le. So we chosed to develop LoongArch based on MIPS naturally.
(4)The patches is a response to the comment(#46229 (comment)). If it is necessary to refactor code in the future, you are welcome to contribute code.

Thanks!

@xen0n
Copy link
Contributor

@xen0n xen0n commented Aug 17, 2021

Hi,

@xenon

You just misspelled my name; my ID has the number 0, not upper-case O.

(1)We just discussed the possibility of refactoring, but we did not reach a unified consensus.

And I thought silence is not exactly the way civilized people express their disagreement?

(2)There is a precedent for the new port based on the existing port, such as mips based on ppc64.

These are ports from the old times; at the time mips was ported, for example, the coding style and conventions are still considered current at that time. That doesn't automatically make the decision to base a new port on these old-style converted-from-C code valid IMHO.

(3)We have done a lot of work on MIPS. For example, we support buildmode=c-shared on linux/mips64le. So we chosed to develop LoongArch based on MIPS naturally.

But LoongArch is indigenously developed and unrelated to MIPS, at least from publicly available material that I have permission to look at; when looking from a userland perspective it's more similar to RISC-V instead (except the FCC thing, that certainly looks MIPS-y). To abandon all the MIPS legacy we clearly need to start from something new, instead of building on the previous pile of code. What's more, why build all things on top of MIPS when you're actually a brand-new unrelated arch?

(4)The patches is a response to the comment(#46229 (comment)). If it is necessary to refactor code in the future, you are welcome to contribute code.

First of all, it's of course good to keep your own promises, that builds trust. However, I take the Go team's acceptance of the proposal as a general clearance for accepting the loong64 port, and not about recognizing the roadmap you outlined as valid and actionable. And the requirement about coding standards should be considered implicit; in other words, you can't throw out this kind of code and hope to get it into acceptable shape in a mere 3 months.

Secondly, as for the contributions, I've already submitted my CLs such as CL 339010 and the whole series earlier, and I posted everything in the private communication groupchat before actually sending the series. I got no review from your side; @mengzhuo was always quick though. Disagreement about my approach is okay, but I heard no response; then you posted all these CLs per your agenda. And I always reviewed these as soon as I had time, and I still think copying MIPS is not acceptable, so we really need to get together and figure out how to get your things done, both quickly and on a fresh starting point. I already have experience upstreaming to various projects including Go, so I hope my approach would help and enable you to finish your objective sooner.

I look forward to your review and comments, and thank you for your kindness to communicate with us in the first place.

Thanks!

steeve added a commit to znly/go that referenced this issue Aug 19, 2021
Per discussion at golang#46229 we are taking the "loong64" GOARCH value for
the upcoming LoongArch 64-bit port. It is not clear whether any 32-bit
non-bare-metal userland will exist for LoongArch, so only reserve
"loong64" for now.

Change-Id: I97d262b4ab68ff61c22ccf83e26baf70eefd568d
GitHub-Last-Rev: ecdd8c5
GitHub-Pull-Request: golang#47129
Reviewed-on: https://go-review.googlesource.com/c/go/+/333909
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Trust: Alexander Rakoczy <alex@golang.org>
steeve added a commit to znly/go that referenced this issue Aug 19, 2021
For golang#46229

Change-Id: I54d01d90f2b0c892d76121f1350c0e8cf4b2772f
Reviewed-on: https://go-review.googlesource.com/c/go/+/334729
Trust: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
XiaodongLoong added a commit to loongson/build that referenced this issue Aug 26, 2021
XiaodongLoong added a commit to loongson/build that referenced this issue Aug 30, 2021
XiaodongLoong added a commit to loongson/build that referenced this issue Aug 31, 2021
XiaodongLoong added a commit to loongson/build that referenced this issue Sep 2, 2021
XiaodongLoong added a commit to loongson/build that referenced this issue Sep 2, 2021
@gopherbot
Copy link

@gopherbot gopherbot commented Sep 2, 2021

Change https://golang.org/cl/347309 mentions this issue: dashboard: add linux-loong64-3a5000 builder

gopherbot pushed a commit that referenced this issue Sep 6, 2021
Updates #46229

Change-Id: Icb736f2440443e9245872b091d13e5bdfb6cb01a
Reviewed-on: https://go-review.googlesource.com/c/go/+/339009
Reviewed-by: Meng Zhuo <mzh@golangcn.org>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Trust: Meng Zhuo <mzh@golangcn.org>
Trust: Michael Knyszek <mknyszek@google.com>
Run-TryBot: Meng Zhuo <mzh@golangcn.org>
TryBot-Result: Go Bot <gobot@golang.org>
@gopherbot
Copy link

@gopherbot gopherbot commented Sep 11, 2021

Change https://golang.org/cl/349269 mentions this issue: cmd/internal/objabi: port to linux/loong64

@gopherbot
Copy link

@gopherbot gopherbot commented Sep 11, 2021

Change https://golang.org/cl/349291 mentions this issue: cmd/internal/obj/loong64: port to linux/loong64

@gopherbot
Copy link

@gopherbot gopherbot commented Sep 11, 2021

Change https://golang.org/cl/349290 mentions this issue: cmd/internal/obj: port to linux/loong64

@gopherbot
Copy link

@gopherbot gopherbot commented Sep 13, 2021

Change https://golang.org/cl/349514 mentions this issue: cmd/link: port to linux/loong64

@gopherbot
Copy link

@gopherbot gopherbot commented Sep 13, 2021

Change https://golang.org/cl/349512 mentions this issue: cmd/link/internal/loong64: port to linux/loong64

@gopherbot
Copy link

@gopherbot gopherbot commented Sep 13, 2021

Change https://golang.org/cl/349510 mentions this issue: cmd/asm/internal: port to linux/loong64

@gopherbot
Copy link

@gopherbot gopherbot commented Sep 13, 2021

Change https://golang.org/cl/349513 mentions this issue: cmd/link/internal: port to linux/loong64

@gopherbot
Copy link

@gopherbot gopherbot commented Sep 13, 2021

Change https://golang.org/cl/349511 mentions this issue: cmd/compile: port to linux/loong64

gopherbot pushed a commit to golang/build that referenced this issue Sep 16, 2021
Updates golang/go#46229

Change-Id: Ib48e1e64d96a07d5c5282d78935124faf75c89d7
Reviewed-on: https://go-review.googlesource.com/c/build/+/347309
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
Reviewed-by: Carlos Amedee <carlos@golang.org>
Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Trust: Cherry Mui <cherryyz@google.com>
@gopherbot
Copy link

@gopherbot gopherbot commented Sep 29, 2021

Change https://golang.org/cl/353032 mentions this issue: api: api check for linux/loong64

@gopherbot
Copy link

@gopherbot gopherbot commented Oct 15, 2021

Change https://golang.org/cl/356089 mentions this issue: go/analysis: add support for loong64

creack pushed a commit to creack/pty that referenced this issue Oct 16, 2021
loong64 GOARCH value reserved for LoongArch architecture:
	https://golang.org/doc/go1.17.

github issues:
	golang/go#46229

LoongArch documents:
	https://loongson.github.io/LoongArch-Documentation/LoongArch-Vol1-EN.html

Signed-off-by: guoqi.chen <chenguoqi@loongson.cn>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Proposals
Accepted
Linked pull requests

Successfully merging a pull request may close this issue.

None yet