[PATCH] D58260: [INLINER] allow inlining of blockaddresses if sole uses are callbrs

[Nick Desaulniers via Phabricator via llvm-commits]

nickdesaulniers marked 2 inline comments as done.
nickdesaulniers added inline comments.


================
Comment at: llvm/include/llvm/Analysis/LazyCallGraph.h:1094
+              break;
+            }
         if (Visited.insert(BA->getFunction()).second)
----------------
efriedma wrote:
> srhines wrote:
> > nickdesaulniers wrote:
> > > Is there a better way to express `SomeConstant->getFunction()`?  I guess not, since the constant can appear in multiple functions?  Is there a better way to do what I'm trying to do here?
> > I don't think there is a much better way to detect this, but BA->getFunction() will always return the same result here, so abstracting that out (e.g. FunctionContainingBlockAddress) would make this more obvious.
> > 
> > One other (possibly silly) question. Is it possible for a use of a blockaddress to be not contained in an Instruction (i.e. as part of the definition of some other global constant)? I don't think that makes sense in the IR, so perhaps it is forbidden some other way, but I just wanted to double check.
> > Is it possible for a use of a blockaddress to be not contained in an Instruction
> 
> Yes; C testcase:
> 
> ```
> void f() { static void*p = &&Z; goto *p; Z:return; }
> ```
> 
> ------
> 
> I'm not sure why we aren't just ignoring blockaddress constants here; they shouldn't really be relevant to the call graph.  You can't get the address of a function from a blockaddress.
> Yes; C testcase:

[[ https://godbolt.org/z/JXbZYW | Godbolt Link ]]


> I'm not sure why we aren't just ignoring blockaddress constants here; they shouldn't really be relevant to the call graph.

I //think// it's because the IR allows you to refer to blocks scoped to different functions, so if you're iterating the call graph, it's possible that you have a reference from one function to another via blockaddresses.


================
Comment at: llvm/include/llvm/Analysis/LazyCallGraph.h:1096
         if (Visited.insert(BA->getFunction()).second)
-          Worklist.push_back(BA->getFunction());
+          // If the blockaddress' function it refers to is a function it's not
+          // defined within, visit the other function.
----------------
srhines wrote:
> "If the blockaddress is used outside of the function it is defined within ..."
Not just that, because if `g()` contains a blockaddress that refers to a block within `f()`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D58260