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

[Eli Friedman via Phabricator via llvm-commits]

efriedma added a comment.

In D132084#3733041 <https://reviews.llvm.org/D132084#3733041>, @ychen wrote:

> In D132084#3733036 <https://reviews.llvm.org/D132084#3733036>, @ychen wrote:
>
>> In D132084#3732941 <https://reviews.llvm.org/D132084#3732941>, @efriedma wrote:
>>
>>> In general, you can't clone when indirectbrs are involved: since the blockaddress is a constant, it needs to be valid for all the cloned versions of the indirectbr.  And you can't make different basic blocks with the same address.
>>
>> I mean, there are existing VMap for new blockaddress and old blockaddress. So each cloned indirectbr has its own set of blockaddress. For this case, the cloning logic does not apply this VMap to GlobalValue that uses blockaddress. no?
>
> And the side effect is that any  GlobalValue referencing blockaddress needs to be cloned too. Most of the time, this is just an array of blockaddress.

The problem isn't a mechanical "how to I clone a value".

The problem is this: suppose you have a function with an indirectbr.  Then you clone it.  Now you have two indirectbrs, each with a different destination.  So you need to rewrite each use of a blockaddress to use the right value, depending on which indirectbr it's going to be passed to.  But in general, you can't determine that at the point of use of the indirectbr; you can't tell until the indirectbr is about to be executed.  blockaddresses can be stored in global variables, passed to functions, returned from functions, etc.

There are only two ways out of this, in general: either avoid cloning, or transform the indirectbr into a switch (or equivalent). We avoid the indirectbr->switch transform where possible because it adds an extra table lookup to code which is probably performance-sensitive.  (We use IndirectBrExpandPass on targets where basic blocks don't have addresses, like wasm.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132084