Skip to content

Conversation

elvinw-intel
Copy link
Contributor

@elvinw-intel elvinw-intel commented Sep 10, 2025

Make IntrinsicsToAttributesMap's func. and arg. fields be able to have
adaptive sizes based on input other than hardcodded 8bits/8bits.
This will ease the pressure for adding new intrinsics in private
downstreams.

func. attr bitsize will become 7(127/128) vs 8(255/256)

@llvmbot
Copy link
Member

llvmbot commented Sep 10, 2025

@llvm/pr-subscribers-tablegen

@llvm/pr-subscribers-llvm-ir

Author: Elvin Wang (elvinw-intel)

Changes

Make IntrinsicsToAttributesMap's func. and arg. fields be able to have different sizes project-wise other than hardcoded 8bits/8bits. This will ease the pressure for adding new intrinsics in private downstreams.

Default setting will still be 8b/8b and this won't introduce functional difference in llvm trunk.

(do I need to document the 2 new cmake options?


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

4 Files Affected:

  • (modified) llvm/lib/IR/Intrinsics.cpp (+1-1)
  • (modified) llvm/test/TableGen/intrinsic-attrs.td (+3-2)
  • (modified) llvm/utils/TableGen/Basic/CMakeLists.txt (+9)
  • (modified) llvm/utils/TableGen/Basic/IntrinsicEmitter.cpp (+14-11)
diff --git a/llvm/lib/IR/Intrinsics.cpp b/llvm/lib/IR/Intrinsics.cpp
index 58a1f745a7122..893a856fb130a 100644
--- a/llvm/lib/IR/Intrinsics.cpp
+++ b/llvm/lib/IR/Intrinsics.cpp
@@ -744,7 +744,7 @@ AttributeSet Intrinsic::getFnAttributes(LLVMContext &C, ID id) {
   if (id == 0)
     return AttributeSet();
   uint16_t PackedID = IntrinsicsToAttributesMap[id - 1];
-  uint8_t FnAttrID = PackedID >> 8;
+  uint16_t FnAttrID = PackedID >> FuncAttrBitSize;
   return getIntrinsicFnAttributeSet(C, FnAttrID);
 }
 
diff --git a/llvm/test/TableGen/intrinsic-attrs.td b/llvm/test/TableGen/intrinsic-attrs.td
index bcded0cd2e9f1..8954edfd88ff2 100644
--- a/llvm/test/TableGen/intrinsic-attrs.td
+++ b/llvm/test/TableGen/intrinsic-attrs.td
@@ -25,9 +25,10 @@ def int_deref_ptr_ret : Intrinsic<[llvm_ptr_ty], [], [Dereferenceable<RetIndex,
 // CHECK-NEXT: });
 
 // CHECK: static constexpr uint16_t IntrinsicsToAttributesMap[] = {
-// CHECK: 0 << 8 | 0, // llvm.deref.ptr.ret
-// CHECK: 1 << 8 | 1, // llvm.random.gen
+// CHECK: 0 << [[ARG_ATTR_BITSIZE:[0-9]+]] | 0, // llvm.deref.ptr.ret
+// CHECK: 1 << [[ARG_ATTR_BITSIZE]] | 1, // llvm.random.gen
 // CHECK: }; // IntrinsicsToAttributesMap
+// CHECK-NEXT: static constexpr uint8_t FuncAttrBitSize = [[ARG_ATTR_BITSIZE]];
 
 // CHECK: static constexpr ArgNoAttrIDPair ArgAttrIdTable[] = {
 // CHECK-NEXT:   {0, 0},
diff --git a/llvm/utils/TableGen/Basic/CMakeLists.txt b/llvm/utils/TableGen/Basic/CMakeLists.txt
index b4a66ecce6440..213216b34efa1 100644
--- a/llvm/utils/TableGen/Basic/CMakeLists.txt
+++ b/llvm/utils/TableGen/Basic/CMakeLists.txt
@@ -8,6 +8,15 @@ set(LLVM_LINK_COMPONENTS
   TableGen
   )
 
+set(FUNC_ATTR_BIT_SIZE "8" CACHE STRING "Number of bits for function attributes")
+set(ARG_ATTR_BIT_SIZE "8" CACHE STRING "Number of bits for argument attributes")
+# Total bits should not exceed 16
+math(EXPR TOTAL_ATTR_BITS "${FUNC_ATTR_BIT_SIZE} + ${ARG_ATTR_BIT_SIZE}")
+if(TOTAL_ATTR_BITS GREATER 16)
+  message(FATAL_ERROR "Too many bits for IntrinsicsToAttributesMap")
+endif()
+add_compile_definitions(FUNC_ATTR_BIT_SIZE=${FUNC_ATTR_BIT_SIZE})
+add_compile_definitions(ARG_ATTR_BIT_SIZE=${ARG_ATTR_BIT_SIZE})
 add_llvm_library(LLVMTableGenBasic OBJECT EXCLUDE_FROM_ALL DISABLE_LLVM_LINK_LLVM_DYLIB
   ARMTargetDefEmitter.cpp
   Attributes.cpp
diff --git a/llvm/utils/TableGen/Basic/IntrinsicEmitter.cpp b/llvm/utils/TableGen/Basic/IntrinsicEmitter.cpp
index 09d29b8522f54..03a7853084e8e 100644
--- a/llvm/utils/TableGen/Basic/IntrinsicEmitter.cpp
+++ b/llvm/utils/TableGen/Basic/IntrinsicEmitter.cpp
@@ -629,27 +629,29 @@ static constexpr uint16_t IntrinsicsToAttributesMap[] = {)";
     UniqAttributes.try_emplace(&Int, ID);
   }
 
-  constexpr uint16_t NoFunctionAttrsID = 255;
-  if (UniqAttributes.size() > 256)
+  constexpr uint16_t NoFunctionAttrsID = (1 << FUNC_ATTR_BIT_SIZE) - 1;
+  if (UniqAttributes.size() > (1 << ARG_ATTR_BIT_SIZE))
     PrintFatalError("Too many unique argument attributes for table!");
-  // Note, ID 255 is used to indicate no function attributes.
-  if (UniqFnAttributes.size() > 255)
+  // Note, ID -1 is used to indicate no function attributes.
+  if (UniqFnAttributes.size() > (1 << FUNC_ATTR_BIT_SIZE) - 1)
     PrintFatalError("Too many unique function attributes for table!");
 
-  // Assign a 16-bit packed ID for each intrinsic. The lower 8-bits will be its
-  // "argument attribute ID" (index in UniqAttributes) and upper 8 bits will be
+  // Assign a 16-bit packed ID for each intrinsic. The lower bits will be its
+  // "argument attribute ID" (index in UniqAttributes) and upper bits will be
   // its "function attribute ID" (index in UniqFnAttributes).
   for (const CodeGenIntrinsic &Int : Ints) {
     uint16_t FnAttrIndex =
         hasFnAttributes(Int) ? UniqFnAttributes[&Int] : NoFunctionAttrsID;
-    OS << formatv("\n    {} << 8 | {}, // {}", FnAttrIndex,
+    OS << formatv("\n    {} << {} | {}, // {}", FnAttrIndex,
+                  ARG_ATTR_BIT_SIZE,
                   UniqAttributes[&Int], Int.Name);
   }
 
   OS << R"(
 }; // IntrinsicsToAttributesMap
 )";
-
+  OS << formatv("static constexpr uint8_t FuncAttrBitSize = {};\n",
+                ARG_ATTR_BIT_SIZE);
   // For a given intrinsic, its attributes are constructed by populating the
   // local array `AS` below with its non-empty argument attributes followed by
   // function attributes if any. Each argument attribute is constructed as:
@@ -749,8 +751,8 @@ AttributeList Intrinsic::getAttributes(LLVMContext &C, ID id,
     return AttributeList();
 
   uint16_t PackedID = IntrinsicsToAttributesMap[id - 1];
-  uint8_t FnAttrID = PackedID >> 8;
-  uint8_t ArgAttrID = PackedID & 0xFF;
+  uint16_t FnAttrID = PackedID >> ({});
+  uint16_t ArgAttrID = PackedID & ({});
   using PairTy = std::pair<unsigned, AttributeSet>;
   alignas(PairTy) char ASStorage[sizeof(PairTy) * {}];
   PairTy *AS = reinterpret_cast<PairTy *>(ASStorage);
@@ -775,7 +777,8 @@ AttributeList Intrinsic::getAttributes(LLVMContext &C, ID id,
 #endif // GET_INTRINSIC_ATTRIBUTES
 
 )",
-                MaxNumAttrs, NoFunctionAttrsID);
+                ARG_ATTR_BIT_SIZE, (1 << ARG_ATTR_BIT_SIZE) - 1, MaxNumAttrs,
+                NoFunctionAttrsID);
 }
 
 void IntrinsicEmitter::EmitIntrinsicToBuiltinMap(

Copy link

github-actions bot commented Sep 10, 2025

✅ With the latest revision this PR passed the C/C++ code formatter.

Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR makes the IntrinsicsToAttributesMap bit allocation configurable through CMake options instead of using hardcoded 8-bit allocation for both function and argument attributes. This change allows downstream projects to customize bit allocation for handling more intrinsics when needed.

  • Introduces FUNC_ATTR_BIT_SIZE and ARG_ATTR_BIT_SIZE CMake variables with default 8-bit allocation
  • Updates the intrinsic attribute mapping logic to use dynamic bit sizes instead of hardcoded values
  • Adds validation to ensure total bit allocation doesn't exceed 16 bits

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File Description
llvm/utils/TableGen/Basic/CMakeLists.txt Adds CMake configuration for bit size settings with validation
llvm/utils/TableGen/Basic/IntrinsicEmitter.cpp Updates attribute mapping logic to use configurable bit sizes
llvm/lib/IR/Intrinsics.cpp Updates attribute extraction to use dynamic bit size
llvm/test/TableGen/intrinsic-attrs.td Updates test expectations for new bit size format

@jurahul
Copy link
Contributor

jurahul commented Sep 11, 2025

Instead of a build option, can we determine the packing automatically and use the right number of bits? And move any unpacking code into the TableGen generated code?

@elvinw-intel elvinw-intel changed the title [IntrinsicEmitter][NFC] Make AttributesMap bit-configurable [IntrinsicEmitter] Make AttributesMap bits adaptive Sep 11, 2025
Make IntrinsicsToAttributesMap's func. and arg. fields be able to have
adaptive sizes based on input other than hardcodded 8bits/8bits.
This will ease the pressure for adding new intrinsics in private
downstreams.
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

@elvinw-intel
Copy link
Contributor Author

Instead of a build option, can we determine the packing automatically and use the right number of bits? And move any unpacking code into the TableGen generated code?

made the bit packing adaptive and moved Intrinsic::getFnAttributes to emitter

Copy link
Contributor

@nikic nikic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@elvinw-intel
Copy link
Contributor Author

Can anyone help with the merge if approval(I don't have write permissions)? Thanks

@nikic nikic merged commit 6af94c5 into llvm:main Sep 12, 2025
9 checks passed
elvinw-intel added a commit to elvinw-intel/llvm-project that referenced this pull request Sep 12, 2025
Following up llvm#157965 which made this bit-adaptive, this patch attempts
to make this map able to choose type ranging from int8 to int64 based on
intrinsic inputs.
elvinw-intel added a commit to elvinw-intel/llvm-project that referenced this pull request Sep 12, 2025
Following up llvm#157965 which made this bit-adaptive, this patch attempts
to make this map able to choose type ranging from int8 to int64 based on
intrinsic inputs.
elvinw-intel added a commit to elvinw-intel/llvm-project that referenced this pull request Sep 12, 2025
Following up llvm#157965 which made this bit-adaptive, this patch attempts
to make this map able to choose type ranging from int8 to int64 based on
intrinsic inputs.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants