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

Merge rustc_codegen_gcc backend as compiler/rustc_codegen_gcc #442

Closed
1 of 3 tasks
antoyo opened this issue Jun 24, 2021 · 10 comments
Closed
1 of 3 tasks

Merge rustc_codegen_gcc backend as compiler/rustc_codegen_gcc #442

antoyo opened this issue Jun 24, 2021 · 10 comments
Labels
major-change A proposal to make a major change to rustc major-change-accepted A major change proposal that was accepted T-compiler Add this label so rfcbot knows to poll the compiler team

Comments

@antoyo
Copy link

antoyo commented Jun 24, 2021

Proposal

rustc_codegen_gcc is a new code generation backend for rustc using the libgccjit library from GCC. (Despite its name, libgccjit works for ahead-of-time compilation as well.) rustc_codegen_gcc will allow Rust to target the wider set of architectures that GCC supports. It'll also allow us to generate code optimized via GCC, which in some cases can provide better code generation.

This MCP proposes incorporating rustc_codegen_gcc into rust-lang/rust as compiler/rustc_codegen_gcc (using git subtree), alongside the other code generation backends. This MCP also proposes gating CI on rustc_codegen_gcc building, but not on it passing any tests.

rustc_codegen_gcc currently passes the entire core testsuite; work on the remainder of the testsuite is in progress. rustc_codegen_gcc benefits from the existing infrastructure to annotate tests as requiring a specific backend, so that it doesn't attempt to pass LLVM-specific tests.

If this MCP is accepted, we'll subsequently submit PRs adding it to rust-lang/rust and adding it to the build process. We'll also make a PR to the highfive bot, to automatically CC @antoyo on changes to compiler/rustc_codegen_gcc. In the future, we'll make a separate proposal to distribute rustc_codegen_gcc via rustup.

Licensing

rustc_codegen_gcc uses the same license as rustc: dual MIT / Apache-2.0. The libgccjit library that rustc_codegen_gcc depends on uses the same license as GCC: GPLv3-or-later. This won't affect users of rustc at all, and it won't affect distributors of rustc who do not build or distribute the GCC backend.

Distributors of rustc (including the Rust project itself) who do choose to build or distribute the GCC backend will need to provide the full source for their distribution of rustc under a GPL-compatible Open Source license; rustc and all its dependencies are under GPL-compatible Open Source licenses, so in practice this just means that distributors of rustc who choose to build and distribute the GCC backend need to supply full source code. This does not seem like a practical issue, nor does it change rustc's normal permissive licensing policy, as anyone who wishes to use rustc under a permissive license may simply avoid building or distributing the GCC backend.

We hope that in practice, Linux distributions will build and distribute the GCC backend once it passes enough of the testsuite to be widely useful, and especially once we have targets that depend on it. Other distributors of rustc may choose whether to build and distribute the GCC backend based on their needs.

We will never make any portion of rustc other than rustc_codegen_gcc depend on libgccjit.

Given the value of a GCC backend in expanding Rust's reach to more targets, and thus enabling the use of Rust in projects that need to continue supporting such targets, we believe this represents a reasonable step that will not in practice affect anyone's use, development, or distribution of rustc.

Authors

@antoyo is the primary author of rustc_codegen_gcc, and will continue to maintain it once merged.

@joshtriplett helped with this MCP, and provided guidance and recommendations on licensing.

Mentors or Reviewers

Not sure who to put here.

Process

The main points of the Major Change Process are as follows:

  • File an issue describing the proposal.
  • A compiler team member or contributor who is knowledgeable in the area can second by writing @rustbot second.
    • Finding a "second" suffices for internal changes. If however, you are proposing a new public-facing feature, such as a -C flag, then full team check-off is required.
    • Compiler team members can initiate a check-off via @rfcbot fcp merge on either the MCP or the PR.
  • Once an MCP is seconded, the Final Comment Period begins. If no objections are raised after 10 days, the MCP is considered approved.

You can read more about Major Change Proposals on forge.

Comments

@antoyo antoyo added major-change A proposal to make a major change to rustc T-compiler Add this label so rfcbot knows to poll the compiler team labels Jun 24, 2021
@rustbot
Copy link
Collaborator

rustbot commented Jun 24, 2021

This issue is not meant to be used for technical discussion. There is a Zulip stream for that. Use this issue to leave procedural comments, such as volunteering to review, indicating that you second the proposal (or third, etc), or raising a concern that you would like to be addressed.

cc @rust-lang/compiler @rust-lang/compiler-contributors

@rustbot rustbot added the to-announce Announce this issue on triage meeting label Jun 24, 2021
@jonas-schievink
Copy link
Contributor

