[WebAssembly] Place 'try' and 'catch' correctly wrt EH_LABELs

[aheejin]

Summary

After instruction selection phase, possibly-throwing calls, which were
previously invoke, are wrapped in EH_LABEL instructions. For example:

EH_LABEL <mcsymbol .Ltmp0>
CALL_VOID @foo ...
EH_LABEL <mcsymbol .Ltmp1>

EH_LABEL is placed also in the beginning of EH pads:

bb.1 (landing-pad):
  EH_LABEL <mcsymbol .Ltmp2>
  ...

And we'd like to maintian this relationship, so when we place a try,

TRY ...
EH_LABEL <mcsymbol .Ltmp0>
CALL_VOID @foo ...
EH_LABEL <mcsymbol .Ltmp1>

When we place a catch,

bb.1 (landing-pad):
  EH_LABEL <mcsymbol .Ltmp2>
  %0:except_ref = CATCH ...
  ...

Previously we didn't treat EH_LABELs specially, so try was placed
right before a call, and catch was placed in the beginning of an EH
pad.

[aheejin]

Add comments on the test case

[dschuff]

So by "correctly" you mean that the TRY goes before the EH_LABEL rather than between the label and the call, and the CATCH goes after the label rather than at the top of the BB?

test/CodeGen/WebAssembly/eh-labels.mir

25 ↗

