tmsriram added a comment. In D73307#1989924 <https://reviews.llvm.org/D73307#1989924>, @erichkeane wrote:
> In D73307#1989740 <https://reviews.llvm.org/D73307#1989740>, @rnk wrote: > > > In D73307#1978140 <https://reviews.llvm.org/D73307#1978140>, @tmsriram > > wrote: > > > > > In D73307#1972388 <https://reviews.llvm.org/D73307#1972388>, @rnk wrote: > > > > > > > Regarding the alias attribute, it occurs to me that reimplementing this > > > > as an early LLVM pass would not have that problem. Do you think that > > > > would be worth doing? > > > > > > > > > I can try doing this. If my understanding is right, you are suggesting > > > that doing this later would mean the alias would have already been > > > applied?. However, for multi-versioning symbols, I must parse the target > > > name to insert the module name in between right? > > > > > > > >> Yes, Clang will apply the alias, do all name mangling, etc, and then >> renaming will happen later. >> >> It's not clear to me if the order of the `.<target>` suffix on >> multi-versioned symbols matters. @erichkeane, should it? Supposing some >> mid-level optimization came along and did function versioning, it would >> rename the internal function and append `.1`. So, I think appending as you >> implemented is fine. >> >> --- >> >> All of my concerns have been addressed, and I think everyone else's are as >> well. We have documentation indicating what we mean by uniqueness which >> @hubert.reinterpretcast wanted clarified, we had the prefix map issue which >> I believe is addressed, so to my knowledge this is ready. >> >> Let's wait a bit to hear from Erich, but otherwise I think this is ready in >> a few days. > > Multiversioning doesn't care about the names, simply that they are > unique/unique from eachother. The suffix that GCC/ICC put together for these > two types is simply to make sure we don't get something that conflicts with > another symbol. As long as these symbols are unique (and correclty updated!) > , they can be named absolutely anything. In D73307#1989924 <https://reviews.llvm.org/D73307#1989924>, @erichkeane wrote: > In D73307#1989740 <https://reviews.llvm.org/D73307#1989740>, @rnk wrote: > > > In D73307#1978140 <https://reviews.llvm.org/D73307#1978140>, @tmsriram > > wrote: > > > > > In D73307#1972388 <https://reviews.llvm.org/D73307#1972388>, @rnk wrote: > > > > > > > Regarding the alias attribute, it occurs to me that reimplementing this > > > > as an early LLVM pass would not have that problem. Do you think that > > > > would be worth doing? > > > > > > > > > I can try doing this. If my understanding is right, you are suggesting > > > that doing this later would mean the alias would have already been > > > applied?. However, for multi-versioning symbols, I must parse the target > > > name to insert the module name in between right? > > > > > > Yes, Clang will apply the alias, do all name mangling, etc, and then > > renaming will happen later. > > > > It's not clear to me if the order of the `.<target>` suffix on > > multi-versioned symbols matters. @erichkeane, should it? Supposing some > > mid-level optimization came along and did function versioning, it would > > rename the internal function and append `.1`. So, I think appending as you > > implemented is fine. > > > > --- > > > > All of my concerns have been addressed, and I think everyone else's are as > > well. We have documentation indicating what we mean by uniqueness which > > @hubert.reinterpretcast wanted clarified, we had the prefix map issue which > > I believe is addressed, so to my knowledge this is ready. > > > > Let's wait a bit to hear from Erich, but otherwise I think this is ready in > > a few days. > > > Multiversioning doesn't care about the names, simply that they are > unique/unique from eachother. The suffix that GCC/ICC put together for these > two types is simply to make sure we don't get something that conflicts with > another symbol. As long as these symbols are unique (and correclty updated!) > , they can be named absolutely anything. Thanks for the comments! I was one of the authors of the GCC multiversioning patch, and if I recall correctly from years ago, one of the reasons for naming the target clones of multi-versioned foo as foo, foo.sse, etc. was so that c++filt works nicely on this. Like, c++filt on _Z3foov.see would demangle as foo() [clone sse]. But c++filt is broken IMO as it is not handling these suffixes correctly any more, like c++filt _Z3foov.sse4.2 does not even demangle as expected. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73307/new/ https://reviews.llvm.org/D73307 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
