[LLVM][Intrinsics] Reduce stack size for Intrinsic::getAttributes

[jurahul]

This change fixes a stack size regression that got introduced in 0de0354. That change did 2 independent things:

1. Uniquify argument and function attributes separately so that we generate a smaller number of unique sets as opposed to uniquifying them together. This is beneficial for code size.
2. Eliminate the fixed size array AS and NumAttrs variable and instead build the returned AttribteList in each case using an initializer list.

The second part seems to have caused a regression in the stack size usage of this function for Windows. This change essentially undoes part 2 and reinstates the use of the fixed size array AS which fixes this stack size regression. The actual measured stack frame size for this function before/after this change is as follows:

  Current trunk data for release build (x86_64 builds for Linux, x86 build for Windows):
  Compiler                              gcc-13.3.0      clang-18.1.3      MSVC 19.43.34810.0
  DLLVM_ENABLE_ASSERTIONS=OFF           0x120           0x110             0x54B0   
  DLLVM_ENABLE_ASSERTIONS=ON            0x2880          0x110             0x5250

  After applying the fix:
  Compiler                              gcc-13.3.0      clang-18.1.3      MSVC 19.43.34810.0
  DLLVM_ENABLE_ASSERTIONS=OFF           0x120           0x118             0x1240h                                               
  DLLVM_ENABLE_ASSERTIONS=ON            0x120           0x118             0x1240h  

Note that for Windows builds with assertions disabled, the stack frame size for this function reduces from 21680 to 4672 which is a 4.6x reduction. Stack frame size for GCC build with assertions also improved and clang builds are unimpacted. The speculation is that clang and gcc is able to reuse the stack space across these switch cases better with existing code, but MSVC is not, and re-introducing the AS variable forces all cases to use the same local variable, addressing the stack space regression.

[jurahul]

This PR addresses a stack size regression that we see in some graphics workloads internally at NVIDIA which seems to be particularly sensitive to stack size. Thanks @Nadharm for triaging this.

[llvmbot]

@llvm/pr-subscribers-tablegen

Author: Rahul Joshi (jurahul)

Changes

This change fixes a stack size regression that got introduced in 0de0354. That change did 2 independent things:

1. Uniquify argument and function attributes separately so that we generate a smaller number of unique sets as opposed to uniquifying them together. This is beneficial for code size.
2. Eliminate the fixed size array AS and NumAttrs variable and instead build the returned AttribteList in each case using an initializer list.

The second part seems to have caused a regression in the stack size usage of this function for Windows. This change essentially undoes part 2 and reinstates the use of the fixed size array AS which fixes this stack size regression. The actual measured stack frame size for this function before/after this change is as follows:

  Current trunk data for release build (x86_64 builds for Linux, x86 build for Windows):
  Compiler                              gcc-13.3.0      clang-18.1.3      MSVC 19.43.34810.0
  DLLVM_ENABLE_ASSERTIONS=OFF           0x120           0x110             0x54B0   
  DLLVM_ENABLE_ASSERTIONS=ON            0x2880          0x110             0x5250

  After applying the fix:
  Compiler                              gcc-13.3.0      clang-18.1.3      MSVC 19.43.34810.0
  DLLVM_ENABLE_ASSERTIONS=OFF           0x120           0x118             0x1240h                                               
  DLLVM_ENABLE_ASSERTIONS=ON            0x120           0x118             0x1240h  

Note that for Windows builds with assertions disabled, the stack frame size for this function reduces from 21680 to 4672 which is a 4.6x reduction. Stack frame size for GCC build with assertions also improved and clang builds are unimpacted. The speculation is that clang and gcc is able to reuse the stack space across these switch cases better with existing code, but MSVC is not, and re-introducing the AS variable forces all cases to use the same local variable, addressing the stack space regression.

---

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

2 Files Affected:

• (modified) llvm/test/TableGen/intrinsic-attrs.td (+7-8)
• (modified) llvm/utils/TableGen/Basic/IntrinsicEmitter.cpp (+30-22)

