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

RFC: Demote i686-pc-windows-gnu to Tier 2 #3771

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

Noratrieb
Copy link
Member

@Noratrieb Noratrieb commented Feb 11, 2025

Co-authored-by: Jubilee Young <workingjubilee@gmail.com>
@Noratrieb Noratrieb added T-compiler Relevant to the compiler team, which will review and decide on the RFC. T-release Relevant to the release team, which will review and decide on the RFC. labels Feb 11, 2025
Copy link
Member

@jieyouxu jieyouxu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm in favor of this, left some remarks / additional context which are not meant to be requested changes.

Comment on lines +57 to +58
While some of these issues apply to all GNU-based targets, 32-bit x86 seems to be especially affected.
And when a 32-bit Windows GNU issue comes up, contributors rarely actually investigate it, because it is such a complex and nonstandard environment compared to 64-bit Windows GNU, which is a lot easier to set up and work with.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remark: one such example: rust-lang/rust#136795

Comment on lines +78 to +82
> The target maintainer team must include at least 3 developers.

This is not the case at all. There is currently no maintainer team.
Though we should note that this is currently also true for many other Tier 1 targets, as this is a new rule not upheld everywhere yet.
But experience tells that it is highly unlikely that 3 maintainers for 32 bit Windows GNU will be found.
Copy link
Member

@jieyouxu jieyouxu Feb 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remark: and I think it's arguably misleading to say (or "promise", for that matter) i686-pc-windows-gnu is a Tier 1 target when it does not really receive Tier 1 maintenance support IMHO.


## Conclusion

Given the usage count and lack of maintenance leading to more than one requirement not being fulfilled, it becomes clear that `i686-pc-windows-gnu` is not worthy of being a Tier 1 target and is already getting much worse support than expected from a Tier 1 target.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remark: arguably, it also imposes a maintenance burden on compiler contributors because if a test fails on just i686-mingw (32-bit windows-gnu) but not 64-bit windows-{msvc,gnu} it's very annoying to bless that.

Comment on lines +163 to +165
`x86_64-pc-windows-gnu` will remain a Tier 1 target after this RFC.
While its popularity is more aligned with Tier 1, it suffers from the same lack of maintenance (but to a lesser degree) as its 32-bit cousin.
It may be demoted as well in the future.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remark: at least the 64-bit windows-gnu is more accessible as a host target, unlike the 32-bit windows-gnu.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if you want to cross-compile from linux to windows, the 64-bit windows-gnu (or gnullvm) target is the best way to do that, so I expect it to stay around a lot longer. we'd probably want to add an arm gnu target too at some point.

Copy link
Member

@workingjubilee workingjubilee Feb 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I expect that we would have an aarch64-pc-windows-gnullvm or whatever target instead (though that's functionally equivalent for practical use-cases).

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This discussion is drifting away from the culprit, but I don't see value in ARM targets (32-bit).

AArch64 is already there as a cross compilation target (gnullvm only until GNU catches up), and once LLVM 20 goes live, I'll try to start MCP for promoting 64-bit gnullvm targets to hosts.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AArch64 is already there as a cross compilation target (gnullvm only until GNU catches up), and once LLVM 20 goes live, I'll try to start MCP for promoting 64-bit gnullvm targets to hosts.

nice! I had meant aarch64, but iirc windows doesn't call it that for some reason...

@Noratrieb
Copy link
Member Author

Noratrieb commented Feb 15, 2025

I addressed some of the feedback I thought was worth putting into the RFC text itself.

The target tier policy says

A proposed new tier 1 target must be reviewed and approved by the compiler team based on these requirements. In addition, the release team must approve the viability and value of supporting the target. For a tier 1 target, this will typically take place via a full RFC proposing the target, to be jointly reviewed and approved by the compiler team and release team.

It does not say what teams should be involved in the demotion of a tier 1 target, but I leaned on the side of caution and chose to include compiler and release. I guess release can judge the non-viability and lack of value of supporting the target :D

I don't expect this to be controversial, so
@rfcbot fcp merge

@rfcbot
Copy link
Collaborator

rfcbot commented Feb 15, 2025

Team member @Noratrieb has proposed to merge this. The next step is review by the rest of the tagged team members:

No concerns currently listed.

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

@rfcbot rfcbot added proposed-final-comment-period Currently awaiting signoff of all team members in order to enter the final comment period. disposition-merge This RFC is in PFCP or FCP with a disposition to merge it. labels Feb 15, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
disposition-merge This RFC is in PFCP or FCP with a disposition to merge it. proposed-final-comment-period Currently awaiting signoff of all team members in order to enter the final comment period. T-compiler Relevant to the compiler team, which will review and decide on the RFC. T-release Relevant to the release team, which will review and decide on the RFC.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants