Skip to content

Conversation

Nadharm
Copy link
Contributor

@Nadharm Nadharm commented Oct 6, 2025

This change resolves a stack usage issue seen in the TableGen'd function setTargetRuntimeLibcallSets when compiled with MSVC. This change reduces the frame size from 47720 bytes to 48 bytes.

@llvmbot
Copy link
Member

llvmbot commented Oct 6, 2025

@llvm/pr-subscribers-tablegen

Author: None (Nadharm)

Changes

This change resolves a stack usage issue seen in the TableGen'd function setTargetRuntimeLibcallSets when compiled with MSVC. This change reduces the frame size from 47720 bytes to 48 bytes.


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

4 Files Affected:

  • (modified) llvm/test/TableGen/RuntimeLibcallEmitter-calling-conv.td (+15-23)
  • (modified) llvm/test/TableGen/RuntimeLibcallEmitter-conflict-warning.td (+6-12)
  • (modified) llvm/test/TableGen/RuntimeLibcallEmitter.td (+16-44)
  • (modified) llvm/utils/TableGen/Basic/RuntimeLibcallsEmitter.cpp (+15-25)
diff --git a/llvm/test/TableGen/RuntimeLibcallEmitter-calling-conv.td b/llvm/test/TableGen/RuntimeLibcallEmitter-calling-conv.td
index c224cd60412a8..7ec70b740c01c 100644
--- a/llvm/test/TableGen/RuntimeLibcallEmitter-calling-conv.td
+++ b/llvm/test/TableGen/RuntimeLibcallEmitter-calling-conv.td
@@ -48,47 +48,39 @@ def MSP430LibraryWithCondCC : SystemRuntimeLibrary<isMSP430,
 // CHECK-NEXT:     Entry = DefaultCC;
 // CHECK-NEXT:   }
 // CHECK-EMPTY:
-// CHECK-NEXT:    setLibcallsImpl({
-// CHECK-NEXT:      {RTLIB::MALLOC, RTLIB::impl_malloc}, // malloc
-// CHECK-NEXT:    });
+// CHECK-NEXT:    setLibcallImpl(RTLIB::MALLOC, RTLIB::impl_malloc); // malloc
 // CHECK-EMPTY:
-// CHECK-NEXT:    setLibcallsImpl({
-// CHECK-NEXT:        {RTLIB::SDIVREM_I8, RTLIB::impl___divmodqi4}, // __divmodqi4
-// CHECK-NEXT:        {RTLIB::UDIVREM_I16, RTLIB::impl___udivmodhi4}, // __udivmodhi4
-// CHECK-NEXT:    }, CallingConv::AVR_BUILTIN);
+// CHECK-NEXT:    setLibcallImpl(RTLIB::SDIVREM_I8, RTLIB::impl___divmodqi4); // __divmodqi4
+// CHECK-NEXT:    setLibcallImplCallingConv(RTLIB::impl___divmodqi4, CallingConv::AVR_BUILTIN);
+// CHECK-NEXT:    setLibcallImpl(RTLIB::UDIVREM_I16, RTLIB::impl___udivmodhi4); // __udivmodhi4
+// CHECK-NEXT:    setLibcallImplCallingConv(RTLIB::impl___udivmodhi4, CallingConv::AVR_BUILTIN);
 // CHECK-EMPTY:
 // CHECK-NEXT:    return;
 // CHECK-NEXT:  }
 // CHECK-EMPTY:
 // CHECK-NEXT: if (TT.getArch() == Triple::avr) {
-// CHECK-NEXT:   setLibcallsImpl({
-// CHECK-NEXT:       {RTLIB::MALLOC, RTLIB::impl_malloc}, // malloc
-// CHECK-NEXT:   });
+// CHECK-NEXT:   setLibcallImpl(RTLIB::MALLOC, RTLIB::impl_malloc); // malloc
 // CHECK-EMPTY:
