Rollup merge of #134337 - RalfJung:riscv-target-features, r=workingjubilee

reject unsound toggling of RISCV target features

~~Stacked on top of #133417, only the last commit is new.~~

Works towards #132618 (but more [remains to be done](#134337 (comment)))
Part of #116344

Cc ``@beetrees`` I hope I got everything.  I didn't do anything about "The f and zfinx features are incompatible" and that's not an ABI thing (right?) and I am not sure how to handle it with these ABI checks.
r? ``@workingjubilee``

Ideally we'd also reject target specs that disable the `f` feature but set an ABI that requires `f`... but I don't want to duplicate this logic. I have some ideas for how maybe the entire float ABI check logic should be different, now that we have some examples of what these ABI checks look like, but that will be a future PR.

Author: matthiaskrgr

compiler/rustc_target/src/spec/mod.rs

@@ -3166,7 +3166,7 @@ impl Target {
                 // Note that the `lp64e` is still unstable as it's not (yet) part of the ELF psABI.
                 check_matches!(
                     &*self.llvm_abiname,
-                    "lp64" | "lp64f" | "lp64d" | "lp64q" | "lp64e",
+                    "lp64" | "lp64f" | "lp64d" | "lp64e",
                     "invalid RISC-V ABI name"
                 );
             }

compiler/rustc_target/src/target_features.rs

@@ -39,7 +39,7 @@ pub enum Stability<Toggleability> {
         allow_toggle: Toggleability,
     },
     /// This feature can not be set via `-Ctarget-feature` or `#[target_feature]`, it can only be
-    /// set in the basic target definition. It is never set in `cfg(target_feature)`. Used in
+    /// set in the target spec. It is never set in `cfg(target_feature)`. Used in
     /// particular for features that change the floating-point ABI.
     Forbidden { reason: &'static str },
 }
@@ -590,9 +590,76 @@ const RISCV_FEATURES: &[(&str, StabilityUncomputed, ImpliedFeatures)] = &[
     // tidy-alphabetical-start
     ("a", STABLE, &["zaamo", "zalrsc"]),
     ("c", STABLE, &[]),
-    ("d", unstable(sym::riscv_target_feature), &["f"]),
-    ("e", unstable(sym::riscv_target_feature), &[]),
-    ("f", unstable(sym::riscv_target_feature), &[]),
+    (
+        "d",
+        Stability::Unstable {
+            nightly_feature: sym::riscv_target_feature,
+            allow_toggle: |target, enable| match &*target.llvm_abiname {
+                "ilp32d" | "lp64d" if !enable => {
+                    // The ABI requires the `d` feature, so it cannot be disabled.
+                    Err("feature is required by ABI")
+                }
+                "ilp32e" if enable => {
+                    // ilp32e is incompatible with features that need aligned load/stores > 32 bits,
+                    // like `d`.
+                    Err("feature is incompatible with ABI")
+                }
+                _ => Ok(()),
+            },
+        },
+        &["f"],
+    ),
+    (
+        "e",
+        Stability::Unstable {
+            // Given that this is a negative feature, consider this before stabilizing:
+            // does it really make sense to enable this feature in an individual
+            // function with `#[target_feature]`?
+            nightly_feature: sym::riscv_target_feature,
+            allow_toggle: |target, enable| {
+                match &*target.llvm_abiname {
+                    _ if !enable => {
+                        // Disabling this feature means we can use more registers (x16-x31).
+                        // The "e" ABIs treat them as caller-save, so it is safe to use them only
+                        // in some parts of a program while the rest doesn't know they even exist.
+                        // On other ABIs, the feature is already disabled anyway.
+                        Ok(())
+                    }
+                    "ilp32e" | "lp64e" => {
+                        // Embedded ABIs should already have the feature anyway, it's fine to enable
+                        // it again from an ABI perspective.
+                        Ok(())
+                    }
+                    _ => {
+                        // *Not* an embedded ABI. Enabling `e` is invalid.
+                        Err("feature is incompatible with ABI")
+                    }
+                }
+            },
+        },
+        &[],
+    ),
+    (
+        "f",
+        Stability::Unstable {
+            nightly_feature: sym::riscv_target_feature,
+            allow_toggle: |target, enable| {
+                match &*target.llvm_abiname {
+                    "ilp32f" | "ilp32d" | "lp64f" | "lp64d" if !enable => {
+                        // The ABI requires the `f` feature, so it cannot be disabled.
+                        Err("feature is required by ABI")
+                    }
+                    _ => Ok(()),
+                }
+            },
+        },
+        &[],
+    ),
+    (
+        "forced-atomics",
+        Stability::Forbidden { reason: "unsound because it changes the ABI of atomic operations" },
+        &[],
+    ),
     ("m", STABLE, &[]),
     ("relax", unstable(sym::riscv_target_feature), &[]),
     ("unaligned-scalar-mem", unstable(sym::riscv_target_feature), &[]),