@rustbot second

@LeSeulArtichaut LeSeulArtichaut added the final-comment-period The FCP has started, most (if not all) team members are in agreement label Jun 24, 2021
dkm added a commit to dkm/compiler-explorer that referenced this issue Jun 28, 2021
GCC backend for rustc is still in a very early state.
It is in the process of being merged in main rustc source:
rust-lang/compiler-team#442

Currently reusing main rust compiler class and simply remove -Cllvm= argument if
any (only for intel asm syntax).

refs compiler-explorer#2683
dkm added a commit to dkm/compiler-explorer that referenced this issue Jun 28, 2021
GCC backend for rustc is still in a very early state.
It is in the process of being merged in main rustc source:
rust-lang/compiler-team#442

Currently reusing main rust compiler class and simply remove -Cllvm= argument if
any (only for intel asm syntax).

refs compiler-explorer#2683
dkm added a commit to dkm/compiler-explorer that referenced this issue Jun 30, 2021
GCC backend for rustc is still in a very early state.
It is in the process of being merged in main rustc source:
rust-lang/compiler-team#442

Currently reusing main rust compiler class and simply remove -Cllvm= argument if
any (only for intel asm syntax).

refs compiler-explorer#2683
@apiraino apiraino removed the to-announce Announce this issue on triage meeting label Jul 1, 2021
@nellshamrell
Copy link

Hello all,

Nell Shamrell-Harrington from the Rust Foundation board here.

The Foundation has reviewed this issue with its counsel and we agree with the conclusions about licensing in the rustc_codegen_gcc change proposal. Specifically, we agree that:

(1) The proposed change will not impact the license of rustc except when it is built with the gcc backend, in which case GPLv3 will apply to the resulting binary. Distribution of that binary will be subject to the terms of GPLv3.
(2) rustc_codegen_gcc may be licensed under the Rust project licenses (MIT & Apache v2), because they are compatible with GPLv3.

In our review, we noticed a couple of related issues that we recommend addressing before the code is merged:

(1) The rustc_codegen_gcc repo contains a folder of patches to libgccjit, a GPLv3-licensed work. Section 4 of GPLv3 requires the following when distributing modifications in source code form:
Include a copy of the GPLv3 license.
Include "prominent notices" that the patches modify libgccjit, along with "a relevant date." (The information in the patch files is probably adequate here.)
Include "prominent notices" that the work (i.e. the patch set) is licensed under GPLv3. The top-level NOTICE file looks like the right place for this.
(2) There is a mailing list thread asking the developer of libgccjit whether the library is subject to the GCC Runtime Library Exception. The developer referred the question to FSF, which apparently never answered. This is only an issue if code from libgccjit itself is compiled into target binaries, which may not be the case, but may be worth running down with the developer.

@antoyo
Copy link
Author

antoyo commented Jul 2, 2021

Thanks!
I made the requested changes and asked the question on the mailing list.

@joshtriplett
Copy link
Member

joshtriplett commented Jul 2, 2021

@nellshamrell Thanks for digging into this!

Regarding issue (1): I think it'd be safer to just remove any GCC patches from the rustc_codegen_gcc repository, and keep those in a separate repository. (That separate repository still needs to follow all the requirements of the GPL, as outlined; I just think we should avoid having such patches in the Rust source repository.)