-// CHECK-NEXT:   setLibcallsImpl({
-// CHECK-NEXT:       {RTLIB::SDIVREM_I8, RTLIB::impl___divmodqi4}, // __divmodqi4
-// CHECK-NEXT:       {RTLIB::UDIVREM_I16, RTLIB::impl___udivmodhi4}, // __udivmodhi4
-// CHECK-NEXT:   }, CallingConv::AVR_BUILTIN);
+// CHECK-NEXT:   setLibcallImpl(RTLIB::SDIVREM_I8, RTLIB::impl___divmodqi4); // __divmodqi4
+// CHECK-NEXT:   setLibcallImplCallingConv(RTLIB::impl___divmodqi4, CallingConv::AVR_BUILTIN);
+// CHECK-NEXT:   setLibcallImpl(RTLIB::UDIVREM_I16, RTLIB::impl___udivmodhi4); // __udivmodhi4
+// CHECK-NEXT:   setLibcallImplCallingConv(RTLIB::impl___udivmodhi4, CallingConv::AVR_BUILTIN);
 // CHECK-EMPTY:
 // CHECK-NEXT:   return;
 // CHECK-NEXT:  }
 // CHECK-EMPTY:
 // CHECK-NEXT:  if (TT.getArch() == Triple::msp430) {
-// CHECK-NEXT:    setLibcallsImpl({
-// CHECK-NEXT:        {RTLIB::MALLOC, RTLIB::impl_malloc}, // malloc
-// CHECK-NEXT:    });
+// CHECK-NEXT:    setLibcallImpl(RTLIB::MALLOC, RTLIB::impl_malloc); // malloc
 // CHECK-EMPTY:
 // CHECK-NEXT:    if ( isFoo() ) {
-// CHECK-NEXT:      setLibcallsImpl({
-// CHECK-NEXT:        {RTLIB::SDIVREM_I8, RTLIB::impl___divmodqi4}, // __divmodqi4
-// CHECK-NEXT:      }, CallingConv::AVR_BUILTIN);
+// CHECK-NEXT:      setLibcallImpl(RTLIB::SDIVREM_I8, RTLIB::impl___divmodqi4); // __divmodqi4
+// CHECK-NEXT:      setLibcallImplCallingConv(RTLIB::impl___divmodqi4, CallingConv::AVR_BUILTIN);
 // CHECK-EMPTY:
 // CHECK-NEXT:    }
 // CHECK-EMPTY:
 // CHECK-NEXT:    if ( isBar() ) {
-// CHECK-NEXT:      setLibcallsImpl({
-// CHECK-NEXT:          {RTLIB::UDIVREM_I16, RTLIB::impl___udivmodhi4}, // __udivmodhi4
-// CHECK-NEXT:      }, CallingConv::MSP430_BUILTIN);
+// CHECK-NEXT:      setLibcallImpl(RTLIB::UDIVREM_I16, RTLIB::impl___udivmodhi4); // __udivmodhi4
+// CHECK-NEXT:      setLibcallImplCallingConv(RTLIB::impl___udivmodhi4, CallingConv::MSP430_BUILTIN);
 // CHECK-EMPTY:
 // CHECK-NEXT:    }
 // CHECK-EMPTY:
diff --git a/llvm/test/TableGen/RuntimeLibcallEmitter-conflict-warning.td b/llvm/test/TableGen/RuntimeLibcallEmitter-conflict-warning.td
index 8169f5691cf0c..112c33e3e3c47 100644
--- a/llvm/test/TableGen/RuntimeLibcallEmitter-conflict-warning.td
+++ b/llvm/test/TableGen/RuntimeLibcallEmitter-conflict-warning.td
@@ -25,9 +25,7 @@ def dup1 : RuntimeLibcallImpl<ANOTHER_DUP>;
 // func_a and func_b both provide SOME_FUNC.
 
 // CHECK: if (isTargetArchA()) {
-// CHECK-NEXT: setLibcallsImpl({
-// CHECK-NEXT:   {RTLIB::SOME_FUNC, RTLIB::impl_func_b}, // func_b
-// CHECK-NEXT: });
+// CHECK-NEXT: setLibcallImpl(RTLIB::SOME_FUNC, RTLIB::impl_func_b); // func_b
 
 // ERR: :[[@LINE+1]]:5: warning: conflicting implementations for libcall SOME_FUNC: func_b, func_a
 def TheSystemLibraryA : SystemRuntimeLibrary<isTargetArchA,
