[PATCH] D63312: [Attributor] Deduce attributes for non-exact functions

[Nick Lewycky via Phabricator via llvm-commits]

nicholas added a comment.

> We can, however, use information from non-exact definitions to improve
>  themselves as they are either replaced as a whole or not at all.

I didn't understand the motivation of this patch by reading the description, I understood it with this comment. In particular I had the impression that you were using the attributes of one internalized function to optimize another internalized function.

---

I'm curious what function-local optimizations we're missing? If I understand the design, you can deduce that a function is 'readnone', but no callers of the function can be optimized based on that deduction, since the 'readnone' body may be replaced. I'm mostly confident we don't use readnone to optimize the body of the function. Do you have an example of an attribute that we can deduce and then use to optimize the body of the function?

---

If it were my design de novo, I would prefer treating the function attributes as being "as authoritative as the linkage", which pushes the problem of deciding whether `F->doesNotReturn()` is sufficient to the caller, it needs to check the linkage (or `isExactDefinition`) before asking that question. Unfortunately that would churn the API as we would need to update attribute consumers to use a convenience wrapper API when they're examining a callee instead of their function parent, but I expect pushing that decision into the pass might be necessary to get corner cases optimal and correct in passes that look at multiple functions. That seems to me like it would be a lot of work, what do you think?

Even if the wrapper function approach works, I don't think the representation is right. There aren't two functions, which is why you'll need to teach the inliner, etc., that these aren't truly functions. Without adding the concept to the LangRef, you've got two separate passes in LLVM that coordinate optimizations with each other though implementation details.

I'm not sure the wrapper can be used on functions that aren't marked `unnamed_addr` or you prove the address isn't taken? You update all users, which means that someone who takes the address of the function in this module might get a different address than someone who takes the address in a different module since they're two different internal functions now? Those pointers need to compare equal if you pass the function pointer around.



================
Comment at: llvm/lib/Transforms/IPO/Attributor.cpp:1187
+
+  StringRef Prefix = "__internal_";
+  F.setName(Prefix + F.getName());
----------------
FYI, I don't think we have a reserved prefix for llvm-produced functions, but you can make it anonymous with `F.setName("");`. Since the function has internal linkage, it doesn't need a name to link with other modules.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D63312