[PATCH] D117509: [PartialInline] Bail out on asm-goto/callbr

[David Li via Phabricator via llvm-commits]

davidxl accepted this revision.
davidxl added a comment.
This revision is now accepted and ready to land.

lgtm



================
Comment at: llvm/lib/Transforms/IPO/PartialInlining.cpp:1420
+    // Don't bother with BlockAddress used by CallBr for asm goto.
+    if (isa<BlockAddress>(User))
+      continue;
----------------
wenlei wrote:
> hoy wrote:
> > davidxl wrote:
> > > hoy wrote:
> > > > davidxl wrote:
> > > > > Is it legal to partial inline in case when the function's block address is taken?
> > > > I think so. The non-inlined callsites will still call the original version, while the inlined callsites will pull in the new version. The new version has some context-irrelevant code outlined, but both versions should be semantically equal. Please correct me if I'm missing anything?
> > > what if the labels and references from asm goto are in different regions -- one in inlined region, and the other in the outllined region?
> > If internal blocks of an inlinee are address taken, the inlining should be disabled during the cost analysis: https://github.com/llvm/llvm-project/blob/main/llvm/lib/Analysis/InlineCost.cpp#L2656
> > 
> > what if the labels and references from asm goto are in different regions -- one in inlined region, and the other in the outllined region? 
> 
> Is this concerning side entrance into the outlined region through asm goto? The outlining for partial inline requires single entry single exit region, so asm goto should not introduce side entrance. 
> 
> > If internal blocks of an inlinee are address taken, the inlining should be disabled during the cost analysis: https://github.com/llvm/llvm-project/blob/main/llvm/lib/Analysis/InlineCost.cpp#L2656
> 
> The code you linked still allow callbr/asm-goto, it disables inlining if address taken blocks are used outside of asm-goto.
In general, the legality check should be done earlier otherwise we may end up with situation where function outlining happens but partial inlining is disabled -- resulting in less efficient code.  

However as Wenlei mentioned, cross region jumps may not be an issue.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117509