@@ -35,10 +33,8 @@ def TheSystemLibraryA : SystemRuntimeLibrary<isTargetArchA,
 >;
 
 // CHECK: if (isTargetArchB()) {
-// CHECK-NEXT: setLibcallsImpl({
-// CHECK-NEXT:   {RTLIB::OTHER_FUNC, RTLIB::impl_other_func}, // other_func
-// CHECK-NEXT:   {RTLIB::SOME_FUNC, RTLIB::impl_func_a}, // func_a
-// CHECK-NEXT: });
+// CHECK-NEXT: setLibcallImpl(RTLIB::OTHER_FUNC, RTLIB::impl_other_func); // other_func
+// CHECK-NEXT: setLibcallImpl(RTLIB::SOME_FUNC, RTLIB::impl_func_a); // func_a
 
 // ERR: :[[@LINE+1]]:5: warning: conflicting implementations for libcall SOME_FUNC: func_a, func_b
 def TheSystemLibraryB : SystemRuntimeLibrary<isTargetArchB,
@@ -46,11 +42,9 @@ def TheSystemLibraryB : SystemRuntimeLibrary<isTargetArchB,
 >;
 
 // CHECK: if (isTargetArchC()) {
-// CHECK-NEXT: setLibcallsImpl({
-// CHECK-NEXT:   {RTLIB::ANOTHER_DUP, RTLIB::impl_dup1}, // dup1
-// CHECK-NEXT:   {RTLIB::OTHER_FUNC, RTLIB::impl_other_func}, // other_func
-// CHECK-NEXT:   {RTLIB::SOME_FUNC, RTLIB::impl_func_a}, // func_a
-// CHECK-NEXT: });
+// CHECK-NEXT: setLibcallImpl(RTLIB::ANOTHER_DUP, RTLIB::impl_dup1); // dup1
+// CHECK-NEXT: setLibcallImpl(RTLIB::OTHER_FUNC, RTLIB::impl_other_func); // other_func
+// CHECK-NEXT: setLibcallImpl(RTLIB::SOME_FUNC, RTLIB::impl_func_a); // func_a
 
 // ERR: :[[@LINE+3]]:5: warning: conflicting implementations for libcall ANOTHER_DUP: dup1, dup0
 // ERR: :[[@LINE+2]]:5: warning: conflicting implementations for libcall SOME_FUNC: func_a, func_b
