[PATCH] D158666: [Clang] Fix linker error for function multiversioning

2023-08-24 Thread Elizabeth Andrews via Phabricator via cfe-commits
eandrews added a comment. aarch64 has different mangling for default functions when using target_clones. Sigh `int __attribute__((target_clones("ls64_v+fp16", "default"))) foo_ovl(int) { return 1; }` results in `@_Z7foo_ovli._Mfp16Mls64_v` and `@_Z7foo_ovlv`. Note there is no ".default" as in

[PATCH] D158666: [Clang] Fix linker error for function multiversioning

2023-08-24 Thread Tom Honermann via Phabricator via cfe-commits
tahonermann added a comment. I think it would also be a good idea to exercise the test case for a target that lacks ifunc support to ensure that inter-TU calls work as expected. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D158666/new/ https://reviews.llvm.org/D158666

[PATCH] D158666: [Clang] Fix linker error for function multiversioning

2023-08-24 Thread Tom Honermann via Phabricator via cfe-commits
tahonermann added a comment. A couple of observations with regard to compatibility: gcc,, at least by default, emits the TC implementations as local functions, the resolver as a weak global function, and the undecorated name as an ifunc. When only a TC declaration (not a definition) is present,

[PATCH] D158666: [Clang] Fix linker error for function multiversioning

2023-08-23 Thread Elizabeth Andrews via Phabricator via cfe-commits
eandrews added a comment. In D158666#4611497 , @erichkeane wrote: > In D158666#4611494 , @eandrews > wrote: > >> In D158666#4611481 , @erichkeane >> wrote: >> >>> I thi

[PATCH] D158666: [Clang] Fix linker error for function multiversioning

2023-08-23 Thread Erich Keane via Phabricator via cfe-commits
erichkeane added a comment. In D158666#4611494 , @eandrews wrote: > In D158666#4611481 , @erichkeane > wrote: > >> I think the .ifunc spelling was an oversight on my part when I implemented >> this, I didn't spe

[PATCH] D158666: [Clang] Fix linker error for function multiversioning

2023-08-23 Thread Elizabeth Andrews via Phabricator via cfe-commits
eandrews added a comment. In D158666#4611481 , @erichkeane wrote: > I think the .ifunc spelling was an oversight on my part when I implemented > this, I didn't spend enough time investigating GCC's behavior when > implementing this feature. I think th

[PATCH] D158666: [Clang] Fix linker error for function multiversioning

2023-08-23 Thread Erich Keane via Phabricator via cfe-commits
erichkeane requested changes to this revision. erichkeane added a comment. This revision now requires changes to proceed. Accepted before I thought about the other order, so requesting changes so we can think this through... 1 thing we DO need to consider is how this works with OLDER versions of

[PATCH] D158666: [Clang] Fix linker error for function multiversioning

2023-08-23 Thread Erich Keane via Phabricator via cfe-commits
erichkeane accepted this revision. erichkeane added a comment. This revision is now accepted and ready to land. I think the .ifunc spelling was an oversight on my part when I implemented this, I didn't spend enough time investigating GCC's behavior when implementing this feature. I think the al

[PATCH] D158666: [Clang] Fix linker error for function multiversioning

2023-08-23 Thread Elizabeth Andrews via Phabricator via cfe-commits
eandrews created this revision. eandrews added reviewers: erichkeane, tahonermann, aaron.ballman, zsrkmyn. Herald added a project: All. eandrews requested review of this revision. Currently target_clones attribute results in a linker error when there are no multi-versioned function declarations i