[PATCH] D53285: [MergeFuncs] Generate alias instead of thunk if possible (default disabled)

whitequark via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Oct 29 02:57:53 PDT 2018


whitequark added inline comments.


================
Comment at: lib/Transforms/IPO/MergeFunctions.cpp:803
 
-    if (!isThunkProfitable(F)) {
+    if (!isThunkProfitable(F) && (!canCreateAliasFor(F) || !canCreateAliasFor(G))) {
       return;
----------------
nikic wrote:
> whitequark wrote:
> > Can you explain this logic a bit? It's not clear to me why the condition should be written like that (two canCreateAliasFor` calls).
> The idea here is that we need both of the writeThunkOrAlias calls below to succeed, otherwise we might be leaving behind functions in a partially rewired state. Both calls will succeed if either both G and H below can be converted to aliases (are global unnamed addr) or if a thunk for F below is profitable.
> 
> The renaming that is going on here makes it slightly confusing, in that H below corresponds to F here for the purpose of the (signature-based) alias check and F below corresponds to F here for the purpose of the (body-based) thunk-profitability check.
Right. Do you think you can add this as a comment?


Repository:
  rL LLVM

https://reviews.llvm.org/D53285





More information about the llvm-commits mailing list