[WinEHPrepare] Replace unreasonable funclet terminators with unreachable

[majnemer]

Summary

It is possible to be in a situation where more than one funclet token is
a valid SSA value. If we see a terminator which exits a funclet which
doesn't use the funclet's token, replace it with unreachable.

[JosephTremoulet]

LGTM with one nit.

lib/CodeGen/WinEHPrepare.cpp

3242–3243 ↗

(On Diff #32274)

I think this is subtle enough to merit a comment (along the lines of "We can't demote a token but it's safe to skip these since cloning will remove mismatched uses").

[JosephTremoulet]

Spoke too soon, sorry.

test/CodeGen/WinEH/wineh-demotion.ll

325 ↗

(On Diff #32274)

Is this input valid? I don't think the def of %cp0 dominates the use here (path where first invoke returns normally, second invoke raises exception reaches here via cleanup2 without passing through the def at cleanup1).

[JosephTremoulet]

test/CodeGen/WinEH/wineh-demotion.ll

325 ↗

(On Diff #32274)

Oh, I misread it; that 2nd invoke is unreachable and cleanup2 is only reachable from this cleanupret, which is dominated by cleanup1. Sorry for the noise, objection rescinded.

llvm/trunk/lib/CodeGen/WinEHPrepare.cpp

for (auto &Funclet : FuncletBlocks) {
     // Loop over all of the instructions in the function, fixing up operand
     // references as we go.  This uses VMap to do all the hard work.
     for (BasicBlock *BB : BlocksInFunclet)
       // Loop over all instructions, fixing each one as we find it...
       for (Instruction &I : *BB)
         RemapInstruction(&I, VMap, RF_IgnoreMissingEntries);
   }
 
+  // Remove implausible terminators and replace them with UnreachableInst.
+  for (auto &Funclet : FuncletBlocks) {
+    BasicBlock *FuncletPadBB = Funclet.first;
+    std::set<BasicBlock *> &BlocksInFunclet = Funclet.second;
+    Instruction *FirstNonPHI = FuncletPadBB->getFirstNonPHI();
+    auto *CatchPad = dyn_cast<CatchPadInst>(FirstNonPHI);
+    auto *CleanupPad = dyn_cast<CleanupPadInst>(FirstNonPHI);
+
+    for (BasicBlock *BB : BlocksInFunclet) {
+      TerminatorInst *TI = BB->getTerminator();
+      // CatchPadInst and CleanupPadInst can't transfer control to a ReturnInst.
+      bool IsUnreachableRet = isa<ReturnInst>(TI) && (CatchPad || CleanupPad);
+      // The token consumed by a CatchReturnInst must match the funclet token.
+      bool IsUnreachableCatchret = false;
+      if (auto *CRI = dyn_cast<CatchReturnInst>(TI))
+        IsUnreachableCatchret = CRI->getReturnValue() != CatchPad;
+      // The token consumed by a CleanupPadInst must match the funclet token.
+      bool IsUnreachableCleanupret = false;
+      if (auto *CRI = dyn_cast<CleanupReturnInst>(TI))
+        IsUnreachableCleanupret = CRI->getReturnValue() != CleanupPad;
+      if (IsUnreachableRet || IsUnreachableCatchret || IsUnreachableCleanupret) {
+        new UnreachableInst(BB->getContext(), TI);
+        TI->eraseFromParent();
+      }
+    }
+  }
+
   // Clean-up some of the mess we made by removing useles PHI nodes, trivial
   // branches, etc.
   for (Function::iterator FI = F.begin(), FE = F.end(); FI != FE;) {
     BasicBlock *BB = FI++;
     SimplifyInstructionsInBlock(BB);
     ConstantFoldTerminator(BB, /*DeleteDeadConditions=*/true);
     MergeBlockIntoPredecessor(BB);
   }
 
-  // TODO: Do something about cleanupblocks which branch to implausible
-  // cleanuprets.
-
   // We might have some unreachable blocks after cleaning up some impossible
   // control flow.
   removeUnreachableBlocks(F);
 
   // Recolor the CFG to verify that all is well.
   for (BasicBlock &BB : F) {
     size_t NumColors = BlockColors[&BB].size();
     assert(NumColors == 1 && "Expected monochromatic BB!");
void WinEHPrepare::insertPHIStore(
   new StoreInst(PredVal, SpillSlot, PredBlock->getTerminator());
 }
 
 // TODO: Share loads for same-funclet uses (requires dominators if funclets
 // aren't properly nested).
 void WinEHPrepare::demoteNonlocalUses(Value *V,
                                       std::set<BasicBlock *> &ColorsForBB,
                                       Function &F) {
+  // Tokens can only be used non-locally due to control flow involving
+  // unreachable edges.  Don't try to demote the token usage, we'll simply
+  // delete the cloned user later.
+  if (isa<CatchPadInst>(V) || isa<CleanupPadInst>(V))
+    return;
+
   DenseMap<BasicBlock *, Value *> Loads;
   AllocaInst *SpillSlot = nullptr;
   for (Value::use_iterator UI = V->use_begin(), UE = V->use_end(); UI != UE;) {
     Use &U = *UI++;
     auto *UsingInst = cast<Instruction>(U.getUser());
     BasicBlock *UsingBB = UsingInst->getParent();
 
     // Is the Use inside a block which is colored with a subset of the Def?

llvm/trunk/test/CodeGen/WinEH/wineh-demotion.ll

right:
   ; CHECK-NEXT: store i32 %y, i32* [[Slot]]
   ; CHECK-NEXT: invoke void @f
   invoke void @f()
           to label %exit unwind label %merge
 merge:
   ; CHECK: merge:
   ; CHECK-NOT: = phi
   %phi = phi i32 [ %x, %left ], [ %y, %right ]
-  catchpad void [] to label %catch unwind label %catchend
+  %cp = catchpad token [] to label %catch unwind label %catchend
 
 catch:
   ; CHECK: catch:
   ; CHECK: [[Reload:%[^ ]+]] = load i32, i32* [[Slot]]
   ; CHECK-NEXT: call void @h(i32 [[Reload]])
   call void @h(i32 %phi)
-  catchret void to label %exit
+  catchret token %cp to label %exit
 
 catchend:
   catchendpad unwind to caller
 
 exit:
   ret void
 }
 
right:
   ; CHECK-DAG:   store i32 2, i32* [[Slot1]]
   ; CHECK-DAG:   store i32 2, i32* [[Slot2]]
   ; CHECK: invoke void @f
   invoke void @f()
           to label %exit unwind label %merge.inner
 merge.inner:
   ; CHECK: merge.inner:
   ; CHECK-NOT: = phi
-  ; CHECK: catchpad void
+  ; CHECK: catchpad token
   %x = phi i32 [ 1, %left ], [ 2, %right ]
-  catchpad void [] to label %catch.inner unwind label %catchend.inner
+  %cpinner = catchpad token [] to label %catch.inner unwind label %catchend.inner
 
 catch.inner:
   ; Need just one store here because only %y is affected
   ; CHECK: catch.inner:
   %z = call i32 @g()
   ; CHECK:   store i32 %z
   ; CHECK-NEXT: invoke void @f
   invoke void @f()
           to label %catchret.inner unwind label %merge.outer
 
 catchret.inner:
-  catchret void to label %exit
+  catchret token %cpinner to label %exit
 catchend.inner:
   catchendpad unwind label %merge.outer
 
 merge.outer:
   ; CHECK: merge.outer:
   ; CHECK-NOT: = phi
-  ; CHECK: catchpad void
+  ; CHECK: [[CatchPad:%[^ ]+]] = catchpad token
   %y = phi i32 [ %x, %catchend.inner ], [ %z, %catch.inner ]
-  catchpad void [] to label %catch.outer unwind label %catchend.outer
+  %cpouter = catchpad token [] to label %catch.outer unwind label %catchend.outer
 
 catchend.outer:
   catchendpad unwind to caller
 
 catch.outer:
   ; Need to load x and y from two different slots since they're both live
   ; and can have different values (if we came from catch.inner)
   ; CHECK: catch.outer:
   ; CHECK-DAG: load i32, i32* [[Slot1]]
   ; CHECK-DAG: load i32, i32* [[Slot2]]
-  ; CHECK: catchret void to label
+  ; CHECK: catchret token [[CatchPad]] to label
   call void @h(i32 %x)
   call void @h(i32 %y)
-  catchret void to label %exit
+  catchret token %cpouter to label %exit
 
 exit:
   ret void
 }
 
 ; CHECK-LABEL: @test3(
 define void @test3(i1 %B) personality i32 (...)* @__CxxFrameHandler3 {
 entry:
   ; need to spill parameter %B and def %x since they're used in a funclet
   ; CHECK: entry:
   ; CHECK-DAG: store i1 %B, i1* [[SlotB:%[^ ]+]]
   ; CHECK-DAG: store i32 %x, i32* [[SlotX:%[^ ]+]]
   ; CHECK: invoke void @f
   %x = call i32 @g()
   invoke void @f()
           to label %exit unwind label %catchpad
 
 catchpad:
-  catchpad void [] to label %catch unwind label %catchend
+  %cp = catchpad token [] to label %catch unwind label %catchend
 
 catch:
   ; Need to reload %B here
   ; CHECK: catch:
   ; CHECK: [[ReloadB:%[^ ]+]] = load i1, i1* [[SlotB]]
   ; CHECK: br i1 [[ReloadB]]
   br i1 %B, label %left, label %right
 left:
   ; Use of %x is in a phi, so need reload here in pred
   ; CHECK: left:
   ; CHECK: [[ReloadX:%[^ ]+]] = load i32, i32* [[SlotX]]
   ; CHECK: br label %merge
   br label %merge
 right:
   br label %merge
 merge:
   ; CHECK: merge:
   ; CHECK:   %phi = phi i32 [ [[ReloadX]], %left ]
   %phi = phi i32 [ %x, %left ], [ 42, %right ]
   call void @h(i32 %phi)
-  catchret void to label %exit
+  catchret token %cp to label %exit
 
 catchend:
   catchendpad unwind to caller
 
 exit:
   ret void
 }
 
right:
   ; CHECK-NOT: store
   ; CHECK: store i32 %r, i32* [[Slot]]
   ; CHECK-NEXT: invoke void @f
   %r = call i32 @g()
   invoke void @f()
           to label %join unwind label %catchpad.inner
 catchpad.inner:
    ; CHECK: catchpad.inner:
-   ; CHECK-NEXT: catchpad void
+   ; CHECK-NEXT: catchpad token
    %phi.inner = phi i32 [ %l, %left ], [ %r, %right ]
-   catchpad void [] to label %catch.inner unwind label %catchend.inner
+   %cp1 = catchpad token [] to label %catch.inner unwind label %catchend.inner
 catch.inner:
-   catchret void to label %join
+   catchret token %cp1 to label %join
 catchend.inner:
    catchendpad unwind label  %catchpad.outer
 join:
   ; CHECK: join:
   ; CHECK-NOT: store
   ; CHECK: store i32 %j, i32* [[Slot]]
   ; CHECK-NEXT: invoke void @f
    %j = call i32 @g()
    invoke void @f()
            to label %exit unwind label %catchpad.outer
 catchpad.outer:
    ; CHECK: catchpad.outer:
-   ; CHECK-NEXT: catchpad void
+   ; CHECK-NEXT: catchpad token
    %phi.outer = phi i32 [ %phi.inner, %catchend.inner ], [ %j, %join ]
-   catchpad void [] to label %catch.outer unwind label %catchend.outer
+   %cp2 = catchpad token [] to label %catch.outer unwind label %catchend.outer
 catch.outer:
    ; CHECK: catch.outer:
    ; CHECK:   [[Reload:%[^ ]+]] = load i32, i32* [[Slot]]
    ; CHECK:   call void @h(i32 [[Reload]])
    call void @h(i32 %phi.outer)
-   catchret void to label %exit
+   catchret token %cp2 to label %exit
 catchend.outer:
    catchendpad unwind to caller
 exit:
    ret void
 }
 
 ; CHECK-LABEL: @test5(
 define void @test5() personality i32 (...)* @__CxxFrameHandler3 {
invoke.cont:
   ; CHECK-NEXT:   store i32 2, i32* [[CleanupSlot]]
   ; CHECK-NEXT:   invoke void @f
   invoke void @f()
           to label %invoke.cont2 unwind label %cleanup
 
 cleanup:
   ; cleanup phi can be loaded at cleanup entry
   ; CHECK: cleanup:
-  ; CHECK-NEXT: cleanuppad void
+  ; CHECK-NEXT: cleanuppad token
   ; CHECK: [[CleanupReload:%[^ ]+]] = load i32, i32* [[CleanupSlot]]
   %phi.cleanup = phi i32 [ 1, %entry ], [ 2, %invoke.cont ]
-  cleanuppad void []
+  %cp = cleanuppad token []
   %b = call i1 @i()
   br i1 %b, label %left, label %right
 
 left:
   ; CHECK: left:
   ; CHECK:   call void @h(i32 [[CleanupReload]]
   call void @h(i32 %phi.cleanup)
   br label %merge
 
 right:
   ; CHECK: right:
   ; CHECK:   call void @h(i32 [[CleanupReload]]
   call void @h(i32 %phi.cleanup)
   br label %merge
 
 merge:
   ; need store for %phi.catch
   ; CHECK:      merge:
   ; CHECK-NEXT:   store i32 [[CleanupReload]], i32* [[CatchSlot:%[^ ]+]]
-  ; CHECK-NEXT:   cleanupret void
-  cleanupret void unwind label %catchpad
+  ; CHECK-NEXT:   cleanupret token
+  cleanupret token %cp unwind label %catchpad
 
 invoke.cont2:
   ; need store for %phi.catch
   ; CHECK:      invoke.cont2:
   ; CHECK-NEXT:   store i32 3, i32* [[CatchSlot]]
   ; CHECK-NEXT:   invoke void @f
   invoke void @f()
           to label %exit unwind label %catchpad
 
 catchpad:
   ; CHECK: catchpad:
-  ; CHECK-NEXT: catchpad void
+  ; CHECK-NEXT: catchpad token
   %phi.catch = phi i32 [ %phi.cleanup, %merge ], [ 3, %invoke.cont2 ]
-  catchpad void [] to label %catch unwind label %catchend
+  %cp2 = catchpad token [] to label %catch unwind label %catchend
 
 catch:
   ; CHECK: catch:
   ; CHECK:   [[CatchReload:%[^ ]+]] = load i32, i32* [[CatchSlot]]
   ; CHECK:   call void @h(i32 [[CatchReload]]
   call void @h(i32 %phi.catch)
-  catchret void to label %exit
+  catchret token %cp2 to label %exit
 
 catchend:
   catchendpad unwind to caller
 
 exit:
   ret void
 }
 
 ; CHECK-LABEL: @test6(
 define void @test6() personality i32 (...)* @__CxxFrameHandler3 {
 entry:
   ; Since %x needs to be stored but the edge to loop is critical,
   ; it needs to be split
   ; CHECK: entry:
   ; CHECK: invoke i32 @g
   ; CHECK-NEXT: to label %[[SplitBlock:[^ ]+]] unwind label %to_caller
   %x = invoke i32 @g()
           to label %loop unwind label %to_caller
   ; The store should be in the split block
   ; CHECK: [[SplitBlock]]:
   ; CHECK: store i32 %x, i32* [[SpillSlot:%[^ ]+]]
   ; CHECK: br label %loop
 to_caller:
-  cleanuppad void []
-  cleanupret void unwind to caller
+  %cp1 = cleanuppad token []
+  cleanupret token %cp1 unwind to caller
 loop:
   invoke void @f()
           to label %loop unwind label %cleanup
 cleanup:
   ; CHECK: cleanup:
   ; CHECK:   [[Load:%[^ ]+]] = load i32, i32* [[SpillSlot]]
   ; CHECK:   call void @h(i32 [[Load]])
-  cleanuppad void []
+  %cp2 = cleanuppad token []
   call void @h(i32 %x)
-  cleanupret void unwind to caller
+  cleanupret token %cp2 unwind to caller
 }
 
 ; CHECK-LABEL: @test7(
 define void @test7() personality i32 (...)* @__CxxFrameHandler3 {
 entry:
   ; %x is an EH pad phi, so gets stored in pred here
   ; CHECK: entry:
   ; CHECK:   store i32 1, i32* [[SlotX:%[^ ]+]]
   ; CHECK:   invoke void @f()
   invoke void @f()
      to label %invoke.cont unwind label %catchpad
 invoke.cont:
   ; %x is an EH pad phi, so gets stored in pred here
   ; CHECK: invoke.cont:
   ; CHECK:   store i32 2, i32* [[SlotX]]
   ; CHECK:   invoke void @f()
   invoke void @f()
     to label %exit unwind label %catchpad
 catchpad:
   ; %x phi should be eliminated
   ; CHECK: catchpad:
-  ; CHECK-NEXT: catchpad void
+  ; CHECK-NEXT: %[[CatchPad:[^ ]+]] = catchpad token
   %x = phi i32 [ 1, %entry ], [ 2, %invoke.cont ]
-  catchpad void [] to label %catch unwind label %catchend
+  %cp = catchpad token [] to label %catch unwind label %catchend
 catch:
   %b = call i1 @i()
   br i1 %b, label %left, label %right
 left:
   ; Edge from %left to %join needs to be split so that
   ; the load of %x can be inserted *after* the catchret
   ; CHECK: left:
-  ; CHECK-NEXT: catchret void to label %[[SplitLeft:[^ ]+]]
-  catchret void to label %join
+  ; CHECK-NEXT: catchret token %[[CatchPad]] to label %[[SplitLeft:[^ ]+]]
+  catchret token %cp to label %join
   ; CHECK: [[SplitLeft]]:
   ; CHECK:   [[LoadX:%[^ ]+]] = load i32, i32* [[SlotX]]
   ; CHECK:   br label %join
 right:
   ; Edge from %right to %join needs to be split so that
   ; the load of %y can be inserted *after* the catchret
   ; CHECK: right:
   ; CHECK:   store i32 %y, i32* [[SlotY:%[^ ]+]]
-  ; CHECK:   catchret void to label %[[SplitRight:[^ ]+]]
+  ; CHECK:   catchret token %[[CatchPad]] to label %[[SplitRight:[^ ]+]]
   %y = call i32 @g()
-  catchret void to label %join
+  catchret token %cp to label %join
   ; CHECK: [[SplitRight]]:
   ; CHECK:   [[LoadY:%[^ ]+]] = load i32, i32* [[SlotY]]
   ; CHECK:   br label %join
 catchend:
   catchendpad unwind to caller
 join:
   ; CHECK: join:
   ; CHECK:   %phi = phi i32 [ [[LoadX]], %[[SplitLeft]] ], [ [[LoadY]], %[[SplitRight]] ]
   %phi = phi i32 [ %x, %left ], [ %y, %right ]
   call void @h(i32 %phi)
   br label %exit
 exit:
   ret void
 }
+
+; CHECK-LABEL: @test8(
+define void @test8() personality i32 (...)* @__CxxFrameHandler3 { entry:
+  invoke void @f()
+          to label %done unwind label %cleanup1
+  invoke void @f()
+          to label %done unwind label %cleanup2
+
+done:
+  ret void
+
+cleanup1:
+  ; CHECK: [[CleanupPad1:%[^ ]+]] = cleanuppad token
+  ; CHECK-NEXT: call void @f()
+  ; CHECK-NEXT: cleanupret token [[CleanupPad1]]
+  %cp0 = cleanuppad token []
+  br label %cleanupexit
+
+cleanup2:
+  ; CHECK: cleanuppad token
+  ; CHECK-NEXT: call void @f()
+  ; CHECK-NEXT: unreachable
+  %cp1 = cleanuppad token []
+  br label %cleanupexit
+
+cleanupexit:
+  call void @f()
+  cleanupret token %cp0 unwind label %cleanup2
+}