[PATCH] D73307: Unique Names for Functions with Internal Linkage

Fri Apr 17 14:05:18 PDT 2020

rnk added a subscriber: erichkeane.
rnk added a comment.

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.

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

https://reviews.llvm.org/D73307