diff --git a/llvm/test/TableGen/intrinsic-attrs.td b/llvm/test/TableGen/intrinsic-attrs.td
index 18309d7419994..92a90dcafd48c 100644
--- a/llvm/test/TableGen/intrinsic-attrs.td
+++ b/llvm/test/TableGen/intrinsic-attrs.td
@@ -30,12 +30,11 @@ def int_deref_ptr_ret : Intrinsic<[llvm_ptr_ty], [], [Dereferenceable<RetIndex,
 
 // CHECK: getAttributes(LLVMContext &C, ID id,
 // CHECK-NEXT: FunctionType *FT) {
-// CHECK: case 1:
-// CHECK-NEXT: return AttributeList::get(C, {
-// CHECK-NEXT:   {AttributeList::FunctionIndex, getIntrinsicFnAttributeSet(C, FnAttrID)}
-// CHECK-NEXT: });
+// CHECK:      case 1:
+// CHECK-NEXT:   HasFnAttr = true;
+// CHECK-NEXT:   break;
 // CHECK-NEXT: case 0:
-// CHECK-NEXT: return AttributeList::get(C, {
-// CHECK-NEXT:   {0, getIntrinsicArgAttributeSet(C, 0, FT->getContainedType(0))},
-// CHECK-NEXT:   {AttributeList::FunctionIndex, getIntrinsicFnAttributeSet(C, FnAttrID)}
-// CHECK-NEXT: });
+// CHECK-NEXT:   AS[0] = {0, getIntrinsicArgAttributeSet(C, 0, FT->getContainedType(0))};
+// CHECK-NEXT:   HasFnAttr = true;
+// CHECK-NEXT:   NumAttrs = 1
+// CHECK-NEXT:   break;
diff --git a/llvm/utils/TableGen/Basic/IntrinsicEmitter.cpp b/llvm/utils/TableGen/Basic/IntrinsicEmitter.cpp
index ac5c455ed63ce..49f719453e0bb 100644
--- a/llvm/utils/TableGen/Basic/IntrinsicEmitter.cpp
+++ b/llvm/utils/TableGen/Basic/IntrinsicEmitter.cpp
@@ -652,6 +652,16 @@ static constexpr uint16_t IntrinsicsToAttributesMap[] = {)";
 
 AttributeList Intrinsic::getAttributes(LLVMContext &C, ID id,
                                        FunctionType *FT) {)";
+  // Find the max number of attributes to create the local array.
+  unsigned MaxNumAttrs = 0;
+  for (const auto [IntPtr, UniqueID] : UniqAttributes) {
+    const CodeGenIntrinsic &Int = *IntPtr;
+    unsigned NumAttrs =
+        llvm::count_if(Int.ArgumentAttributes,
+                       [](const auto &Attrs) { return !Attrs.empty(); });
+    NumAttrs += hasFnAttributes(Int);
+    MaxNumAttrs = std::max(MaxNumAttrs, NumAttrs);
+  }
 
   OS << formatv(R"(
   if (id == 0)
@@ -659,46 +669,44 @@ AttributeList Intrinsic::getAttributes(LLVMContext &C, ID id,
 
   uint16_t PackedID = IntrinsicsToAttributesMap[id - 1];
   uint8_t FnAttrID = PackedID >> 8;
+  std::pair<unsigned, AttributeSet> AS[{}];
+  unsigned NumAttrs = 0;
+  bool HasFnAttr = false;
   switch(PackedID & 0xFF) {{
     default: llvm_unreachable("Invalid attribute number");
-)");
+)",
+                MaxNumAttrs);
 
   for (const auto [IntPtr, UniqueID] : UniqAttributes) {
     OS << formatv("  case {}:\n", UniqueID);
     const CodeGenIntrinsic &Int = *IntPtr;
 
-    // Keep track of the number of attributes we're writing out.
-    unsigned NumAttrs =
-        llvm::count_if(Int.ArgumentAttributes,
-                       [](const auto &Attrs) { return !Attrs.empty(); });
-    NumAttrs += hasFnAttributes(Int);
-    if (NumAttrs == 0) {
-      OS << "    return AttributeList();\n";
-      continue;
-    }
+    unsigned NumAttrs = 0;
 
-    OS << "    return AttributeList::get(C, {\n";
-    ListSeparator LS(",\n");
     for (const auto &[AttrIdx, Attrs] : enumerate(Int.ArgumentAttributes)) {
       if (Attrs.empty())
         continue;
 
       unsigned ArgAttrID = UniqArgAttributes.find(Attrs)->second;
-      OS << LS
-         << formatv("      {{{}, getIntrinsicArgAttributeSet(C, {}, "
-                    "FT->getContainedType({}))}",
-                    AttrIdx, ArgAttrID, AttrIdx);
+      OS << formatv("    AS[{}] = {{{}, getIntrinsicArgAttributeSet(C, {}, "
+                    "FT->getContainedType({}))};\n",
+                    NumAttrs++, AttrIdx, ArgAttrID, AttrIdx);
     }
 
-    if (hasFnAttributes(Int)) {
-      OS << LS
-         << "      {AttributeList::FunctionIndex, "
-            "getIntrinsicFnAttributeSet(C, FnAttrID)}";
-    }
-    OS << "\n    });\n";
+    if (hasFnAttributes(Int))
+      OS << "    HasFnAttr = true;\n";
+
+    if (NumAttrs)
+      OS << formatv("    NumAttrs = {};\n", NumAttrs);
+    OS << "    break;\n";
   }
 
   OS << R"(  }
+  if (HasFnAttr) {
+    AS[NumAttrs++] = {AttributeList::FunctionIndex,
+                      getIntrinsicFnAttributeSet(C, FnAttrID)};
+  }
+  return AttributeList::get(C, ArrayRef(AS, NumAttrs));
 }
 #endif // GET_INTRINSIC_ATTRIBUTES
 

[nikic]

LGTM