Regarding issue (2), libgccjit isn't a runtime library and never gets linked into target binaries; it generates code that calls libgcc for that. libgccjit is not licensed under the runtime library exception, and isn't intended to be. (I'm hoping that question doesn't generate any acrimony from GCC, as it might be misinterpreted to imply that we want to link proprietary code with it, which we emphatically don't.) libgccjit is part of GCC, and embodies GCC's code generation, so GCC's copyleft is very much intended to apply to it without exception.

@davidmalcolm
Copy link

I'm the author/maintainer of libgccjit.

For better or worse, the FSF holds copyright on libgccjit (FWIW, I used to be OK with this, but I've been reconsidering my views on the FSF lately ...but that's a whole other issue).

libgccjit is a GPLv3 library, in particular, it's essentially a thin wrapper around GCC's implementation (but designed to work as a shared library rather than a command-line tool). Despite the name, it can also be run as an ahead-of-time compiler, which is how this project is using it..

As I understand it, any host code directly linking with libgccjit needs to comply with the GPLv3, but the target code generated by libgccjit isn't affected by the GPLv3 (but might link against the target libgcc runtime library, which has its own license); this is analogous to the classic usage of GCC as a command-line tool. My understanding is that the FSF is OK with GCC being used to develop code under other licenses (including proprietary), and GCC's license only affects that code in-as-much as it links to the target libgcc runtime library (which is under a different license). It might be worth having your counsel check that license.

FWIW, I'm a Rust fanboy, and hope that this project succeeds; I have no problem with it myself.

That said, I'm not a lawyer, and am not qualified to give legal advice. I don't speak for my employer (Red Hat), or for the FSF.

@joshtriplett
Copy link
Member

@davidmalcolm Thanks so much for your kind and detailed response, David!

dkm added a commit to dkm/compiler-explorer that referenced this issue Jul 6, 2021
GCC backend for rustc is still in a very early state.
It is in the process of being merged in main rustc source:
rust-lang/compiler-team#442

Currently reusing main rust compiler class and simply remove -Cllvm= argument if
any (only for intel asm syntax).

Disabling binary until the result is more friendly (currently binary are too
big).

refs compiler-explorer#2683
dkm added a commit to dkm/compiler-explorer that referenced this issue Jul 7, 2021
GCC backend for rustc is still in a very early state.
It is in the process of being merged in main rustc source:
rust-lang/compiler-team#442

Currently reusing main rust compiler class and simply remove -Cllvm= argument if
any (only for intel asm syntax).

Disabling binary until the result is more friendly (currently binary are too
big).

refs compiler-explorer#2683
@apiraino
Copy link
Contributor

apiraino commented Jul 8, 2021

@rustbot label -final-comment-period +major-change-accepted

@apiraino apiraino closed this as completed Jul 8, 2021
@rustbot rustbot added major-change-accepted A major change proposal that was accepted to-announce Announce this issue on triage meeting and removed final-comment-period The FCP has started, most (if not all) team members are in agreement labels Jul 8, 2021
@apiraino apiraino removed the to-announce Announce this issue on triage meeting label Jul 8, 2021
dkm added a commit to dkm/compiler-explorer that referenced this issue Jul 8, 2021
GCC backend for rustc is still in a very early state.
It is in the process of being merged in main rustc source:
rust-lang/compiler-team#442

Currently reusing main rust compiler class and simply remove -Cllvm= argument if
any (only for intel asm syntax).

Disabling binary until the result is more friendly (currently binary are too
big).

refs compiler-explorer#2683
dkm added a commit to dkm/compiler-explorer that referenced this issue Jul 9, 2021
GCC backend for rustc is still in a very early state.
It is in the process of being merged in main rustc source:
rust-lang/compiler-team#442

Currently reusing main rust compiler class and simply remove -Cllvm= argument if
any (only for intel asm syntax).

Disabling binary until the result is more friendly (currently binary are too
big).

refs compiler-explorer#2683
mattgodbolt pushed a commit to compiler-explorer/compiler-explorer that referenced this issue Jul 12, 2021
GCC backend for rustc is still in a very early state.
It is in the process of being merged in main rustc source:
rust-lang/compiler-team#442

Currently reusing main rust compiler class and simply remove -Cllvm= argument if
any (only for intel asm syntax).

Disabling binary until the result is more friendly (currently binary are too
big).

refs #2683
mattgodbolt pushed a commit to compiler-explorer/compiler-explorer that referenced this issue Aug 2, 2021
GCC backend for rustc is still in a very early state.
It is in the process of being merged in main rustc source:
rust-lang/compiler-team#442

Currently reusing main rust compiler class and simply remove -Cllvm= argument if
any (only for intel asm syntax).

Disabling binary until the result is more friendly (currently binary are too
big).

refs #2683
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this issue Sep 28, 2021
…mulacrum

Libgccjit codegen

This PR introduces a subtree for a gcc-based codegen backend to the repository, per decision in rust-lang/compiler-team#442. We do not yet expect to ship this backend on nightly or run tests in CI, but we do verify that the backend checks (i.e., `cargo check`) successfully.

Work is expected to progress primarily in https://github.com/rust-lang/rustc_codegen_gcc, with semi-regular upstreaming, like with other subtrees.
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this issue Sep 28, 2021
…mulacrum

Libgccjit codegen

This PR introduces a subtree for a gcc-based codegen backend to the repository, per decision in rust-lang/compiler-team#442. We do not yet expect to ship this backend on nightly or run tests in CI, but we do verify that the backend checks (i.e., `cargo check`) successfully.

Work is expected to progress primarily in https://github.com/rust-lang/rustc_codegen_gcc, with semi-regular upstreaming, like with other subtrees.
@TruncatedDinoSour

This comment has been minimized.

@TruncatedDinoSour

This comment has been minimized.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
major-change A proposal to make a major change to rustc major-change-accepted A major change proposal that was accepted T-compiler Add this label so rfcbot knows to poll the compiler team
Projects
None yet
Development

No branches or pull requests

9 participants