[reg2mem] Add special handling to CatchSwitchInst

[Naville]

Summary

Fix https://github.com/llvm/llvm-project/issues/59185

[Naville]

Add TestCase

[nikic]

Can you please upload the patch with full context (-U99999)?

llvm/lib/Transforms/Utils/DemoteRegToStack.cpp

100

You can probably combine handlers and unwind dest by iterating over successors(CSI).

158

Why does this insert a load before uses, rather than doing the same as in the previous function (insert load in each successor)?

llvm/test/Transforms/Reg2Mem/catchswitch-crach2.ll

1 ↗

(On Diff #477731)

Typo in file name: crach -> crash?

8 ↗

(On Diff #477731)

Can you please convert the test to use opaque pointers? This will remove these types.

I'd also replace the invoke undef with an invoke of a real function, to avoid UB. Something like this:

declare i32 @__CxxFrameHandler3(...)
declare i64 @fn() 

define void @testreg2mem(ptr %_Val) personality ptr @__CxxFrameHandler3 {
entry:
  %call.i166167 = invoke i64 @fn()
  to label %if.end56 unwind label %catch.dispatch

if.end56:                                         ; preds = %entry
  invoke void @"extfunc1"()
  to label %invoke.cont75 unwind label %catch.dispatch

invoke.cont75:                                    ; preds = %if.end56
  unreachable

catch.dispatch:                                   ; preds = %if.end56, %entry
  %_State.3 = phi i32 [ undef, %if.end56 ], [ 0, %entry ]
  %0 = catchswitch within none [label %catch] unwind label %ehcleanup105

catch:                                            ; preds = %catch.dispatch
  %1 = catchpad within %0 [ptr null, i32 64, ptr null]
  invoke void @"extfunc2"() [ "funclet"(token %1) ]
  to label %invoke.cont98 unwind label %ehcleanup105

invoke.cont98:                                    ; preds = %catch
  catchret from %1 to label %if.end99

if.end99:                                         ; preds = %invoke.cont98
  %or.i = or i32 undef, %_State.3
  unreachable

ehcleanup105:                                     ; preds = %catch, %catch.dispatch
  %2 = cleanuppad within none []
  cleanupret from %2 unwind to caller
}

declare void @"extfunc1"()

declare void @"extfunc2"()

[Naville]

llvm/lib/Transforms/Utils/DemoteRegToStack.cpp

158

Because otherwise we need another Map to record successor and the loadinst in it?

[Naville]

Don't we need :

• Insert loadinst in each immediate successor, map each successor BasicBlock with the load inst
• For each user(which might not be inside the immediate successor), query the dominator tree, check which immediate successor dominates the user and replace the use with the loadinst in that block?

[Naville]

DemoteReg does the same already in U->replaceUsesOfWith(&I, V);, so I think we're all good here

[nikic]

LGTM with some nits.

llvm/lib/Transforms/Utils/DemoteRegToStack.cpp

100

nit: Omit braces for single-line for.

152

This should be just isa<>, you are not using CSI.

154

nit: users -> Users.

158

Okay, I get it now: If we insert in successor, we need to replace dominating uses. This is possible, but your way is much simpler.

[Naville]

Fix style issues

llvm/lib/Transforms/Utils/DemoteRegToStack.cpp

AllocaInst *llvm::DemoteRegToStack(Instruction &I, bool VolatileLoads,
   }
 
   // Insert stores of the computed value into the stack slot. We have to be
   // careful if I is an invoke instruction, because we can't insert the store
   // AFTER the terminator instruction.
   BasicBlock::iterator InsertPt;
   if (!I.isTerminator()) {
     InsertPt = ++I.getIterator();
+    // Don't insert before PHI nodes or landingpad instrs.
     for (; isa<PHINode>(InsertPt) || InsertPt->isEHPad(); ++InsertPt)
-      /* empty */;   // Don't insert before PHI nodes or landingpad instrs.
+      if (isa<CatchSwitchInst>(InsertPt))
+        break;
+    if (isa<CatchSwitchInst>(InsertPt)) {
+      for (BasicBlock *Handler : successors(&*InsertPt))
+        new StoreInst(&I, Slot, &*Handler->getFirstInsertionPt());
+      return Slot;
+    }
   } else {
     InvokeInst &II = cast<InvokeInst>(I);
     InsertPt = II.getNormalDest()->getFirstInsertionPt();
   }
 
   new StoreInst(&I, Slot, &*InsertPt);
   return Slot;
 }
if (InvokeInst *II = dyn_cast<InvokeInst>(P->getIncomingValue(i))) {
              "Invoke edge not supported yet"); (void)II;
     }
     new StoreInst(P->getIncomingValue(i), Slot,
                   P->getIncomingBlock(i)->getTerminator());
   }
 
   // Insert a load in place of the PHI and replace all uses.
   BasicBlock::iterator InsertPt = P->getIterator();
-
+  // Don't insert before PHI nodes or landingpad instrs.
   for (; isa<PHINode>(InsertPt) || InsertPt->isEHPad(); ++InsertPt)
