[PATCH] D137905: [GlobalISel] Add new G_INVOKE_REGION_START/END instructions to fix an EH bug

[Amara Emerson via Phabricator via llvm-commits]

aemerson added a comment.

In D137905#3931413 <https://reviews.llvm.org/D137905#3931413>, @arsenm wrote:

> I don't know anything about exceptions, but looking at translateInvoke, I'm confused by several things.
>
> Why do you need this new thing, instead of just looking for EH_LABEL? Why is this specially handling inlineasm such that it may turn off the label if the called operand can't throw as some kind of optimization? If it can't throw, wouldn't it optimize to use a regular call in the first place?

I’m not sure about the inline asm thing since I’m new to this code too, but semantically EH_LABEL isn’t a terminator, it’s a label used for computing exception table information. It seems cleaner to have a dedicated barrier instruction. The reason the legalizations insert at the first terminator is because that guarantees the instruction will execute. If you specifically look for an EH label then it’s leaking implementation details about instruction placement to the API client. Granted, having to use a forward walking ‘getFirstIteratorForward()’ isn’t ideal either.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137905