[PATCH] D75590: [Attributor] IPO across definition boundary of a function marked alwaysinline

Johannes Doerfert via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Mar 6 16:34:20 PST 2020


jdoerfert added a comment.

In the `initialize` method in question, ask the Attributor if a function is "IPO amendable". We need a new helper method for that in the Attributor. It will look at the position, linkage (via the isexact definition), and `alwaysinline` + inlinable "attributes". This way it we have a single location to do the lookup. It is reusable and easier to understand.



================
Comment at: llvm/lib/Transforms/IPO/Attributor.cpp:8216
+
+  if (isInlineViable(F).isSuccess())
+    InfoCache.InlineableFunctions.insert(&F);
----------------
Only check this if the `alwaysinline` attribute is set, it is not free and otherwise not needed.


================
Comment at: llvm/test/Transforms/Attributor/alwaysinline.ll:21
+
+; CHECK: Function Attrs: nofree nosync nounwind
+define void @outer1() {
----------------
bbn wrote:
> jdoerfert wrote:
> > is this one not readnone?
> I think it should be readnone, but I tried opt **without the patch**  with the following example:
> 
> ```
> define void @inner1() alwaysinline {
> [...]
> }
> ```
> 
>  and the `outer1` function does not have the `readnone` attribute, either.
> But when the function does not have `alwayinline` attribute:
> 
> ```
> define void @inner1() {
> [....]
> }
> ```
> 
> The `outer1` function will have the `readnone` attribute
I'm confused. Take a look at https://godbolt.org/z/EsQWyN . without the linkonce we determine readnone in either case.


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

https://reviews.llvm.org/D75590





More information about the llvm-commits mailing list