Rename target.abi to target.cfg_abi and enum-ify llvm_abiname

[RalfJung]

Based on top of #153769. Also see Zulip for more context. Discussed a bit in #153769 (comment) too.

This renames target.abi to target.cfg_abi to make it less likely that someone will use it to determine things about the actual ccABI, i.e. the calling convention used on the target. target.abi does not control that calling convention, it just sometimes informs the user about that calling convention (and also about other aspects of the ABI).

Also turn llvm_abiname into an enum to make it more natural to match on.
Cc @workingjubilee @madsmtm

[rustbot]

Some changes occurred in compiler/rustc_codegen_cranelift

cc @bjorn3

Some changes occurred in cfg and check-cfg configuration

cc @Urgau

[rustbot]

r? @jieyouxu

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

Why was this reviewer chosen?

The reviewer was selected based on:

• Owners of files modified in this PR: compiler
• compiler expanded to 69 candidates
• Random selection from 15 candidates

[RalfJung]

We force all riscv targets to set an ABI so no check for an empty ABI name is needed here.

[RalfJung]

ElfV1 and ElfV2 are the only allowed ABI names here so we can error on everything else.

[RalfJung]

Also, to quote some of my arguments from #153769 where this change was first proposed (this was in reply to questions/comments by @madsmtm):

I kinda like that the fields on TargetOptions are named os, env and abi, and that they match target_os, target_env and target_abi.

I agree that it is a bummer to lose this consistency, but I think it's the preferable outcome over people misunderstanding what target.abi is or does.

But then again, I do also see your point that target.abi could be the wrong tool for the job in some cases. Do you have an example of a place where it's gone wrong in the past?

Yeah we just recently fixed a few of them

• rustc_target: callconv: powerpc64: Use llvm_abiname rather than target_abi for ABI determination #153035
• prefer actual ABI-controling fields over target.abi when making ABI decisions #152941
• and arguably the vector extension situation on AIX is also an example of this

In both cases AFAIK all targets had a consistent value for target.abi and the actual ccABI, but given how target.abi also often has other values not indicative of the ccABI, I think it is a massive footgun for it to have the name that "obviously" sounds like it controls the ccABI, when really it is just loosely correlated. It's arguably gotten worse when the type of abi was made an enum as that makes it look more canonical than it really is. That's what motivated me to also enum-ify LlvmAbi.

Looking at the current usage, it seems like only has_reliable_f16_f128 and the comparisons with target.abi == Abi::Spe that may be suspect? The rest seem like they use target.abi for checking certain target capabilities, which seems like the right thing to use it for?

Yeah, agreed. has_reliable_f16_f128 is a hopefully temporary internal hack/heuristic so it's not very critical. I don't know anything about the SPE situation (hence the FIXME).

Another solution would be to rename enum Abi to enum CfgAbi, we should at least discuss which would be preferable?

You mean instead of or on top of the field rename? I'd be fine with doing both, but I do think we should do the field rename. target.abi is just too juicy of a name.

I guess part of the problem here is that target_abi is kinda shitty... And is being used for a bunch of stuff it shouldn't be (looking at you Abi::Sim and Abi::Uwp...).

Exactly.

Maybe we should instead focus on actually making target.abi able to represent this stuff well enough that rustc could use it for determining ABI details? I.e. somehow combine llvm_abiname, llvm_floatabi, rustc_abi and cfg_abi into one field. Is that at all feasible?

We discussed this a bit on Zulip but it's not clear what one could do. I think even if we do that, a PR like this one is a helpful first step to map the territory. Personally I have no concrete plans to further cleanup after this PR (mostly due to a lack of good ideas, and also because I don't want to touch the hot potato of actually changing the user-visible target_abi values which is a non-trivial transition).