(On Diff #189171)

Should this be CHECK: EH_LABEL? (also below)

[aheejin]

In D58914#1426543, @dschuff wrote:

So by "correctly" you mean that the TRY goes before the EH_LABEL rather than between the label and the call, and the CATCH goes after the label rather than at the top of the BB?

Yes. We currently don't really use these labels (there are platforms who do), but they are generated anyway, so I'd respect their semantics for now.

[aheejin]

• Test fix

llvm/trunk/lib/Target/WebAssembly/WebAssemblyCFGStackify.cpp

#endif
   // throw.
   if (MBB.isPredecessor(Header)) {
     auto TermPos = Header->getFirstTerminator();
     if (TermPos == Header->end() ||
         TermPos->getOpcode() != WebAssembly::RETHROW) {
       for (const auto &MI : reverse(*Header)) {
         if (MI.isCall()) {
           AfterSet.insert(&MI);
+          // Possibly throwing calls are usually wrapped by EH_LABEL
+          // instructions. We don't want to split them and the call.
+          if (MI.getIterator() != Header->begin() &&
+              std::prev(MI.getIterator())->isEHLabel())
+            AfterSet.insert(&*std::prev(MI.getIterator()));
           break;
         }
       }
     }
   }
 
   // Add the TRY.
   auto InsertPos = getLatestInsertPos(Header, BeforeSet, AfterSet);

llvm/trunk/lib/Target/WebAssembly/WebAssemblyLateEHPrepare.cpp

 #include "WebAssemblySubtarget.h"
 #include "WebAssemblyUtilities.h"
 #include "llvm/ADT/SmallSet.h"
 #include "llvm/CodeGen/MachineInstrBuilder.h"
 #include "llvm/CodeGen/WasmEHFuncInfo.h"
 #include "llvm/MC/MCAsmInfo.h"
 using namespace llvm;
 
-#define DEBUG_TYPE "wasm-exception-prepare"
+#define DEBUG_TYPE "wasm-late-eh-prepare"
 
 namespace {
 class WebAssemblyLateEHPrepare final : public MachineFunctionPass {
   StringRef getPassName() const override {
     return "WebAssembly Late Prepare Exception";
   }
 
   bool runOnMachineFunction(MachineFunction &MF) override;

 // Add catch instruction to beginning of catchpads and cleanuppads.
 bool WebAssemblyLateEHPrepare::addCatches(MachineFunction &MF) {
   bool Changed = false;
   const auto &TII = *MF.getSubtarget<WebAssemblySubtarget>().getInstrInfo();
   MachineRegisterInfo &MRI = MF.getRegInfo();
   for (auto &MBB : MF) {
     if (MBB.isEHPad()) {
       Changed = true;
+      auto InsertPos = MBB.begin();
+      if (InsertPos->isEHLabel()) // EH pad starts with an EH label
+        ++InsertPos;
       unsigned DstReg =
           MRI.createVirtualRegister(&WebAssembly::EXCEPT_REFRegClass);
-      BuildMI(MBB, MBB.begin(), MBB.begin()->getDebugLoc(),
+      BuildMI(MBB, InsertPos, MBB.begin()->getDebugLoc(),
               TII.get(WebAssembly::CATCH), DstReg);
     }
   }
   return Changed;
 }
 
 // Wasm uses 'br_on_exn' instruction to check the tag of an exception. It takes
 // except_ref type object returned by 'catch', and branches to the destination
for (auto &MI : MBB) {
           TerminatePads.insert(getMatchingEHPad(&MI));
       }
     }
   }
 
   for (auto *Extract : ExtractInstrs) {
     MachineBasicBlock *EHPad = getMatchingEHPad(Extract);
     assert(EHPad && "No matching EH pad for extract_exception");
-    MachineInstr *Catch = &*EHPad->begin();
+    auto CatchPos = EHPad->begin();
+    if (CatchPos->isEHLabel()) // EH pad starts with an EH label
+      ++CatchPos;
+    MachineInstr *Catch = &*CatchPos;
+
     if (Catch->getNextNode() != Extract)
       EHPad->insert(Catch->getNextNode(), Extract->removeFromParent());
 
     // - Before:
     // ehpad:
     //   %exnref:except_ref = catch
     //   %exn:i32 = extract_exception
     //   ... use exn ...
for (auto &MBB : MF) {
 
     // Insert __stack_pointer restoring instructions at the beginning of each EH
     // pad, after the catch instruction. Here it is safe to assume that SP32
     // holds the latest value of __stack_pointer, because the only exception for
     // this case is when a function uses the red zone, but that only happens
     // with leaf functions, and we don't restore __stack_pointer in leaf
     // functions anyway.
     auto InsertPos = MBB.begin();
-    if (MBB.begin()->getOpcode() == WebAssembly::CATCH)
-      InsertPos++;
+    if (InsertPos->isEHLabel()) // EH pad starts with an EH label
+      ++InsertPos;
+    if (InsertPos->getOpcode() == WebAssembly::CATCH)
+      ++InsertPos;
     FrameLowering->writeSPToGlobal(WebAssembly::SP32, MF, MBB, InsertPos,
                                    MBB.begin()->getDebugLoc());
   }
   return Changed;
 }

llvm/trunk/test/CodeGen/WebAssembly/eh-labels.mir

+# RUN: llc -mtriple=wasm32-unknown-unknown -exception-model=wasm -mattr=+exception-handling -run-pass wasm-late-eh-prepare -run-pass wasm-cfg-stackify %s -o - | FileCheck %s
+
+# This tests 'try' and 'catch' instructions are correctly placed with respect to
+# EH_LABEL instructions.
+--- |
+  target datalayout = "e-m:e-p:32:32-i64:64-n32:64-S128"
+  target triple = "wasm32-unknown-unknown"
+
+  declare i32 @__gxx_wasm_personality_v0(...)
+  declare void @foo()
+  define void @eh_label_test() personality i8* bitcast (i32 (...)* @__gxx_wasm_personality_v0 to i8*) {
+    ret void
+  }
+...
+
+---
+# CHECK-LABEL: eh_label_test
+name: eh_label_test
+liveins:
+  - { reg: '$arguments' }
+body: |
+  bb.0:
+    ; TRY should be before EH_LABEL wrappers of throwing calls
+    ; CHECK:      TRY
+    ; CHECK-NEXT: EH_LABEL
+    ; CHECK-NEXT: CALL_VOID @foo
+    ; CHECK-NEXT: EH_LABEL
+    successors: %bb.1, %bb.2
+    EH_LABEL <mcsymbol .Ltmp0>
+    CALL_VOID @foo, implicit-def dead $arguments, implicit $sp32, implicit $sp64
+    EH_LABEL <mcsymbol .Ltmp1>
+
+  bb.1 (landing-pad):
+  ; predecessors: %bb.0
+    successors: %bb.2
+    ; CATCH should be after an EH_LABEL at the beginning of an EH pad
+    ; CHECK:      EH_LABEL
+    ; CHECK-NEXT: CATCH
+    EH_LABEL <mcsymbol .Ltmp2>
+    dead %0:i32 = EXTRACT_EXCEPTION_I32 implicit-def dead $arguments
+    CATCHRET %bb.2, %bb.0, implicit-def dead $arguments
+
+  bb.2:
+  ; predecessors: %bb.0, %bb.1
+    RETURN_VOID implicit-def dead $arguments
+...