[PATCH] D132084: [Cloning] handle blockaddress array clone in the same module

[Yuanfang Chen via Phabricator via llvm-commits]

ychen added a comment.

In D132084#3736000 <https://reviews.llvm.org/D132084#3736000>, @efriedma wrote:

> In D132084#3735812 <https://reviews.llvm.org/D132084#3735812>, @ychen wrote:
>
>>>> the current situation of silently creating broken IR when cloning blockaddress is not ideal.
>>>
>>> I'm not sure what invariant we can enforce here.  Maybe something related to the CloneFunctionChangeType?
>>
>> That's hard in general and feels insufficient to just handle some cases but not the others. How about emitting a warning of sorts all the way back to the frontend if any blockaddress is written? Also, add a boolean flag for CloneFunction API to optionally call IndirectBrExpandPass's logic.
>
> Oh, I was just talking about the API invariant.  Clearly coroutine lowering needs to do something different here.  And probably clang should warn or error if indirect goto can't generate the expected code.

Add a new boolean flag `HandleIndirectBranch` defaulted to true: if a caller can know the blockaddress is safe to clone, they could set this to false(I expect this to be false most of the time). When `HandleIndirectBranch` is true, special-casing the case like `global blockaddress array for function F where F has no blockaddress captured/escaped(https://llvm.org/docs/LangRef.html#pointer-capture)` by cloning that array; for other cases, call IndirectBrExpandPass. Regardless of `HandleIndirectBranch` value, if a function references any global value and blockaddress maybe captured, we emit the warning/error because IndirectBrExpandPass can not handle all cases correctly.

> I forget, why do we have to clone the coroutine body?  I think I read an explanation at some point, but I'm not remembering where it was; it seems like it should be possible to avoid cloning.

Since the instructions executed before the first `co_await` call need to stay at the original function(ramp function). But for patterns like the below, this could not be decided statically so cloning is needed.

  task corofoo(){
    while(...){
      if(..) co_await();
      else <code>;
    }
  }

PS: I think it is helpful to add bound check for indirectbr. But I'm not sure where to put it, a new clang flag?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132084