[PATCH] D139883: [llvm][CallBrPrepare] add llvm.callbr.landingpad intrinsic

[Bill Wendling via Phabricator via llvm-commits]

void added a comment.

In D139883#4012018 <https://reviews.llvm.org/D139883#4012018>, @nickdesaulniers wrote:

> In D139883#4011907 <https://reviews.llvm.org/D139883#4011907>, @void wrote:
>
>> As we talked before, I *really* hate the use of an intrinsic here. We used an intrinsic for the old exception handling and it was nothing but buggy. Theoretically they work just fine, but the can't take into account issues due to inlining, block splitting, and other code motion changes (at least not without a hefty dose of special checks throughout the compiler). If we use an intrinsic here, I believe we're just going to revisit the errors of the past.
>
> The use of an instruction was tried here: https://reviews.llvm.org/D139565.
>
> Specifically, please see the feedback:
>
> - https://reviews.llvm.org/D139565#3979289
> - https://reviews.llvm.org/D139565#3982939
>
> While I agree with your points about intrinsic vs instruction; with the approach of the series (as mentioned in the RFC <https://discourse.llvm.org/t/rfc-syncing-asm-goto-with-outputs-with-gcc/65453/8?u=nickdesaulniers>), I'll note that the intrinsic doesn't live long enough to be relevant to inlining, block splitting, or other code motion changes. It is produced by callbrprepare shortly before being consumed by selectiondag.
>
> Please see the pipeline test changes in https://reviews.llvm.org/D140180 to get a sense of the pass "distance" between Prepare callbr (which introduces these intrinsics) and <arch> DAG->DAG Pattern Instruction Selection which consumes them.
>
> I need @void and @efriedma (and me and maybe @jyknight ) to be in agreement as the feedback around instruction vs intrinsic is giving me whiplash in code review. It's up to you two to agree; I can't have @void  reject intrinsics while @efriedma rejects instructions.

Thanks for the information. (And sorry I wasn't involved in those discussions.)

The main issue with the way EH used to be handled was that the intrinsic that held all of the information about what EH values are used, etc., could be moved away from the landing pad (via inlining and other normal code motion). It made associating the intrinsic with the proper `invoke` instruction(s) more-or-less impossible to get correct without having to jump through a ton of hoops. Creating the `landingpad` instruction, and forcing it to be the first non-PHI instruction in a BB, basically solved all EH issues that existed at that time.

So, if you can ensure that the intrinsic won't be moved around due to normal passes, or, if it is moved, that it won't matter (i.e., you'll be able to associate it with the `callbr` when necessary), then I'll withdraw my opposition to using an intrinsic.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139883