Skip to content

[mlir][DLTI] Make getPreferredAlignment default to getABIAlignment #128754

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

Merged
merged 1 commit into from
Feb 25, 2025

Conversation

Mogball
Copy link
Contributor

@Mogball Mogball commented Feb 25, 2025

Many types don't have a preferred alignment, but often specifying an ABI alignment is required to implement APIs on top of data layouts. Default the preferred alignment to getABIAlignment to simplify things.

Many types don't have a preferred alignment, but often specifying an ABI
alignment is required to implement APIs on top of data layouts. Default
the preferred alignment to `getABIAlignment` to simplify things.
@llvmbot
Copy link
Member

llvmbot commented Feb 25, 2025

@llvm/pr-subscribers-mlir

@llvm/pr-subscribers-mlir-llvm

Author: Jeff Niu (Mogball)

Changes

Many types don't have a preferred alignment, but often specifying an ABI alignment is required to implement APIs on top of data layouts. Default the preferred alignment to getABIAlignment to simplify things.


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

4 Files Affected:

  • (modified) mlir/include/mlir/Dialect/LLVMIR/LLVMTypes.td (+5-3)
  • (modified) mlir/include/mlir/Dialect/Ptr/IR/PtrDialect.td (+2-1)
  • (modified) mlir/include/mlir/Interfaces/DataLayoutInterfaces.td (+5-1)
  • (modified) mlir/test/lib/Dialect/Test/TestTypeDefs.td (+2-2)
diff --git a/mlir/include/mlir/Dialect/LLVMIR/LLVMTypes.td b/mlir/include/mlir/Dialect/LLVMIR/LLVMTypes.td
index 0b9e4b9c55738..2ecbf8f50c50c 100644
--- a/mlir/include/mlir/Dialect/LLVMIR/LLVMTypes.td
+++ b/mlir/include/mlir/Dialect/LLVMIR/LLVMTypes.td
@@ -25,7 +25,8 @@ class LLVMType<string typeName, string typeMnemonic, list<Trait> traits = []>
 //===----------------------------------------------------------------------===//
 
 def LLVMArrayType : LLVMType<"LLVMArray", "array", [
-    DeclareTypeInterfaceMethods<DataLayoutTypeInterface, ["getTypeSize"]>,
+    DeclareTypeInterfaceMethods<DataLayoutTypeInterface,
+                                ["getTypeSize", "getPreferredAlignment"]>,
     DeclareTypeInterfaceMethods<DestructurableTypeInterface>]> {
   let summary = "LLVM array type";
   let description = [{
@@ -124,7 +125,7 @@ def LLVMFunctionType : LLVMType<"LLVMFunction", "func"> {
 def LLVMStructType : LLVMType<"LLVMStruct", "struct", [
   MutableType,
   DeclareTypeInterfaceMethods<DataLayoutTypeInterface,
-    ["areCompatible", "verifyEntries"]>,
+    ["areCompatible", "verifyEntries", "getPreferredAlignment"]>,
   DeclareTypeInterfaceMethods<DestructurableTypeInterface,
     ["getSubelementIndexMap", "getTypeAtIndex"]>
 ]> {
@@ -257,7 +258,8 @@ def LLVMStructType : LLVMType<"LLVMStruct", "struct", [
 
 def LLVMPointerType : LLVMType<"LLVMPointer", "ptr", [
     DeclareTypeInterfaceMethods<DataLayoutTypeInterface, [
-      "getIndexBitwidth", "areCompatible", "verifyEntries"]>]> {
+      "getIndexBitwidth", "areCompatible", "verifyEntries",
+      "getPreferredAlignment"]>]> {
   let summary = "LLVM pointer type";
   let description = [{
     The `!llvm.ptr` type is an LLVM pointer type. This type typically represents
diff --git a/mlir/include/mlir/Dialect/Ptr/IR/PtrDialect.td b/mlir/include/mlir/Dialect/Ptr/IR/PtrDialect.td
index 14d72c3001d91..bc377dcc72e48 100644
--- a/mlir/include/mlir/Dialect/Ptr/IR/PtrDialect.td
+++ b/mlir/include/mlir/Dialect/Ptr/IR/PtrDialect.td
@@ -38,7 +38,8 @@ class Ptr_Type<string name, string typeMnemonic, list<Trait> traits = []>
 def Ptr_PtrType : Ptr_Type<"Ptr", "ptr", [
     MemRefElementTypeInterface,
     DeclareTypeInterfaceMethods<DataLayoutTypeInterface, [
-      "areCompatible", "getIndexBitwidth", "verifyEntries"]>
+      "areCompatible", "getIndexBitwidth", "verifyEntries",
+      "getPreferredAlignment"]>
   ]> {
   let summary = "pointer type";
   let description = [{
diff --git a/mlir/include/mlir/Interfaces/DataLayoutInterfaces.td b/mlir/include/mlir/Interfaces/DataLayoutInterfaces.td
index 0d09b92928fe3..3e4733d252015 100644
--- a/mlir/include/mlir/Interfaces/DataLayoutInterfaces.td
+++ b/mlir/include/mlir/Interfaces/DataLayoutInterfaces.td
@@ -598,7 +598,11 @@ def DataLayoutTypeInterface : TypeInterface<"DataLayoutTypeInterface"> {
       /*retTy=*/"uint64_t",
       /*methodName=*/"getPreferredAlignment",
       /*args=*/(ins "const ::mlir::DataLayout &":$dataLayout,
-                    "::mlir::DataLayoutEntryListRef":$params)
+                    "::mlir::DataLayoutEntryListRef":$params),
+      /*methodBody=*/"",
+      /*defaultImplementation=*/[{
+        return $_type.getABIAlignment(dataLayout, params);
+      }]
     >,
     InterfaceMethod<
       /*description=*/"Returns the bitwidth that should be used when "
diff --git a/mlir/test/lib/Dialect/Test/TestTypeDefs.td b/mlir/test/lib/Dialect/Test/TestTypeDefs.td
index 6335701786ecc..079ffb22e02c2 100644
--- a/mlir/test/lib/Dialect/Test/TestTypeDefs.td
+++ b/mlir/test/lib/Dialect/Test/TestTypeDefs.td
@@ -148,8 +148,8 @@ def TestType : Test_Type<"Test", [
 }
 
 def TestTypeWithLayoutType : Test_Type<"TestTypeWithLayout", [
-  DeclareTypeInterfaceMethods<DataLayoutTypeInterface, ["getIndexBitwidth",
-                                                        "areCompatible"]>
+  DeclareTypeInterfaceMethods<DataLayoutTypeInterface,
+    ["getIndexBitwidth", "areCompatible", "getPreferredAlignment"]>
 ]> {
   let mnemonic = "test_type_with_layout";
   let parameters = (ins "unsigned":$key);

Copy link
Member

@zero9178 zero9178 left a comment

Choose a reason for hiding this comment

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

Seems like a good default to me!

@Mogball Mogball merged commit 4357a66 into main Feb 25, 2025
12 of 13 checks passed
@Mogball Mogball deleted the users/mogball/dlti branch February 25, 2025 19:32
dkolsen-pgi added a commit to dkolsen-pgi/llvm-project that referenced this pull request Feb 25, 2025
In llvm#128754, `DataLayoutTypeInterface` was changed to give
`getPreferredAlignment` a default implemention.  As a result, table-gen
no longer declared `getPreferredAlignment` when defining a class that
contained `[DeclareTypeInterfaceMethods<DataLayoutTypeInterface>]` in
the table-gen definition.  That means all of the definitions in
`CIRTypes.cpp`, such as `PointerType::getPreferredAligment`, were
compilation errors.

Delete all the definitions of `getPreferredAlignment`.  I verified that
the default implementation does the exact same thing as the explicit
overrides that are being deleted.
dkolsen-pgi added a commit that referenced this pull request Feb 25, 2025
In #128754, `DataLayoutTypeInterface` was changed to give
`getPreferredAlignment` a default implemention. As a result, table-gen
no longer declared `getPreferredAlignment` when defining a class that
contained `[DeclareTypeInterfaceMethods<DataLayoutTypeInterface>]` in
the table-gen definition. That means all of the definitions in
`CIRTypes.cpp`, such as `PointerType::getPreferredAligment`, were
compilation errors.

Delete all the definitions of `getPreferredAlignment`. I verified that
the default implementation does the exact same thing as the explicit
overrides that are being deleted.
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