Repository navigation
tier 3 target policy: target CPU and features must be supported by upstream backend - #162840
Conversation
|
r? @mati865 rustbot has assigned @mati865. Use Why was this reviewer chosen?The reviewer was selected based on:
|
|
I think this will require FCP. I hope it's okay for me to spawn the checkboxes. @rfcbot merge compiler |
|
@RalfJung 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. |
e8f69c2 to
342d9ca
Compare
| rustc's supported backends from any host target. The default target CPU and | ||
| target features must be supported by that backend. Having support in a fork | ||
| of the backend is not sufficient, it must be upstream in the version of the | ||
| backend that ships with rustc. |
There was a problem hiding this comment.
I also clarified that when we say "upstream", we mean the version we ship, not the latest dev branch.
|
Cc @rust-lang/wg-llvm |
I definitely see how it may be useful to add a non-upstreamed LLVM target to rustc. But given that the no-fork requirement is already in the policy, the additions from this PR do not change things significantly. |
|
I can see how it is useful for companies that have proprietary backends but I don't think we should spend our time and resources catering to those companies. If we can't even easily run smoke tests (such as |
342d9ca to
aad80a2
Compare
|
@TaKO8Ki @estebank @jdonszelmann @spastorino @wesleywiser please have a look at this, there's a checkbox waiting for you. :) The change is tiny and hopefully uncontroversial. |
|
🔔 This is now entering its final comment period, as per the review above. 🔔 |
|
The final comment period, with a disposition to merge, as per the review above, is now complete. As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed. |
|
@bors r=mati865 rollup |
tier 3 target policy: target CPU and features must be supported by upstream backend I am proposing we clarify this point in our tier 3 target policy: it's not enough to just generate some assembly; whatever values are set for target CPU and target features must actually be recognized by the backend. In particular, if a target emits a warning like this when used without any `-C` flag, it does not satisfy the policy: ``` 'esp32s3' is not a recognized processor for this target (ignoring processor) ``` The context here is rust-lang#162828: we landed the xtensa targets two years ago when they had basically no upstream LLVM support, and even now we have one target that uses a CPU that upstream LLVM does not recognize. I think we should consider this target as not satisfying our policy.
…uwer Rollup of 8 pull requests Successful merges: - #162460 (Feat - `#[inline(always)] + #[target_feature(enable = "....")]` #2) - #163485 (`rustc_builtin_macros` cleanup, part 8) - #164578 (Don't ignore unknown LLVM diagnostics) - #162369 (Document the `recursion_limit` built-in attribute) - #162840 (tier 3 target policy: target CPU and features must be supported by upstream backend) - #164330 (Skip redundant monomorphization checks) - #164656 (stabilize ExitCodeExt for Windows) - #164660 (Touch up attribute docs)
tier 3 target policy: target CPU and features must be supported by upstream backend I am proposing we clarify this point in our tier 3 target policy: it's not enough to just generate some assembly; whatever values are set for target CPU and target features must actually be recognized by the backend. In particular, if a target emits a warning like this when used without any `-C` flag, it does not satisfy the policy: ``` 'esp32s3' is not a recognized processor for this target (ignoring processor) ``` The context here is rust-lang#162828: we landed the xtensa targets two years ago when they had basically no upstream LLVM support, and even now we have one target that uses a CPU that upstream LLVM does not recognize. I think we should consider this target as not satisfying our policy.
…uwer Rollup of 13 pull requests Successful merges: - #162460 (Feat - `#[inline(always)] + #[target_feature(enable = "....")]` #2) - #163485 (`rustc_builtin_macros` cleanup, part 8) - #164457 (Add `vec_visualizer` lldb test) - #161253 (Implement `AsMut<T>` and `AsRef<T>` for `!`.) - #162369 (Document the `recursion_limit` built-in attribute) - #162840 (tier 3 target policy: target CPU and features must be supported by upstream backend) - #164329 (use default_field_values in declare_lint) - #164330 (Skip redundant monomorphization checks) - #164527 (doc: added addtional docs for Write) - #164656 (stabilize ExitCodeExt for Windows) - #164660 (Touch up attribute docs) - #164681 (tidy: use repository-relative path for style checks) - #164688 (re-enable test_dir_remove_dir test on QNX)
Rollup merge of #162840 - RalfJung:tier-3-target-cpu, r=mati865 tier 3 target policy: target CPU and features must be supported by upstream backend I am proposing we clarify this point in our tier 3 target policy: it's not enough to just generate some assembly; whatever values are set for target CPU and target features must actually be recognized by the backend. In particular, if a target emits a warning like this when used without any `-C` flag, it does not satisfy the policy: ``` 'esp32s3' is not a recognized processor for this target (ignoring processor) ``` The context here is #162828: we landed the xtensa targets two years ago when they had basically no upstream LLVM support, and even now we have one target that uses a CPU that upstream LLVM does not recognize. I think we should consider this target as not satisfying our policy.
I am proposing we clarify this point in our tier 3 target policy: it's not enough to just generate some assembly; whatever values are set for target CPU and target features must actually be recognized by the backend.
In particular, if a target emits a warning like this when used without any
-Cflag, it does not satisfy the policy:The context here is #162828: we landed the xtensa targets two years ago when they had basically no upstream LLVM support, and even now we have one target that uses a CPU that upstream LLVM does not recognize. I think we should consider this target as not satisfying our policy.