Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[TableGen] Refactor Intrinsics record #106986

Merged
merged 1 commit into from
Sep 4, 2024

Conversation

jurahul
Copy link
Contributor

@jurahul jurahul commented Sep 2, 2024

Eliminate unused isTarget field in Intrinsic record.

Eliminate isOverloaded, Types and TypeSig fields from the record, as they are already available through the TypeInfo field. Change intrinsic emitter code to look for this info using fields of the TypeInfo record attached to the Intrinsic record.

Fix several intrinsic related unit tests to source the Intrinsic class def from Intrinsics.td as opposed to defining a skeleton in the test.

This eliminates some duplication of information in the Intrinsic class, as well as reduces the memory allocated for record fields, resulting in ~2% reduction (though that's not the main goal).

@jurahul jurahul marked this pull request as ready for review September 2, 2024 18:25
@llvmbot llvmbot added the llvm:ir label Sep 2, 2024
@llvmbot
Copy link
Member

llvmbot commented Sep 2, 2024

@llvm/pr-subscribers-llvm-ir

Author: Rahul Joshi (jurahul)

Changes
  • Eliminate unused isTarget field in Intrinsic record.
  • Eliminate isOverloaded, Types and TypeSig fields from the record, as they are already available through the TypeInfo field. Change intrinsic emitter code to look for this info using fields of the TypeInfo record attached to the Intrinsic record.
  • Fix several intrinsic related unit tests to source the Intrinsic class def from Intrinsics.td as opposed to defining a skeleton in the test.
  • This eliminate some duplication of information in the Intrinsic class, as well as reduces the memory allocated for record fields, resulting in ~2% reduction (though that's not the main goal).

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

7 Files Affected:

  • (modified) llvm/include/llvm/IR/Intrinsics.td (-5)
  • (modified) llvm/test/TableGen/intrinsic-attrs.td (+2-50)
  • (modified) llvm/test/TableGen/intrinsic-long-name.td (+4-32)
  • (modified) llvm/test/TableGen/intrinsic-struct.td (+5-32)
  • (modified) llvm/test/TableGen/searchabletables-intrinsic.td (+4-33)
  • (modified) llvm/utils/TableGen/Basic/CodeGenIntrinsics.cpp (+10-9)
  • (modified) llvm/utils/TableGen/IntrinsicEmitter.cpp (+6-5)
diff --git a/llvm/include/llvm/IR/Intrinsics.td b/llvm/include/llvm/IR/Intrinsics.td
index 232d6be1073f49..1bc895eee60f1a 100644
--- a/llvm/include/llvm/IR/Intrinsics.td
+++ b/llvm/include/llvm/IR/Intrinsics.td
@@ -669,12 +669,7 @@ class Intrinsic<list<LLVMType> ret_types,
   // IntrinsicProperty<1>
   bit DisableDefaultAttributes = disable_default_attributes;
 
-  bit isTarget = false;
-
   TypeInfoGen TypeInfo = TypeInfoGen<RetTypes, ParamTypes>;
-  bit isOverloaded = TypeInfo.isOverloaded;
-  list<LLVMType> Types = TypeInfo.Types;
-  list<list<int>> TypeSig = TypeInfo.TypeSig;
 }
 
 // Intrinsic with default attributes (disable_default_attributes = false).
diff --git a/llvm/test/TableGen/intrinsic-attrs.td b/llvm/test/TableGen/intrinsic-attrs.td
index 29e8cb1e89bb01..3228b32405103e 100644
--- a/llvm/test/TableGen/intrinsic-attrs.td
+++ b/llvm/test/TableGen/intrinsic-attrs.td
@@ -1,54 +1,6 @@
-// RUN: llvm-tblgen -gen-intrinsic-impl -I %p/../../include %s | FileCheck %s
+// RUN: llvm-tblgen -gen-intrinsic-impl -I %p/../../include -DTEST_INTRINSICS_SUPPRESS_DEFS %s | FileCheck %s
 
-// Get the minimum blurb necessary to process ...
-include "llvm/CodeGen/ValueTypes.td"
-include "llvm/CodeGen/SDNodeProperties.td"
-
-class LLVMType<ValueType vt> {
-  ValueType VT = vt;
-  int isAny = 0;
-}
-
-def llvm_i32_ty     : LLVMType<i32>;
-def llvm_ptr_ty     : LLVMType<iPTR>;
-
-class AttrIndex<int idx> {
-  int Value = idx;
-}
-
-def FuncIndex : AttrIndex<-1>;
-def RetIndex : AttrIndex<0>;
-class ArgIndex<int argNo> : AttrIndex<!add(argNo, 1)>;
-
-class IntrinsicProperty<bit is_default = 0> {
-  bit IsDefault = is_default;
-}
-
-def IntrNoMem : IntrinsicProperty;
-def IntrHasSideEffects : IntrinsicProperty;
-class Dereferenceable<AttrIndex idx, int bytes> : IntrinsicProperty {
-  int ArgNo = idx.Value;
-  int Bytes = bytes;
-}
-
-class Intrinsic<list<LLVMType> ret_types,
-                list<LLVMType> param_types = [],
-                list<IntrinsicProperty> intr_properties = [],
-                string name = "",
-                list<SDNodeProperty> sd_properties = [],
-                bit disable_default_attributes = 0> : SDPatternOperator {
-  string LLVMName = name;
-  string TargetPrefix = "";
-  list<LLVMType> RetTypes = ret_types;
-  list<LLVMType> ParamTypes = param_types;
-  list<IntrinsicProperty> IntrProperties = intr_properties;
-  let Properties = sd_properties;
-  bit DisableDefaultAttributes = 1;
-
-
-  bit isTarget = 0;
-  bit DisableDefaultAttributes = disable_default_attributes;
-}
+include "llvm/IR/Intrinsics.td"
 
 // ... this intrinsic.
 def int_random_gen   : Intrinsic<[llvm_i32_ty], [], [IntrNoMem, IntrHasSideEffects]>;
diff --git a/llvm/test/TableGen/intrinsic-long-name.td b/llvm/test/TableGen/intrinsic-long-name.td
index d66173202302ba..c19910d474ed10 100644
--- a/llvm/test/TableGen/intrinsic-long-name.td
+++ b/llvm/test/TableGen/intrinsic-long-name.td
@@ -1,38 +1,10 @@
-// RUN: llvm-tblgen -gen-intrinsic-enums %s | FileCheck %s
+// RUN: llvm-tblgen -gen-intrinsic-enums -I %p/../../include %s -DTEST_INTRINSICS_SUPPRESS_DEFS | FileCheck %s
 // XFAIL: vg_leak
 
-class IntrinsicProperty<bit is_default = 0> {
-  bit IsDefault = is_default;
-}
-
-class SDNodeProperty;
-
-class ValueType<int size, int value> {
-  string Namespace = "MVT";
-  int Size = size;
-  int Value = value;
-}
-
-class LLVMType<ValueType vt> {
-  ValueType VT = vt;
-}
-
-class Intrinsic<string name, list<LLVMType> param_types = []> {
-  string LLVMName = name;
-  bit isTarget = 0;
-  string TargetPrefix = "";
-  list<LLVMType> RetTypes = [];
-  list<LLVMType> ParamTypes = param_types;
-  list<IntrinsicProperty> IntrProperties = [];
-  list<SDNodeProperty> Properties = [];
-  bit DisableDefaultAttributes = 1;
-}
-
-def iAny : ValueType<0, 253>;
-def llvm_anyint_ty : LLVMType<iAny>;
+include "llvm/IR/Intrinsics.td"
 
 // Make sure we generate the long name without crashing
 // CHECK: this_is_a_really_long_intrinsic_name_but_we_should_still_not_crash,  // llvm.this.is.a.really.long.intrinsic.name.but.we.should.still.not.crash
-def int_foo : Intrinsic<"llvm.foo", [llvm_anyint_ty]>;
-def int_this_is_a_really_long_intrinsic_name_but_we_should_still_not_crash : Intrinsic<"llvm.this.is.a.really.long.intrinsic.name.but.we.should.still.not.crash", [llvm_anyint_ty]>;
+def int_foo : Intrinsic<[llvm_anyint_ty], [], [], "llvm.foo">;
+def int_this_is_a_really_long_intrinsic_name_but_we_should_still_not_crash : Intrinsic<[llvm_anyint_ty], [], [], "llvm.this.is.a.really.long.intrinsic.name.but.we.should.still.not.crash">;
 
diff --git a/llvm/test/TableGen/intrinsic-struct.td b/llvm/test/TableGen/intrinsic-struct.td
index bc044a4a6f858e..f23a7a7643af27 100644
--- a/llvm/test/TableGen/intrinsic-struct.td
+++ b/llvm/test/TableGen/intrinsic-struct.td
@@ -1,38 +1,11 @@
-// RUN: llvm-tblgen -gen-intrinsic-enums %s | FileCheck %s
+// RUN: llvm-tblgen -gen-intrinsic-enums -I %p/../../include %s -DTEST_INTRINSICS_SUPPRESS_DEFS | FileCheck %s
 // XFAIL: vg_leak
 
-class IntrinsicProperty<bit is_default = 0> {
-  bit IsDefault = is_default;
-}
-
-class SDNodeProperty;
-
-class ValueType<int size, int value> {
-  string Namespace = "MVT";
-  int Size = size;
-  int Value = value;
-}
-
-class LLVMType<ValueType vt> {
-  ValueType VT = vt;
-}
-
-class Intrinsic<string name, list<LLVMType> ret_types = []> {
-  string LLVMName = name;
-  bit isTarget = 0;
-  string TargetPrefix = "";
-  list<LLVMType> RetTypes = ret_types;
-  list<LLVMType> ParamTypes = [];
-  list<IntrinsicProperty> IntrProperties = [];
-  list<SDNodeProperty> Properties = [];
-  bit DisableDefaultAttributes = 1;
-}
-
-def iAny : ValueType<0, 253>;
-def llvm_anyint_ty : LLVMType<iAny>;
+include "llvm/IR/Intrinsics.td"
 
 // Make sure we can return up to 8 values
 // CHECK: returns_8_results = {{[0-9]+}}, // llvm.returns.8.results
-def int_returns_8_results : Intrinsic<"llvm.returns.8.results",
+def int_returns_8_results : Intrinsic<
     [llvm_anyint_ty, llvm_anyint_ty, llvm_anyint_ty, llvm_anyint_ty,
-     llvm_anyint_ty, llvm_anyint_ty, llvm_anyint_ty, llvm_anyint_ty]>;
+     llvm_anyint_ty, llvm_anyint_ty, llvm_anyint_ty, llvm_anyint_ty],
+     [], [], "llvm.returns.8.results">;
diff --git a/llvm/test/TableGen/searchabletables-intrinsic.td b/llvm/test/TableGen/searchabletables-intrinsic.td
index 75722d19b16e99..d4ec105f0243b1 100644
--- a/llvm/test/TableGen/searchabletables-intrinsic.td
+++ b/llvm/test/TableGen/searchabletables-intrinsic.td
@@ -1,48 +1,19 @@
-// RUN: llvm-tblgen -gen-searchable-tables -I %p/../../include %s | FileCheck %s
+// RUN: llvm-tblgen -gen-searchable-tables -I %p/../../include -DTEST_INTRINSICS_SUPPRESS_DEFS %s | FileCheck %s
 // XFAIL: vg_leak
 
 include "llvm/TableGen/SearchableTable.td"