-    /* empty */;   // Don't insert before PHI nodes or landingpad instrs.
-
+    if (isa<CatchSwitchInst>(InsertPt))
+      break;
+  if (isa<CatchSwitchInst>(InsertPt)) {
+    // We need a separate load before each actual use of the PHI
+    SmallVector<Instruction *, 4> Users;
+    for (User *U : P->users()) {
+      Instruction *User = cast<Instruction>(U);
+      Users.push_back(User);
+    }
+    for (Instruction *User : Users) {
+      Value *V =
+          new LoadInst(P->getType(), Slot, P->getName() + ".reload", User);
+      User->replaceUsesOfWith(P, V);
+    }
+  } else {
     Value *V =
         new LoadInst(P->getType(), Slot, P->getName() + ".reload", &*InsertPt);
     P->replaceAllUsesWith(V);
-
+  }
   // Delete PHI.
   P->eraseFromParent();
   return Slot;
 }

llvm/test/Transforms/Reg2Mem/catchswitch-crash2.ll

+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
+; RUN: opt -passes=reg2mem -S < %s | FileCheck %s
+%opaque = type opaque
+
+declare i32 @__CxxFrameHandler3(...)
+
+define void @testreg2mem(i8* %_Val) personality i8* bitcast (i32 (...)* @__CxxFrameHandler3 to i8*) {
+; CHECK-LABEL: @testreg2mem(
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:    [[_STATE_3_REG2MEM:%.*]] = alloca i32, align 4
+; CHECK-NEXT:    [[_STATE_3_REG2MEM1:%.*]] = alloca i32, align 4
+; CHECK-NEXT:    %"reg2mem alloca point" = bitcast i32 0 to i32
+; CHECK-NEXT:    store i32 0, i32* [[_STATE_3_REG2MEM1]], align 4
+; CHECK-NEXT:    [[CALL_I166167:%.*]] = invoke noundef i64 @extfunc_i64()
+; CHECK-NEXT:    to label [[IF_END56:%.*]] unwind label [[CATCH_DISPATCH:%.*]]
+; CHECK:       if.end56:
+; CHECK-NEXT:    store i32 1, i32* [[_STATE_3_REG2MEM1]], align 4
+; CHECK-NEXT:    invoke void @extfunc()
+; CHECK-NEXT:    to label [[INVOKE_CONT75:%.*]] unwind label [[CATCH_DISPATCH]]
+; CHECK:       invoke.cont75:
+; CHECK-NEXT:    unreachable
+; CHECK:       catch.dispatch:
+; CHECK-NEXT:    [[TMP0:%.*]] = catchswitch within none [label %catch] unwind label [[EHCLEANUP105:%.*]]
+; CHECK:       catch:
+; CHECK-NEXT:    [[TMP1:%.*]] = catchpad within [[TMP0]] [i8* null, i32 64, i8* null]
+; CHECK-NEXT:    [[_STATE_3_RELOAD2:%.*]] = load i32, i32* [[_STATE_3_REG2MEM1]], align 4
+; CHECK-NEXT:    store i32 [[_STATE_3_RELOAD2]], i32* [[_STATE_3_REG2MEM]], align 4
+; CHECK-NEXT:    invoke void @extfunc() [ "funclet"(token [[TMP1]]) ]
+; CHECK-NEXT:    to label [[INVOKE_CONT98:%.*]] unwind label [[EHCLEANUP105]]
+; CHECK:       invoke.cont98:
+; CHECK-NEXT:    catchret from [[TMP1]] to label [[IF_END99:%.*]]
+; CHECK:       if.end99:
+; CHECK-NEXT:    [[_STATE_3_RELOAD:%.*]] = load i32, i32* [[_STATE_3_REG2MEM]], align 4
+; CHECK-NEXT:    [[OR_I:%.*]] = or i32 0, [[_STATE_3_RELOAD]]
+; CHECK-NEXT:    unreachable
+; CHECK:       ehcleanup105:
+; CHECK-NEXT:    [[TMP2:%.*]] = cleanuppad within none []
+; CHECK-NEXT:    [[_STATE_3_RELOAD3:%.*]] = load i32, i32* [[_STATE_3_REG2MEM1]], align 4
+; CHECK-NEXT:    store i32 [[_STATE_3_RELOAD3]], i32* [[_STATE_3_REG2MEM]], align 4
+; CHECK-NEXT:    cleanupret from [[TMP2]] unwind to caller
+;
+entry:
+  %call.i166167 = invoke noundef i64 @"extfunc_i64"()
+  to label %if.end56 unwind label %catch.dispatch
+
+if.end56:                                         ; preds = %entry
+  invoke void @"extfunc"()
+  to label %invoke.cont75 unwind label %catch.dispatch
+
+invoke.cont75:                                    ; preds = %if.end56
+  unreachable
+
+catch.dispatch:                                   ; preds = %if.end56, %entry
+  %_State.3 = phi i32 [ 1, %if.end56 ], [ 0, %entry ]
+  %0 = catchswitch within none [label %catch] unwind label %ehcleanup105
+
+catch:                                            ; preds = %catch.dispatch
+  %1 = catchpad within %0 [i8* null, i32 64, i8* null]
+  invoke void @"extfunc"() [ "funclet"(token %1) ]
+  to label %invoke.cont98 unwind label %ehcleanup105
+
+invoke.cont98:                                    ; preds = %catch
+  catchret from %1 to label %if.end99
+
+if.end99:                                         ; preds = %invoke.cont98
+  %or.i = or i32 0, %_State.3
+  unreachable
+
+ehcleanup105:                                     ; preds = %catch, %catch.dispatch
+  %2 = cleanuppad within none []
+  cleanupret from %2 unwind to caller
+}
+
+declare void @"extfunc"()
+
+declare i64 @"extfunc_i64"()