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

[Chandler Carruth via Phabricator via llvm-commits]

chandlerc added a comment.

I think you mentioned that there is already a test case that checks a global initialized with `blockaddress`? If I've mis-remembered, we should definitely add one and check that it *doesn't* inline (today). We can update it if/when we add support. If it is possible to add one that directly works w/ callbr, that'd be great.

FWIW, I largely agree with Bill that we *should* be able to do this in way more cases. I'd love to see patches doing that. I think the most difficult aspect will just be writing all the test cases for all the different patterns so that we're confident we're doing the right updates in each case (or detecting when we can't and blocking). But I don't think you need to do that to land this patch. =D I'm mostly hoping Bill or you works on it as a follow-up.



================
Comment at: llvm/include/llvm/Analysis/LazyCallGraph.h:1088-1098
+        bool AllUsersAreSameFn = true;
+        for (User *U : BA->users())
+          if (Instruction *I = dyn_cast<Instruction>(U))
+            if (I->getFunction() != BA->getFunction()) {
+              AllUsersAreSameFn = false;
+              break;
+            }
----------------
This can be written more simply with a predicate:

```
if (llvm::any_of(BA->users(),
                 [&](User *U) {
                   if (Instruction *I = dyn_cast<Instruction>(U))
                     return I->getFunction() != BA->getFunction();

                   return false;
                 }))
  Worklist.push_back(BA->getFunction());

continue;
```


================
Comment at: llvm/lib/Analysis/InlineCost.cpp:1830
+    // the function, e.g., via a global variable, inlining may lead to an
+    // invalid cross-function reference.
     if (BB->hasAddressTaken())
----------------
Add a FIXME that we should probably continue relaxing this along the lines suggested by Bill?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D58260