[AArch64][TargetParser] autogen ArchExtKind enum - renaming

[tmatheson-arm]

Rename of several SubtargetFeatures or ArmExtKind (AEK_*) enum values to bring them inline with the user-facing spelling on the command line.

[sdkrystian]

@tmatheson-arm This breaks builds without the AArch64 target enabled (e.g. LLVM_TARGETS_TO_BUILD=X86 ) because llvm/TargetParser/AArch64TargetParserDef.inc is not generated for such configurations.

[jthackray]

LGTM (presumably these were mechanically renamed, given the diff size).

[davemgreen]

IMO This patch looks far too large to sensibly review and needs to be split up. A lot of the changes don't really looks like mechanical renamings, and it is hard to see how they would not break existing uses of llvm arch64 target features?

[github-actions]

✅ With the latest revision this PR passed the C/C++ code formatter.

[ostannard]

@davemgreen This is already split into 18 commits, I don't think there's any reason to split it into 18 PRs, since comments on one of them likely apply to the others.

@tmatheson-arm I don't know if there's any more specific precedent for changing target feature names, but I think this will be a breaking change for existing bitcode files, which we do try to maintain backward compatibility with: https://llvm.org/docs/DeveloperPolicy.html#ir-backwards-compatibility. Maybe we could add IR auto-upgrade code to handle old bitcode?

[davemgreen]

This is already split into 18 commits, I don't think there's any reason to split it into 18 PRs, since comments on one of them likely apply to the others.

I disagree. This is going to be awkward for a lot of users of llvm and contains at least some details I don't agree with. I think it will can cause a lot of subtle bugs and can end up wasting a lot of peoples time. It should at least be awkward for us too.

[davemgreen]

@tmatheson-arm reached out and we have a bit of a conversation internally. I do think that there is too much going on in this one pr to be sensible to review, but from what I've looked at my main points I think are:

• Some AEK names get renamed in ways I would not expect them to, like AEK_FP16 being changed to AEK_FULLFP16. The command-line names or FEAT_ names should probably be what we are aiming for if we are changing them one-way or the other.
• Some of the backend features have been renamed in ways that could cause breakages for external users, like +complxnum becoming +fcma. The new name is better, but ideally the old name would continue to work (maybe by adding an alias/extra target feature dependant on the new name? That might not work with negative features).
• Some of changes do look mechanical and a good change (P1->p1, V2->v2 etc), and if they were separate they would be easy to get in and out of the way of the contentious stuff that remains.
• The ones that have changed should have release notes.

[jthackray]

The command-line names or FEAT_ names should probably be what we are aiming for if we are changing them one-way or the other.

Yes, standardising on FEAT_* names would be good to match the TRM, so we avoid the
AEK_PREDRES/FEAT_SPECRES, AEK_PERFMON/FEAT_PMUv3, etc. mismatches.

[davemgreen]

Rust (https://github.com/rust-lang/rust/blob/79734f1db8dbe322192dea32c0f6b80ab14c4c1d/compiler/rustc_codegen_llvm/src/llvm_util.rs#L229) and zig (https://github.com/ziglang/zig/blob/44db92d1ca90c9cfdfb29fe46f04ff8f11c80901/lib/std/Target/aarch64.zig#L43) are two examples of what I meant by external dependencies. They can adapt, especially if there are release notes, but there may be many more projects out there using the old names and it would be good if they kept working, if we can.

[tmatheson-arm]

Thanks for the comments everyone. Given that this requires an IR break and additions to the importer, and there is still some question about which way to go with the renaming (i.e. FEAT_ names or user-facing names) I think it makes sense to defer renaming until later. For now, I will add mechanisms to handle the various inconsistencies as in #90987.