Add target_feature_available_at_call_site intrinsic

[calebzulawski]

View all comments

This intrinsic allows writing intrinsic-like functions in plain Rust, that can be aware of which target features are available post-inlining.

r? @RalfJung
Feel free to pass this review on to anyone else, I just remember that we talked about this concept a while back and you might be interested.

Motivation

For a very simple example:

#[inline(always)]
fn fast_fma(a: f32, b: f32, c: f32) -> f32 {
    let has_fma = unsafe { core::intrinsics::target_feature_available_at_call_site("fma") };
    if has_fma {
        a.mul_add(b, c)
    } else {
        a * b + c
    }
}

For a more concrete example, see std::simd's swizzle_dyn. This function heavily demands target features, but:

• cfg!(target_feature = "ssse3") doesn't work without build-std (and you lose compatibility with old CPUs)
• is_x86_feature_detected is much too slow
• the default case without SSSE3 is terrible!

This function requires a low-overhead method of determining the target features available at the call site.

How it works

In rustc, we replace the intrinsic with true (if the feature is already known to be enabled), false (some specific cases, e.g. compiling without optimizations), or a special marker function (e.g. rust.target_feature_available_at_call_site.avx).

A post-inlining LLVM pass replaces instances of the marker with true or false based on the caller. Then it runs instruction and CFG simplification passes to remove any evidence of branches relying on the intrinsic. This is the same mechanism Julia uses for fma. For GCC and Cranelift, no post-inlining pass is done, so the intrinsic only returns based on the global features and function attribute in MIR.

[rustbot]

Some changes occurred to the intrinsics. Make sure the CTFE / Miri interpreter
gets adapted for the changes, if necessary.

cc @rust-lang/miri, @RalfJung, @oli-obk, @lcnr

[rustbot]

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

[rustbot]

rustc_codegen_gcc is developed in its own repository. If possible, consider making this change to rust-lang/rustc_codegen_gcc instead.

cc @antoyo, @GuillaumeGomez

[programmerjake]

looks good to me, though definitely get someone more familiar with the compiler to review it.

View changes since this review

[RalfJung]

I won't be able to review the implementation, but can give feedback on design.
However, we don't usually design in the implementation PR. Presumably this won't remain just an intrinsic, but the intrinsic will be exposed in a stable wrapper or so? That'd be a new language + libs-api feature, which seems RFC-worthy. Even if we don't need an RFC for a lang experiment, we need enough of a clear problem statement and rough design idea for what this will look like to our users that the lang team can sign off on this becoming a lang experiment. (I can be the liasion or champion or whatever it is called these days for that experiment.)

[RalfJung]

This can't be correct. Clearly, having the intrinsic randomly return true cannot go well.

I think what you mean is:

• If the intrinsic returns true, then the feature is definitely available, and it is sound to call other functions that need the feature.
• If the intrinsic returns false, that conveys no information whatsoever.

View changes since the review

[workingjubilee]

Yes, probably something like "this intrinsic can return false even if the call site does have the feature statically available because this depends on the vagaries of optimization passes and inlining".

[nikic]

Why not do this in OptimizerEarly instead, so more passes run afterwards?

View changes since the review

[calebzulawski]

That makes sense, I moved it

[nikic]

This should be a metadata string argument instead.

View changes since the review

[calebzulawski]

I didn't know you could do this! Much better

[nikic]

Doing this in a custom pass is okay for experimentation purposes, but this should really be proposed as an LLVM intrinsic upstream.

[workingjubilee]

I think currently Caleb's intent is to experiment with this for internal usage for std::simd APIs which would like to be able to select an implementation that is "feature aware" with respect to, effectively, where we expect to be inlined, so that it can resolve to the preferred instruction. There are a few functions where the fact that we can't enable features on the function is extremely brutal because we're already compiled into std.

The alternative is to tell people to recompile std with the features they have available which... well, I can't throw stones about slow progress but -Zbuild-std isn't already-solved for a reason.

[programmerjake]

Doing this in a custom pass is okay for experimentation purposes, but this should really be proposed as an LLVM intrinsic upstream.

yes, that has additional benefits such as still working correctly when no LLVM optimization passes are run and then it's compiled the rest of the way using clang or something to compile the LLVM IR.

[calebzulawski]

I won't be able to review the implementation, but can give feedback on design. However, we don't usually design in the implementation PR. Presumably this won't remain just an intrinsic, but the intrinsic will be exposed in a stable wrapper or so? That'd be a new language + libs-api feature, which seems RFC-worthy. Even if we don't need an RFC for a lang experiment, we need enough of a clear problem statement and rough design idea for what this will look like to our users that the lang team can sign off on this becoming a lang experiment. (I can be the liasion or champion or whatever it is called these days for that experiment.)

I would certainly eventually like a public interface which would certainly need an RFC/experiment, but more urgently I think the intrinsic is useful within std::simd. I'm happy to write an RFC or problem statement or whatever is necessary. Thanks :)

[calebzulawski]

Doing this in a custom pass is okay for experimentation purposes, but this should really be proposed as an LLVM intrinsic upstream.

Agreed that I would like to eventually move this to LLVM. I haven't personally had much luck getting simple bugfixes merged there

[RalfJung]

Doing this in a custom pass is okay for experimentation purposes, but this should really be proposed as an LLVM intrinsic upstream.

Agreed that I would like to eventually move this to LLVM. I haven't personally had much luck getting simple bugfixes merged there

@nikic is the head LLVM maintainer, I'm sure he can give you some guidance. :)

[RalfJung]

I would certainly eventually like a public interface which would certainly need an RFC/experiment, but more urgently I think the intrinsic is useful within std::simd. I'm happy to write an RFC or problem statement or whatever is necessary. Thanks :)

If it's just for internal use for now, then... maybe make it a t-compiler MCP where we also ping t-opsem? It's a new compiler feature with opsem considerations so that seems like a reasonable use of our process to me.

We should be sure to get some sort of t-lang vibes before any stable part of libcore/libstd relies on this, though. But for experimentation that shouldn't be needed.

[rustbot]

Some changes occurred to the platform-builtins intrinsics. Make sure the
LLVM backend as well as portable-simd gets adapted for the changes.

cc @antoyo, @GuillaumeGomez, @bjorn3, @programmerjake

[rustbot]

rustc_codegen_cranelift is developed in its own repository. If possible, consider making this change to rust-lang/rustc_codegen_cranelift instead.

cc @bjorn3

[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.

[bjorn3]

Per-function target features aren't supported by cg_clif yet. The TargetIsa is fixed for a Module. Would need Cranelift changes to implement.

View changes since the review