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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
