[PATCH] D103867: [CHR] Don't run ControlHeightReduction if any BB has address taken

[Hongtao Yu via Phabricator via llvm-commits]

hoy added a comment.

In D103867#2814911 <https://reviews.llvm.org/D103867#2814911>, @wenlei wrote:

> In D103867#2814684 <https://reviews.llvm.org/D103867#2814684>, @lxfind wrote:
>
>>> Poking around it looks like this should be handled for inlining, not sure why the previous case we ran into wasn't covered by that code below from `CallAnalyzer::analyze()`
>>>
>>>   // Disallow inlining a blockaddress with uses other than strictly callbr.
>>>   // A blockaddress only has defined behavior for an indirect branch in the
>>>   // same function, and we do not currently support inlining indirect
>>>   // branches.  But, the inliner may not see an indirect branch that ends up
>>>   // being dead code at a particular call site. If the blockaddress escapes
>>>   // the function, e.g., via a global variable, inlining may lead to an
>>>   // invalid cross-function reference.
>>>   // FIXME: pr/39560: continue relaxing this overt restriction.
>>>   if (BB->hasAddressTaken())
>>>     for (User *U : BlockAddress::get(&*BB)->users())
>>>       if (!isa<CallBrInst>(*U))
>>>         return InlineResult::failure("blockaddress used outside of callbr");
>>>
>>>> LGTM, thanks for the fix.
>>
>> I think because that code is only checking for CallBrInst users. But in a computed goto pattern, the addresses are usually just stored in a global variable.
>
> Taking address of a block and storing it in a jump table does not introduce a use in the IR, however, if we use indirect branch to access the jump table like computed goto, we do count the indirect branch as use of the address taken labels, which in turns blocks inlining. See https://godbolt.org/z/8jYxzq867 (If we comment out the check for disallowing all indirbranch, it would be caught by the check quoted above)..

The following example can be used to tell the difference between GCC and LLVM regarding inlining. GCC won't inline function `foo` into `bar` while clang will. I was thinking this can be fixed in LLVM by enforcing a noinline attribute to functions of which the code labels are used in a static variable initializer. Fixing in the inliner might be better.

  char *g;
  void go();
  void   foo(char *p)
  {
     static const void* array[] = {&&L};
  L:
     if (p) {
     p--;
     go();
     goto L;
     }
     return;
  }
  void bar()
  {
    foo(g);
  }


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103867