Fix unnecessary error when using neon target feature

[adamgemmell]

Fixes #95002

We previously made the decision #91608 to tie neon and fp, breaking code that just use #[target_feature(enable = "neon")]. However a deeper search revealed that this pattern was much more common than our first search suggested, including in all the aarch64/ARM neon intrinsics. Hence the proper fix should probably be to not error in this case.

r? @Amanieu

[workingjubilee]

As I was saying, for all practical intents and purposes, Rust programmers do not perceive "+fp" and "+neon" to be separate entities, which is why I recommended removing "+fp" as a distinct feature entirely, because it is forward compatible to re-add it if we encounter user need for it.

[pnkfelix]

Doing &mut collection.iter().stuff().collect() seems like an anti-pattern to me; you're passing in a collection to be mutated, but immediately throwing it away? Its certainly not clear on the callee side that that is what you'll be doing.

I skimmed the source; it seems like all (two?) callsites are creating the hashmap and throwing it away after the call to llvm_util::check_tied_features.

If I'm correct and all call sites are indeed freshly creating the hashmap and immediately throwing it away, then I would either change check_tied_features to take the hashmap by value, or I'd revise the code for check_tied_features so that it doesn't need to modify the hashmap at all.

[Amanieu]

How exactly does this differ from the behavior prior to #90621? The original behavior was that neon implied fp, are we just going back to that behavior?

Also, why was this not caught by CI? One potential issue is that check_tied_features is being called very late in the compilation pipeline, in the codegen stage. Ideally this check should occur much sooner when the target_feature attributes are being validated. This would have helped us catch the issues in stdarch since stdarch itself would have failed to compile.

[workingjubilee]

How exactly does this differ from the behavior prior to #90621? The original behavior was that neon implied fp, are we just going back to that behavior?

Yes, essentially. The catch is that we check the target feature restrictions on our end, before LLVM does its own feature management. So in order to be able to enforce both +neon and +fp as an additional check, we have to also add in additional feature management on our end, before we delegate to the codegen backend. By simply setting +neon, we allow any implicit subsets (as understood by the backend) to be included, but that means rustc doesn't know about the subset relationship.

This is not required in the case of the #91608 solution, which also should resolve #95002. This is why I proposed #91608: it is the solution that requires the least intervention on our part in order to make it work.