[PATCH] D127812: [AArch64] Function multiversioning support added.

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D127812#3587223 <https://reviews.llvm.org/D127812#3587223>, @ilinpv wrote:

> In D127812#3585249 <https://reviews.llvm.org/D127812#3585249>, @erichkeane wrote:
>
>> I'm concerned as to the design of this addition, I don't particularly appreciate the reasons for making 'target_clones' different, nor the purpose for adding a new attribute instead of using 'target' for what seems like exactly that?  IF the new spelling is THAT necessary, we perhaps don't need a whole new attribute for it either.
>
> Thank you for fair concern, "target_clones" for AArch64 has different format, semantic, e.g. "default" is not required.  Therefore it diverges with X86 in these parts.

Is it *necessary* that it diverges like this? (Is there some published standards document you're trying to conform to, is there an implementation difficulty with not diverging, something else?)

> "target" attribute has been already used and supported on AArch64 in a different sense, like target("arm"), target("dotprod"), target("branch-protection=bti"). The intention of creating new "target_version" attribute is not to overlap with that. It also has different format, mangling and semantic, e.g. treating function without attribute as "default", and option to disable attribute droping function multi versions. Do these explanations dispel your concern?

Do I understand correctly that AArch64 was using an attribute named `target` which does not behave like the attribute with the same name in GCC and Clang on other architectures, and now you'd like to introduce a new attribute which does behave like `target` but under a different name? If I have that correct, I don't think that's reasonable -- there's far too much possibility for confusion with that situation already, and adding a new attribute only increases the confusion. I'm not certain where the technical debt came from, but we shouldn't increase the burden on users here; I think `target` and `target_clones` should be made to work consistently across architectures if at all possible.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D127812/new/

https://reviews.llvm.org/D127812