[win][x64] Re-use fixed object if multiple catchpads use the same alloca for their catch objects

[dpaoliello]

Addresses #147421 (comment) for x86

If more than one catchpad uses the same alloca for their catch objects, then we will allocate more than one object in the fixed area resulting in wasted stack space.

As a follow up, Clang could be updated to re-use the same alloca for all by-reference and by-pointer catch objects.

[llvmbot]

@llvm/pr-subscribers-platform-windows

@llvm/pr-subscribers-backend-x86

Author: Daniel Paoliello (dpaoliello)

Changes

Addresses <#147421 (comment)> for x86

If more than one catchpad uses the same alloca for their catch objects, then we will allocate more than one object in the fixed area resulting in wasted stack space.

As a follow up, Clang could be updated to re-use the same alloca for all by-reference and by-pointer catch objects.

---

Full diff: https://github.com/llvm/llvm-project/pull/147849.diff

2 Files Affected:

• (modified) llvm/lib/Target/X86/X86FrameLowering.cpp (+1-1)
• (added) llvm/test/CodeGen/WinEH/wineh-reuse-catch-alloca.ll (+100)

diff --git a/llvm/lib/Target/X86/X86FrameLowering.cpp b/llvm/lib/Target/X86/X86FrameLowering.cpp
index c96d3c15a8823..80b51cc5ab89b 100644
--- a/llvm/lib/Target/X86/X86FrameLowering.cpp
+++ b/llvm/lib/Target/X86/X86FrameLowering.cpp
@@ -4241,7 +4241,7 @@ void X86FrameLowering::adjustFrameForMsvcCxxEh(MachineFunction &MF) const {
   for (WinEHTryBlockMapEntry &TBME : EHInfo.TryBlockMap) {
     for (WinEHHandlerType &H : TBME.HandlerArray) {
       int FrameIndex = H.CatchObj.FrameIndex;
-      if (FrameIndex != INT_MAX) {
+      if ((FrameIndex != INT_MAX) && MFI.getObjectOffset(FrameIndex) == 0) {
         // Ensure alignment.
         unsigned Align = MFI.getObjectAlign(FrameIndex).value();
         MinFixedObjOffset -= std::abs(MinFixedObjOffset) % Align;
diff --git a/llvm/test/CodeGen/WinEH/wineh-reuse-catch-alloca.ll b/llvm/test/CodeGen/WinEH/wineh-reuse-catch-alloca.ll
new file mode 100644
index 0000000000000..2bd004ed7dec5
--- /dev/null
+++ b/llvm/test/CodeGen/WinEH/wineh-reuse-catch-alloca.ll
@@ -0,0 +1,100 @@
+; RUN: llc %s --mtriple=x86_64-pc-windows-msvc -o - | FileCheck %s
+
+; Tests the fixed object layouts when two catchpads re-use the same stack
+; allocation for this catch objects.
+
+; Generated from this C++ code, with modifications to the IR (see comments in
+; IR):
+; https://godbolt.org/z/9qv5Yn68j
+; > clang --target=x86_64-pc-windows-msvc test.cpp
+; ```
+; extern "C" void boom();
+; extern "C" int calls_boom();
+; {
+;     try { boom(); }
+;     catch (int& i) { return i; }
+;     catch (long& l) { return l; }
+;     return 0;
+; }
+; ```
+
+; Minimum stack alloc is 64 bytes, so no change there.
+; CHECK-LABEL:  calls_boom:
+; CHECK:        subq    $64, %rsp
+; CHECK:        .seh_stackalloc 64
+
+; Both the catch blocks load from the same address.
+; CHECK-LABEL:  "?catch$3@?0?calls_boom@4HA":
+; CHECK:        movq    -8(%rbp), %rax
+; CHECK-LABEL:  "?catch$4@?0?calls_boom@4HA":
+; CHECK:        movq    -8(%rbp), %rax
+
+; There's enough space for the UnwindHelp to be at 48 instead of 40
+; CHECK-LABEL:  $cppxdata$calls_boom:
+; CHECK:        .long   48                              # UnwindHelp
+
+; Both catches have the same object offset.
+; CHECK-LABEL:  $handlerMap$0$calls_boom:
+; CHECK:        .long   56                              # CatchObjOffset
+; CHECK-NEXT:   .long   "?catch$3@?0?calls_boom@4HA"@IMGREL # Handler
+; CHECK:        .long   56                              # CatchObjOffset
+; CHECK-NEXT:   .long   "?catch$4@?0?calls_boom@4HA"@IMGREL # Handler
+
+%rtti.TypeDescriptor2 = type { ptr, ptr, [3 x i8] }
+
+$"??_R0H@8" = comdat any
+
+$"??_R0J@8" = comdat any
+
+@"??_7type_info@@6B@" = external constant ptr
+@"??_R0H@8" = linkonce_odr global %rtti.TypeDescriptor2 { ptr @"??_7type_info@@6B@", ptr null, [3 x i8] c".H\00" }, comdat
+@"??_R0J@8" = linkonce_odr global %rtti.TypeDescriptor2 { ptr @"??_7type_info@@6B@", ptr null, [3 x i8] c".J\00" }, comdat
+
+define dso_local i32 @calls_boom() personality ptr @__CxxFrameHandler3 {
+entry:
+  %retval = alloca i32, align 4
+; MODIFICATION: Remove unusued alloca
+;  %l = alloca ptr, align 8
+  %i = alloca ptr, align 8
+  invoke void @boom()
+          to label %invoke.cont unwind label %catch.dispatch
+
+catch.dispatch:
+  %0 = catchswitch within none [label %catch1, label %catch] unwind to caller
+
+catch1:
+  %1 = catchpad within %0 [ptr @"??_R0H@8", i32 8, ptr %i]
+  %2 = load ptr, ptr %i, align 8
+  %3 = load i32, ptr %2, align 4
+  store i32 %3, ptr %retval, align 4
+  catchret from %1 to label %catchret.dest2
+
+catch:
+; MODIFICATION: Use %i instead of %l
+  %4 = catchpad within %0 [ptr @"??_R0J@8", i32 8, ptr %i]
+  %5 = load ptr, ptr %i, align 8
+  %6 = load i32, ptr %5, align 4
+  store i32 %6, ptr %retval, align 4
+  catchret from %4 to label %catchret.dest
+
+invoke.cont:
+  br label %try.cont
+
+catchret.dest:
+  br label %return
+
+catchret.dest2:
+  br label %return
+
+try.cont:
+  store i32 0, ptr %retval, align 4
+  br label %return
+
+return:
+  %7 = load i32, ptr %retval, align 4
+  ret i32 %7
+}
+
+declare dso_local void @boom() #1
+
+declare dso_local i32 @__CxxFrameHandler3(...)

[dpaoliello]

FYI, my plan is to unify the duplicated tests with (or after) #147860

[efriedma-quic]

LGTM