[PATCH] D58260: [INLINER] allow inlining of address taken blocks

[Eli Friedman via Phabricator via llvm-commits]

efriedma added a comment.

I'm concerned that "nocapture" is not sufficient to describe the necessary property here.

Two examples I've thought of:

1. Suppose the function is recursive.  It could pass a blockaddress to itself, which wouldn't actually capture the address because the copy goes away before the function returns.
2. Suppose the CallInst is not in the same function as the basic block.  Similar to above, it could pass the blockaddress the function.  Or maybe it do something else like an equality comparison against another copy of the blockaddress.  (I think nocapture doesn't actually forbid checking if two nocapture pointers are equal, although the documentation isn't really clear on that.)



================
Comment at: llvm/lib/IR/BasicBlock.cpp:447
+  for (const Use& U : BlockAddress::get(this)->uses())
+    if (const CallInst* CI = dyn_cast<CallInst>(U))
+      if (!CI->paramHasAttr(U.getOperandNo(), Attribute::NoCapture))
----------------
This is backwards; you need to return true if the use is not a CallInst.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D58260