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

[Amara Emerson via Phabricator via llvm-commits]

aemerson created this revision.
aemerson added reviewers: paquette, arsenm, t.p.northover, Kai, qcolombet.
aemerson added a project: LLVM.
Herald added a subscriber: hiraditya.
Herald added a project: All.
aemerson requested review of this revision.
Herald added a subscriber: wdng.

We currently have a bug where the legalizer, when dealing with phi operands,
may create instructions in the phi's incoming blocks at points which are effectively
dead due to a possible exception throw.

Say we have:

  throwbb:
  EH_LABEL
  x0 = %callarg1
  BL @may_throw_call
  EH_LABEL
  B returnbb
  
  bb:
  %v = phi i1 %true, throwbb, %false....

When legalizing we may need to widen the `i1 %true` value, and to do that we need
to create new extension instructions in the incoming block. Our insertion point
currently is the `MBB::getFirstTerminator()` which puts the IP before the unconditional
branch terminator in throwbb. These extensions may never be executed if the call
throws, and therefore we need to emit them before the call (but not too early, since
our new instruction may need values defined within throwbb as well).

  throwbb:
  EH_LABEL
  x0 = %callarg1
  BL @may_throw_call
  EH_LABEL
  %true = G_CONSTANT i32 1 ; <<<-- ruh'roh, this never executes if may_throw_call() throws!
  B returnbb
  
  bb:
  %v = phi i32 %true, throwbb, %false....

To fix this, I've added two new instructions. The main idea is that `G_INVOKE_REGION_START`
is a terminator, which tries to model the fact that in the IR, the original invoke inst
is actually a terminator as well. By using that as the new insertion point, we
make sure to place new instructions on always executing paths.

`G_INVOKE_REGION_END` isn't actually used, but I added it for symmetry.

Unfortunately we still need to make the legalizer use a new insertion point API
that I've added, since the existing `getFirstTerminator()` method does a reverse
walk up the block, and any non-terminator instructions cause it to bail out. To
avoid impacting compile time for all `getFirstTerminator()` uses, I've added a new
method that does a forward walk instead.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D137905

Files:
  llvm/docs/GlobalISel/GenericOpcode.rst
  llvm/include/llvm/CodeGen/MachineBasicBlock.h
  llvm/include/llvm/Support/TargetOpcodes.def
  llvm/include/llvm/Target/GenericOpcodes.td
  llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp
  llvm/lib/CodeGen/GlobalISel/InstructionSelect.cpp
  llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp
  llvm/lib/CodeGen/MachineBasicBlock.cpp
  llvm/lib/CodeGen/MachineVerifier.cpp
  llvm/test/CodeGen/AArch64/GlobalISel/invoke-region.ll
  llvm/test/CodeGen/AArch64/GlobalISel/irtranslator-exceptions.ll
  llvm/test/CodeGen/AArch64/GlobalISel/irtranslator-unwind-inline-asm.ll
  llvm/test/CodeGen/AArch64/GlobalISel/legalize-exceptions.ll
  llvm/test/CodeGen/AArch64/GlobalISel/legalizer-info-validation.mir

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D137905.474988.patch
Type: text/x-patch
Size: 16747 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20221113/dbff44d5/attachment.bin>