-
-class IntrinsicProperty<bit is_default = 0> {
-  bit IsDefault = is_default;
-}
-
-class SDNodeProperty;
-
-class ValueType<int size, int value> {
-  string Namespace = "MVT";
-  int Size = size;
-  int Value = value;
-}
-
-class LLVMType<ValueType vt> {
-  ValueType VT = vt;
-}
-
-class Intrinsic<list<LLVMType> param_types = []> {
-  string LLVMName = "";
-  bit isTarget = 0;
-  string TargetPrefix = "";
-  list<LLVMType> RetTypes = [];
-  list<LLVMType> ParamTypes = param_types;
-  list<IntrinsicProperty> IntrProperties = [];
-  list<SDNodeProperty> Properties = [];
-  bit DisableDefaultAttributes = 1;
-}
-
-def iAny : ValueType<0, 253>;
-def llvm_anyint_ty : LLVMType<iAny>;
+include "llvm/IR/Intrinsics.td"
 
 def int_abc : Intrinsic<[llvm_anyint_ty]>;
 def int_xyz : Intrinsic<[llvm_anyint_ty]>;
 
-let isTarget = 1, TargetPrefix = "gtarget" in {
+let TargetPrefix = "gtarget" in {
   def int_gtarget_def : Intrinsic<[llvm_anyint_ty]>;
   def int_gtarget_defg : Intrinsic<[llvm_anyint_ty]>;
   def int_gtarget_uvw : Intrinsic<[llvm_anyint_ty]>;
 }
 
-let isTarget = 1, TargetPrefix = "ftarget" in {
+let TargetPrefix = "ftarget" in {
   def int_ftarget_ghi : Intrinsic<[llvm_anyint_ty]>;
   def int_ftarget_ghi_x : Intrinsic<[llvm_anyint_ty]>;
   def int_ftarget_rst : Intrinsic<[llvm_anyint_ty]>;
diff --git a/llvm/utils/TableGen/Basic/CodeGenIntrinsics.cpp b/llvm/utils/TableGen/Basic/CodeGenIntrinsics.cpp
index 4bca904e9f38bd..3f48baed7ade6a 100644
--- a/llvm/utils/TableGen/Basic/CodeGenIntrinsics.cpp
+++ b/llvm/utils/TableGen/Basic/CodeGenIntrinsics.cpp
@@ -96,17 +96,18 @@ CodeGenIntrinsic::CodeGenIntrinsic(const Record *R,
                                   TargetPrefix + ".'!");
   }
 
-  if (auto *Types = R->getValue("Types")) {
-    auto *TypeList = cast<ListInit>(Types->getValue());
-    isOverloaded = R->getValueAsBit("isOverloaded");
+  const Record *TypeInfo = R->getValueAsDef("TypeInfo");
+  assert(TypeInfo->isSubClassOf("TypeInfoGen"));
 
-    unsigned I = 0;
-    for (unsigned E = R->getValueAsListInit("RetTypes")->size(); I < E; ++I)
-      IS.RetTys.push_back(TypeList->getElementAsRecord(I));
+  isOverloaded = TypeInfo->getValueAsBit("isOverloaded");
+  const ListInit *TypeList = TypeInfo->getValueAsListInit("Types");
 
-    for (unsigned E = TypeList->size(); I < E; ++I)
-      IS.ParamTys.push_back(TypeList->getElementAsRecord(I));
-  }
+  unsigned I = 0;
+  for (unsigned E = R->getValueAsListInit("RetTypes")->size(); I < E; ++I)
+    IS.RetTys.push_back(TypeList->getElementAsRecord(I));
+
+  for (unsigned E = TypeList->size(); I < E; ++I)
+    IS.ParamTys.push_back(TypeList->getElementAsRecord(I));
 
   // Parse the intrinsic properties.
   ListInit *PropList = R->getValueAsListInit("IntrProperties");
diff --git a/llvm/utils/TableGen/IntrinsicEmitter.cpp b/llvm/utils/TableGen/IntrinsicEmitter.cpp
index 70ccecf7752af7..f57cc94c8afbfc 100644
--- a/llvm/utils/TableGen/IntrinsicEmitter.cpp
+++ b/llvm/utils/TableGen/IntrinsicEmitter.cpp
@@ -273,11 +273,12 @@ using TypeSigTy = SmallVector<unsigned char>;
 /// Computes type signature of the intrinsic \p Int.
 static TypeSigTy ComputeTypeSignature(const CodeGenIntrinsic &Int) {
   TypeSigTy TypeSig;
-  if (const auto *R = Int.TheDef->getValue("TypeSig")) {
-    for (const auto *a : cast<ListInit>(R->getValue())->getValues()) {
-      for (const auto *b : cast<ListInit>(a)->getValues())
-        TypeSig.emplace_back(cast<IntInit>(b)->getValue());
-    }
+  const Record *TypeInfo = Int.TheDef->getValueAsDef("TypeInfo");
+  const ListInit *OuterList = TypeInfo->getValueAsListInit("TypeSig");
+
+  for (const auto *Outer : OuterList->getValues()) {
+    for (const auto *Inner : cast<ListInit>(Outer)->getValues())
+      TypeSig.emplace_back(cast<IntInit>(Inner)->getValue());
   }
   return TypeSig;
 }

- Eliminate unused `isTarget` field in Intrinsic record.
- Eliminate `isOverloaded`, `Types` and `TypeSig` fields from the record, as
  they are already available through the `TypeInfo` field. Change intrinsic
  emitter code to look for this info using fields of the `TypeInfo` record
  attached to the `Intrinsic` record.
- Fix several intrinsic related unit tests to source the `Intrinsic` class
  def from Intrinsics.td as opposed to defining a skeleton in the test.
- This eliminate some duplication of information in the Intrinsic class,
  as well as reduces the memory allocated for record fields, resulting in
  ~2% reduction (though that's not the main goal).
@jurahul jurahul force-pushed the cleanup_intrinsic_record branch from b41192d to 2fbf16e Compare September 4, 2024 13:14
@arsenm arsenm added the tablegen label Sep 4, 2024
@jurahul jurahul merged commit 98c6bbf into llvm:main Sep 4, 2024
9 checks passed
@jurahul jurahul deleted the cleanup_intrinsic_record branch September 4, 2024 22:04
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.

3 participants