riscv: stabilize 'd' and 'f' target features

[RalfJung]

#156188 has been reverted in #161064 due to concerns about "e" being a negative target feature.

"d" and "f" however are almost normal target features, so let's re-land them.
The one thing that's odd about them is that we cannot allow you to use -Ctarget-feature or #[target_feature] for them on some riscv ABIs. (For now, it remains unstable on all riscv ABIs.) So this means we need the concept of target-features that are fixed by the target spec and cannot be changed by the user. OTOH we kind of already have that, even for fully stable target features:

• On x86-64, we don't let people disable "sse2". On aarch64, we don't let people disable "neon".
• Soon, on x86-64 softfloat, we don't let people enable "sse2".

"neon" and "sse2" are even stable target features. With this PR, "d" and "f" remain unstable to toggle; if we ever stabilize them then the rules would be

• on riscv targets that use a "d" ABI (ilp32d, lp64d), you can't disable either of them
• on riscv targets that use an "f" ABI (ilp32f, lp64f), you can't disable "f"
• on riscv ilp32e targets, you can't enable "d"

@rust-lang/lang you already FPC'd this in #156188; please let me know if you want another FCP or if we can just re-land this. Also see below for a proposal to just fully stabilize these target features.

Cc @romancardenas

[rustbot]

r? @mejrs

rustbot has assigned @mejrs.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

Why was this reviewer chosen?

The reviewer was selected based on:

• Owners of files modified in this PR: compiler
• compiler expanded to 75 candidates
• Random selection from 17 candidates

[beetrees]

I think it would be worth including the cfg-stable-toggle-unstable-* tests from the original PR (#156188).

[beetrees]

EDIT: Never mind, just remembered ILP32E.

[RalfJung]

Ah, right. I should probably cherry-pick the original commit and then remove the "e" part.

[RalfJung]

@workingjubilee pointed out that we already have some fully stable target features that can't be toggled:

• On x86-64, we don't let people disable "sse2". On aarch64, we don't let people disable "neon".
• Soon, on x86-64 softfloat, we don't let people enable "sse2".

So... should we just fully stabilize "d" and "f", i.e. also for use in -Ctarget-feature and #[target_feature]? The rules as currently implemented would be

• on riscv targets that use a "d" ABI (ilp32d, lp64d), neither "f" nor "d" can be disabled
• on riscv targets that use an "f" ABI (ilp32f, lp64f), you can't disable "f"
• on riscv ilp32e targets, you can't enable "d"

Cc @Amanieu @ZhongyaoChen @CaiWeiran @kito-cheng @michaelmaitland @robin-randhawa-sifive @topperc @sanchuanhehe @almindor @rmsyn

[almindor]

LGTM

View changes since this review

[sanchuanhehe]

LGTM

View changes since this review

[workingjubilee]

Making "d" and "f" fully stable feels like a big "shrug?" to me. I can't think of an enormously strong motivation or, er, demotivation, given that we have to apply restrictions anyways. But unlike with "e", where I'm uncertain about the entire idea of the feature per se, it seems acceptable if unexciting to allow it to join the peculiar ranks of "sse2" and "neon".

Though I also didn't remember that ilp32e works with F and not just that specialist set of target features for, I think, floating point in integer registers, that are mutually-exclusive with F. I guess either way is plausible, at least.

[RalfJung]

I mostly just don't see a reason not to do it. I am reasonably confident in our story for ensuring ABI correctness, and I don't see how we'd become more confident in it by waiting another year or two. We already rely on that story for x86-64 and aarch64 as mentioned above, though only in corner cases that few people are ever going to venture into. At some point we just have to ship it and see if it survives contact with the real world...

[mejrs]

Looks good to me, r=me after lang decision

View changes since this review

[tmandry]

We talked about this in the lang meeting today and are comfortable doing the full stabilization, as described in #161385 (comment).

@rfcbot fcp merge lang

[rust-rfcbot]

@tmandry 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.

[traviscross]

Thanks @RalfJung.

@rfcbot reviewed

[rust-rfcbot]

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

[RalfJung]

We talked about this in the lang meeting today and are comfortable doing the full stabilization, as described in

Nice. :) Done.
@mejrs this invalidates your review unfortunately. Please have another look :)

[rustbot]

This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.