add optional soft-float feature for s390x target

[fneddy]

followup on #150766

to use the soft-float feature one need to also set the s390x-softfloat flag in the .rustc_abi target field. This will automatically enable the LLVM +soft-float feature flag. On normal compilation with the default ABI, soft-float is still marked incompatible! So the ABI cannot be be broken by accident.

open soft-float points from previous PR:

• should there be a s390x-unknown-none-softfloat target?

r? @cuviper

[tgross35]

Cc @RalfJung who is the expert in this bit of code.

to use the soft-float feature one need to also set the s390x-softfloat flag in the .rustc_abi target field. This will automatically enable the LLVM +soft-float feature flag. On normal compilation with the default ABI, soft-float is still marked incompatible! So the ABI cannot be be broken by accident.

There aren't any tests indicating how this is meant to be used; do you mean that attempting to use +soft-float is rejected via target properties? I.e., what happens when a binary built with +soft-float gets linked against the normal s390x-unknown-none libcore?

The ABI change can be covered with a simple test in tests/assembly-llvm/, using //@ add-minicore so it can build on all platforms. The test indicating that we reject mixing crates built with and without +soft-float can probably live in tests/ui/target_modifiers (one crate will live in auxiliary/, the other lives in that directory and gets the ABI-changing flags).

[workingjubilee]

The way the code works allows us to take a simpler route for this case:

• Add a target that is s390x-unknown-none-softfloat (assuming that name is amenable to the relevant maints who are effectively going to be asked to maintain this target) cc @cuviper @uweigand again
• Add a warning for trying to set the +soft-float or -soft-float feature at all through the CLI by adding it as a feature but with Stability::Forbidden:

  rust/compiler/rustc_target/src/target_features.rs

  Lines 27 to 31 in 503745e

  	/// This feature can not be set via `-Ctarget-feature` or `#[target_feature]`, it can only be
  	/// set in the target spec. It is never set in `cfg(target_feature)`. Used in
  	/// particular for features are actually ABI configuration flags (not all targets are as nice as
  	/// RISC-V and have an explicit way to set the ABI separate from target features).
  	Forbidden { reason: &'static str },

• Have the s390x-unknown-none-softfloat target pass "+soft-float" in the list of features enabled as part of its target definition, because the relevant field in the target spec is speaking directly to what LLVM features are enabled, last I checked.

Then we do not need to have "agreement" on the soft-float flag between target-features in FeatureConstraints, because it becomes "use the target or bust".

[workingjubilee]

The above description of how it is easier to solve this by avoiding the question of target-features being set at all is why we have nudged towards "a new target":

@fneddy @uweigand To bring you up to speed on a detail that has become important to clarify, and I think may have been missed: Rust target features are not LLVM target features. This is not just because Cranelift and GCC are backends in the process of being implemented. It is mandated by the language itself.

Long ago, in Rust RFC 2045, Rust decided to allow #[target_feature(enable = "...")], a syntax which adds a target feature to a function. It also decided to allow cfg(target_feature = "..."), which works like you would expect. "..." can be any old string you like. In those glorious old times, this would go straight-through to LLVM.

However, because these attributes live in Rust source code, they are subject to rustc's stability policies around the language, or at least there is an expectation that they are. People are not amused when you tell them that their source code doesn't build because LLVM changed how they support that feature! Unfortunately, the list of LLVM features changes surprisingly often. This story is well-told by the mapping of Rust features to LLVM features in rustc_codegen_llvm.

And that is on top of the realization that many target features are nonsensical to set on a per-function basis, despite being implemented as settable in that way by LLVM!

Since that realization, the compiler and language teams have more-or-less been trying to walk that back as much as possible. I'm approximating. This is the other big reason for my and Ralf's suggestion of "new target" as the way to "configure" this, instead of requiring agreement: There's no reason to reflect the way C compilers configure something like this when we already have to go so far in a different direction.

The main reason to introduce target modifiers for a target is when you want to support many subtle variations, especially along more than one axis. The many-splendored world of ABI-incompatible RISCV variants, for example. Here, we only have heard calls for two de facto targets.

[RalfJung]

Can you just rename X86Softfloat to Softfloat and use that also for s390x?

[RalfJung]

Add a warning for trying to set the +soft-float or -soft-float feature at all through the CLI by adding it as a feature but with Stability::Forbidden:

Note that the target feature already exists, it just needs the message to be changed.

Also I think we can't make it Forbidden when it is used like this for the ABI. (Maybe we should fix that.) But that's fine, the ABI check ensures that it cannot be toggled. However, without a builtin target we won't know if this actually works... so it'd probably be best to add such a target in the same PR.

[RalfJung]

Also I think we can't make it Forbidden when it is used like this for the ABI. (Maybe we should fix that.)

Actually, it seems I misremembered. It can remain Frobidden. The only practical difference between Forbidden and (Un)Stable here is whether the feature can be queried with cfg(target_feature).