diff --git a/llvm/test/TableGen/RuntimeLibcallEmitter.td b/llvm/test/TableGen/RuntimeLibcallEmitter.td
index 78705e25dc5dc..f4577f89ed618 100644
--- a/llvm/test/TableGen/RuntimeLibcallEmitter.td
+++ b/llvm/test/TableGen/RuntimeLibcallEmitter.td
@@ -190,40 +190,20 @@ def BlahLibrary : SystemRuntimeLibrary<isBlahArch, (add calloc, LibraryWithCondi
 // CHECK-NEXT: }
 
 // CHECK: void llvm::RTLIB::RuntimeLibcallsInfo::setTargetRuntimeLibcallSets(const llvm::Triple &TT, ExceptionHandling ExceptionModel, FloatABI::ABIType FloatABI, EABI EABIVersion, StringRef ABIName) {
-// CHECK-NEXT:  struct LibcallImplPair {
-// CHECK-NEXT:    RTLIB::Libcall Func;
-// CHECK-NEXT:    RTLIB::LibcallImpl Impl;
-// CHECK-NEXT:  };
-// CHECK-NEXT:  auto setLibcallsImpl = [this](
-// CHECK-NEXT:    ArrayRef<LibcallImplPair> Libcalls,
-// CHECK-NEXT:    std::optional<llvm::CallingConv::ID> CC = {})
-// CHECK-NEXT:  {
-// CHECK-NEXT:    for (const auto [Func, Impl] : Libcalls) {
-// CHECK-NEXT:      setLibcallImpl(Func, Impl);
-// CHECK-NEXT:      if (CC)
-// CHECK-NEXT:        setLibcallImplCallingConv(Impl, *CC);
-// CHECK-NEXT:    }
-// CHECK-NEXT:  };
 // CHECK-EMPTY:
 // CHECK-NEXT: if (TT.getArch() == Triple::blah) {
-// CHECK-NEXT:     setLibcallsImpl({
-// CHECK-NEXT:         {RTLIB::BZERO, RTLIB::impl_bzero}, // bzero
-// CHECK-NEXT:         {RTLIB::CALLOC, RTLIB::impl_calloc}, // calloc
-// CHECK-NEXT:         {RTLIB::SQRT_F128, RTLIB::impl_sqrtl_f128}, // sqrtl
-// CHECK-NEXT:     });
+// CHECK-NEXT:    setLibcallImpl(RTLIB::BZERO, RTLIB::impl_bzero); // bzero
+// CHECK-NEXT:    setLibcallImpl(RTLIB::CALLOC, RTLIB::impl_calloc); // calloc
+// CHECK-NEXT:    setLibcallImpl(RTLIB::SQRT_F128, RTLIB::impl_sqrtl_f128); // sqrtl
 // CHECK-EMPTY:
 // CHECK-NEXT:    if (TT.hasCompilerRT()) {
-// CHECK-NEXT:      setLibcallsImpl({
-// CHECK-NEXT:          {RTLIB::SHL_I32, RTLIB::impl___ashlsi3}, // __ashlsi3
-// CHECK-NEXT:          {RTLIB::SRL_I64, RTLIB::impl___lshrdi3}, // __lshrdi3
-// CHECK-NEXT:      });
+// CHECK-NEXT:      setLibcallImpl(RTLIB::SHL_I32, RTLIB::impl___ashlsi3); // __ashlsi3
+// CHECK-NEXT:      setLibcallImpl(RTLIB::SRL_I64, RTLIB::impl___lshrdi3); // __lshrdi3
 // CHECK-EMPTY:
 // CHECK-NEXT:    }
 // CHECK-EMPTY:
 // CHECK-NEXT:    if (TT.getOS() == Triple::bar) {
-// CHECK-NEXT:      setLibcallsImpl({
-// CHECK-NEXT:          {RTLIB::MEMSET, RTLIB::impl____memset}, // ___memset
-// CHECK-NEXT:      });
+// CHECK-NEXT:      setLibcallImpl(RTLIB::MEMSET, RTLIB::impl____memset); // ___memset
 // CHECK-EMPTY:
 // CHECK-NEXT:    }
 // CHECK-EMPTY:
@@ -231,25 +211,19 @@ def BlahLibrary : SystemRuntimeLibrary<isBlahArch, (add calloc, LibraryWithCondi
 // CHECK-NEXT: }
 // CHECK-EMPTY:
 // CHECK-NEXT: if (TT.getArch() == Triple::buzz) {
-// CHECK-NEXT:    setLibcallsImpl({
-// CHECK-NEXT:        {RTLIB::SHL_I32, RTLIB::impl___ashlsi3}, // __ashlsi3
-// CHECK-NEXT:        {RTLIB::SQRT_F80, RTLIB::impl_sqrtl_f80}, // sqrtl
-// CHECK-NEXT:        {RTLIB::SRL_I64, RTLIB::impl___lshrdi3}, // __lshrdi3
-// CHECK-NEXT:    });
+// CHECK-NEXT:    setLibcallImpl(RTLIB::SHL_I32, RTLIB::impl___ashlsi3); // __ashlsi3
+// CHECK-NEXT:    setLibcallImpl(RTLIB::SQRT_F80, RTLIB::impl_sqrtl_f80); // sqrtl
+// CHECK-NEXT:    setLibcallImpl(RTLIB::SRL_I64, RTLIB::impl___lshrdi3); // __lshrdi3
 // CHECK-EMPTY:
 // CHECK-NEXT:   return;
 // CHECK-NEXT: }
 // CHECK-EMPTY:
 // CHECK-NEXT: if (TT.getArch() == Triple::foo) {
-// CHECK-NEXT:    setLibcallsImpl({
-// CHECK-NEXT:        {RTLIB::BZERO, RTLIB::impl_bzero}, // bzero
-// CHECK-NEXT:        {RTLIB::SQRT_F128, RTLIB::impl_sqrtl_f128}, // sqrtl
-// CHECK-NEXT:    });
+// CHECK-NEXT:    setLibcallImpl(RTLIB::BZERO, RTLIB::impl_bzero); // bzero
+// CHECK-NEXT:    setLibcallImpl(RTLIB::SQRT_F128, RTLIB::impl_sqrtl_f128); // sqrtl
 // CHECK-EMPTY:
 // CHECK-NEXT:    if (TT.getOS() == Triple::bar) {
-// CHECK-NEXT:      setLibcallsImpl({
-// CHECK-NEXT:          {RTLIB::MEMSET, RTLIB::impl____memset}, // ___memset
-// CHECK-NEXT:      });
+// CHECK-NEXT:      setLibcallImpl(RTLIB::MEMSET, RTLIB::impl____memset); // ___memset
 // CHECK-EMPTY:
 // CHECK-NEXT:    }
 // CHECK-EMPTY:
@@ -257,12 +231,10 @@ def BlahLibrary : SystemRuntimeLibrary<isBlahArch, (add calloc, LibraryWithCondi
 // CHECK-NEXT:  }
 // CHECK-EMPTY:
 // CHECK-NEXT: if (TT.getArch() == Triple::simple) {
-// CHECK-NEXT:    setLibcallsImpl({
-// CHECK-NEXT:        {RTLIB::CALLOC, RTLIB::impl_calloc}, // calloc
-// CHECK-NEXT:        {RTLIB::SHL_I32, RTLIB::impl___ashlsi3}, // __ashlsi3
-// CHECK-NEXT:        {RTLIB::SQRT_F80, RTLIB::impl_sqrtl_f80}, // sqrtl
-// CHECK-NEXT:        {RTLIB::SRL_I64, RTLIB::impl___lshrdi3}, // __lshrdi3
-// CHECK-NEXT:    });
+// CHECK-NEXT:    setLibcallImpl(RTLIB::CALLOC, RTLIB::impl_calloc); // calloc
+// CHECK-NEXT:    setLibcallImpl(RTLIB::SHL_I32, RTLIB::impl___ashlsi3); // __ashlsi3
+// CHECK-NEXT:    setLibcallImpl(RTLIB::SQRT_F80, RTLIB::impl_sqrtl_f80); // sqrtl
+// CHECK-NEXT:    setLibcallImpl(RTLIB::SRL_I64, RTLIB::impl___lshrdi3); // __lshrdi3
 // CHECK-EMPTY:
 // CHECK-NEXT:   return;
 // CHECK-NEXT: }
diff --git a/llvm/utils/TableGen/Basic/RuntimeLibcallsEmitter.cpp b/llvm/utils/TableGen/Basic/RuntimeLibcallsEmitter.cpp
index 45cb2099f246b..c96331c3bb77d 100644
--- a/llvm/utils/TableGen/Basic/RuntimeLibcallsEmitter.cpp
+++ b/llvm/utils/TableGen/Basic/RuntimeLibcallsEmitter.cpp
@@ -543,21 +543,8 @@ void RuntimeLibcallEmitter::emitSystemRuntimeLibrarySetCalls(
   OS << "void llvm::RTLIB::RuntimeLibcallsInfo::setTargetRuntimeLibcallSets("
         "const llvm::Triple &TT, ExceptionHandling ExceptionModel, "
         "FloatABI::ABIType FloatABI, EABI EABIVersion, "
-        "StringRef ABIName) {\n"
-        "  struct LibcallImplPair {\n"
-        "    RTLIB::Libcall Func;\n"
-        "    RTLIB::LibcallImpl Impl;\n"
-        "  };\n"
-        "  auto setLibcallsImpl = [this](\n"
-        "    ArrayRef<LibcallImplPair> Libcalls,\n"
-        "    std::optional<llvm::CallingConv::ID> CC = {})\n"
-        "  {\n"
-        "    for (const auto [Func, Impl] : Libcalls) {\n"
-        "      setLibcallImpl(Func, Impl);\n"
-        "      if (CC)\n"
-        "        setLibcallImplCallingConv(Impl, *CC);\n"
-        "    }\n"
-        "  };\n";
+        "StringRef ABIName) {\n";
+
   ArrayRef<const Record *> AllLibs =
       Records.getAllDerivedDefinitions("SystemRuntimeLibrary");
 
@@ -682,18 +669,21 @@ void RuntimeLibcallEmitter::emitSystemRuntimeLibrarySetCalls(
 
       Funcs.erase(UniqueI, Funcs.end());
 
-      OS << indent(IndentDepth + 2) << "setLibcallsImpl({\n";
+      StringRef CCEnum;
+      if (FuncsWithCC.CallingConv)
+        CCEnum = FuncsWithCC.CallingConv->getValueAsString("CallingConv");
+
       for (const RuntimeLibcallImpl *LibCallImpl : Funcs) {
-        OS << indent(IndentDepth + 4);
-        LibCallImpl->emitTableEntry(OS);
-      }
-      OS << indent(IndentDepth + 2) << "}";
-      if (FuncsWithCC.CallingConv) {
-        StringRef CCEnum =
-            FuncsWithCC.CallingConv->getValueAsString("CallingConv");
-        OS << ", " << CCEnum;
+        OS << indent(IndentDepth + 2);
+        LibCallImpl->emitSetImplCall(OS);
+
+        if (FuncsWithCC.CallingConv) {
+          OS << indent(IndentDepth + 2) << "setLibcallImplCallingConv(";
+          LibCallImpl->emitEnumEntry(OS);
+          OS << ", " << CCEnum << ");\n";
+        }
       }
-      OS << ");\n\n";
+      OS << '\n';
 
       if (!SubsetPredicate.isAlwaysAvailable()) {
         OS << indent(IndentDepth);

@Nadharm
Copy link
Contributor Author

Nadharm commented Oct 6, 2025

@jurahul @arsenm , could you review please?

@jurahul jurahul self-requested a review October 7, 2025 00:11
@arsenm arsenm merged commit f490dbd into llvm:main Oct 7, 2025
11 checks passed
@arsenm
Copy link
Contributor

arsenm commented Oct 7, 2025

Actually we should continue to use a range loop over the libcall entry functions. Fully unrolling this is much larger code. That is, we should roll back this, and 9c361cc and do what it originally did

@jurahul
Copy link
Contributor

jurahul commented Oct 7, 2025

Or can we emit a single static table for all entries and use appropriate ArrayRef slice? My understanding is that the stack regression was due to in place lists. This will be similar to the changes to intrinsic emitter I did a few weeks back, where we made it table driven.

@arsenm
Copy link
Contributor

arsenm commented Oct 7, 2025

Or can we emit a single static table for all entries and use appropriate ArrayRef slice?

That would require making a much larger table. We don't need entries for the calling convention or predicate in most cases

@arsenm
Copy link
Contributor

arsenm commented Oct 7, 2025

Plus I really do not want to generate more conflicts with #150869 and the other patches I have to post on top of that. This is still bootstrapping to a different system

@arsenm
Copy link
Contributor

arsenm commented Oct 7, 2025

#162221 goes back to the old system

@Nadharm
Copy link
Contributor Author

Nadharm commented Oct 7, 2025

Actually we should continue to use a range loop over the libcall entry functions. Fully unrolling this is much larger code. That is, we should roll back this, and 9c361cc and do what it originally did

Ah fair, yeah. As long as 9c361cc is also reverted and we go back to the static array implementation, this would also resolve the stack usage regression.

My understanding is that the stack regression was due to in place lists

Yeah I think this is another case of MSVC not optimizing initializer lists, but I don't think that should be an issue in the pre-9c361cc static array implementation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants