riscv: promote d, e, and f target_features to CfgStableToggleUnstable

[romancardenas]

Reference PR:

• Document RISC-V d, e, and f extensions reference#2274

This PR is a continuation of #155962

It uses the new CfgStableToggleUnstable stability level for the "d", "e", and "f" target_features of RISC-V. This way, it will be possible to add conditional code blocks depending on whether the target architecture has FPU, for instance.

The PR is related to #150257

r? @RalfJung

[rustbot]

RalfJung is not on the review rotation at the moment.
They may take a while to respond.

[RalfJung]

Looks good on the test side, thanks. :)
Cc @Amanieu

Let's nominate this for the @rust-lang/lang. This is about stabilizing the use of the "d", "e", and "f" RISC-V target feature for use in cfg only. IOW, this lets code check whether the features are available, but it doesn't let code enable or disable the features. That should therefore be entirely harmless ABI-wise.

[joshtriplett]

Blocked waiting on feedback from @Amanieu.

[Amanieu]

I don't see any harm in stabilizing those. The real issues comes from selectively enabling these on a per-function basis, which this doesn't do.

[traviscross]

Thanks @Amanieu.

@rfcbot fcp merge lang

[rust-rfcbot]

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

• @joshtriplett
• @nikomatsakis
• @scottmcm
• @tmandry
• @traviscross

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!

cc @rust-lang/lang-advisors: FCP proposed for lang, please feel free to register concerns.
See this document for info about what commands tagged team members can give me.

[ehuss]

I posted a PR at rust-lang/reference#2274 to update the documentation.

[tmandry]

@rfcbot reviewed

My understanding is that we are moving toward supporting these with #[target_feature], just trying to avoid potential implementation hazards.

If it turned out that this was not a coherent feature to expose to end users because of the ABI interaction, we might need to have a conversation about whether it makes sense to use the same abstractions for features that are only ever program level vs features that can be enabled at runtime (probably it does make sense, but it's worth having a conversation).

[rust-rfcbot]

🔔 This is now entering its final comment period, as per the review above. 🔔

[RalfJung]

My understanding is that we are moving toward supporting these with #[target_feature], just trying to avoid potential implementation hazards.

I don't think we are. I don't think we ever could. Maybe we can one day allow them with a target modifier version of -Ctarget-feature that forces the same features to be used across the entire crate graph, but that's the most we can soundly do.

[tmandry]

Ok, thanks for the clarification.

I don't like that there will always be differences between what is supported by #[cfg(target_feature)] and #[target_feature], because we don't have a common and unambiguous way to speak about the second, making the difference difficult to talk about.

The terminology "target_feature attribute" is clear though, and I would encourage people to use or "cfg target_feature" instead of the ambiguous "target_feature". In retrospect I would have preferred to call it #[detect_target_feature], #[runtime_target_feature], or something like that.

Not going to block this stabilization on a pre-existing naming issue though.

[RalfJung]

I don't like that there will always be differences between what is supported by #[cfg(target_feature)] and #[target_feature], because we don't have a common and unambiguous way to speak about the second, making the difference difficult to talk about.

We could decide to expose "d", "e", "f" via another flag, like cfg(target_modifier) or whatever.

I also need to slightly correct what I said earlier: for some RISC-V ABIs, we could allow #[target_feature = "d"]. It looks like on the ilp32f and lp64f ABIs, enabling "d" is harmless. On the ilp32 and lp64 ABIs, enabling "f" or "d" is harmless. On the ilp32e and lp64e ABIs, enabling "e" is harmless. On some other ABIs, "d"/"f" are required so enabling them is a NOP.

tl;dr it's complicated...

[rust-rfcbot]

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.

[RalfJung]

Awesome, let's ship this. :)

@bors r+ rollup

[rust-bors]

📌 Commit 4e9a3de has been approved by RalfJung

It is now in the queue for this repository.