[PATCH] D129288: [IR] Don't use blockaddresses as callbr arguments

Fri Jul 8 13:08:14 PDT 2022

nikic added a comment.

In D129288#3639440 <https://reviews.llvm.org/D129288#3639440>, @nickdesaulniers wrote:

> Also, if we eliminate `blockaddress` operands from `callbr`, I wonder if we can revert the bitcode reader+writer changes added recently in:
>
> - 23ec5782c3cc0d77b5e44285e5124cd4a3ffeeef <https://reviews.llvm.org/rG23ec5782c3cc0d77b5e44285e5124cd4a3ffeeef>
> - 6baaad740a5abb4bfcff022a8114abb4eea66a2d <https://reviews.llvm.org/rG6baaad740a5abb4bfcff022a8114abb4eea66a2d>
>
> Perhaps while retaining their tests.

I don't think so. These cases are related to `&&labels` (without `callbr`).

As a side note, this kind of usage of block labels is not supported by LLVM blockaddress semantics. `&&labels` are only supported in conjunction with computed goto (`indirectbr`). If the function with the label doesn't contain any computed goto, then LLVM is free to DCE all these blockaddresses. I believe the only reason why this currently doesn't happen is that we have a bunch of non-principled `hasAddressTaken()` checks strewn over the codebase that really shouldn't be there. If we drop those, `_THIS_IP_` will probably become equal to `1` in most cases.

I think the kernel would be much better served by a new intrinsic `llvm.instructionpointer` or so (and a corresponding clang builtin). As far as I understand, `_THIS_IP_` is only used as debugging information. Importantly, this address will never be used as an actual jump target. This usage really just wants to get the current instruction pointer at the point of the call, and doesn't need any of the blockaddress baggage that both inhibits a lot of optimization and only works by accident in the first place.

But well, this is kind of getting off topic...

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

https://reviews.llvm.org/D129288