Extract some shared code from codegen backend target feature handling

[RalfJung]

There's a bunch of code duplication between the GCC and LLVM backends in target feature handling. This moves that into new shared helper functions. I placed those in rustc_middle since I couldn't think of a better place to put them... I'm open for suggestions. :)

The first three commits should be purely refactoring. I am fairly sure the LLVM-side behavior stays the same; if the GCC side deliberately diverges from this then I may have missed that. I did account for one divergence, which I do not know is deliberate or not: GCC does not seem to use the -Ctarget-feature flag to populate cfg(target_feature). That seems odd, since the -Ctarget-feature flag is used to populate the return value of global_gcc_features which controls the target features actually used by GCC. @GuillaumeGomez @antoyo is there a reason target_config ignores -Ctarget-feature but global_gcc_features does not?

The last commit extracts some shared logic out of the functions that populate cfg(target_feature) and the backend target feature set, respectively. This one actually has some slight functional changes:

• Before, with -Ctarget-feature=-feat, if there is some other feature x that implies feat we would not add -x to the backend target feature set. Now, we do. This fixes Target feature implications for negative features are handled inconsistently between codegen and cfg(target_feature) #134792.
• The logic that removes x from cfg(target_feature) in this case also changed a bit, avoiding a large number of calls to the (uncached) sess.target.implied_target_features (if there were a large number of positive features listed before a negative feature) but instead constructing a full inverse implication map when encountering the first negative feature. Ideally this would be done with queries but the backend target feature logic runs before tcx so we can't use that...
• Previously, if feature "a" implied "b" and "b" was unstable, then using -Ctarget-feature=+a would also emit a warning about b. I had to remove this since when accounting for negative implications, this emits a ton of warnings in a bunch of existing tests... I assume this was unintentional anyway.

@bjorn3 I did not touch the cranelift backend here, since AFAIK it doesn't really support target features. But if you ever do, please use the new helpers. :)

Cc @workingjubilee

[rustbot]

r? @compiler-errors

rustbot has assigned @compiler-errors.
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

[rustbot]

Some changes occurred in compiler/rustc_codegen_ssa

cc @WaffleLapkin

Some changes occurred in compiler/rustc_codegen_ssa/src/codegen_attrs.rs

cc @jdonszelmann

Some changes occurred in compiler/rustc_codegen_gcc

cc @antoyo, @GuillaumeGomez

[RalfJung]

There's a lot more logic in this file that seems like it should be shared across backends (almost all of fn codegen_fn_attrs), but I shied away from the huge refactor that would have been.

[RalfJung]

Turns out properly adding negative implications to the target_features we set for LLVM tends to add a lot of target features -- we have some quite extensive implication chains. Is that a problem? Cc @nikic @taiki-e

[antoyo]

if the GCC side deliberately diverges from this then I may have missed that. I did account for one divergence, which I do not know is deliberate or not: GCC does not seem to use the -Ctarget-feature flag to populate cfg(target_feature). That seems odd, since the -Ctarget-feature flag is used to populate the return value of global_gcc_features which controls the target features actually used by GCC. @GuillaumeGomez @antoyo is there a reason target_config ignores -Ctarget-feature but global_gcc_features does not?

There is no reason for that. I probably missed some stuff when I implemented this.
I'd be happy to have the same behavior as LLVM here.

[nikic]

Turns out properly adding negative implications to the target_features we set for LLVM tends to add a lot of target features -- we have some quite extensive implication chains. Is that a problem? Cc @nikic @taiki-e

Anything in particular you're concerned about? But generally, no, it shouldn't be a problem.

[RalfJung]

I can imagine two concerns:

• So many attributes cause some sort of perf issue?
• We actually have more implications in rustc than LLVM recognizes, so now we're disabling more things than before. This is probably a bugfix but still it'd be good to know if there is such a case...