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

[MLIR][LLVM] Lift alignstack attribute ptr type restriction #133195

Merged
merged 1 commit into from
Mar 27, 2025

Conversation

bcardosolopes
Copy link
Member

Current usage of alignstack is restricted to LLVM pointer types, whereas when it's used in parameters it's possible to use it for other types, see examples like {i8, i8}, [2 x float], etc in llvm/test/CodeGen. This PR lifts the restriction and add testcases.

Current usage of alignstack is restricted to LLVM pointer types, whereas when
it's used in parameters it's possible to use it for other types, see examples
like `{i8, i8}, [2 x float], etc` in `llvm/test/CodeGen`. This PR lifts the
restriction and add testcases.
@llvmbot
Copy link
Member

llvmbot commented Mar 27, 2025

@llvm/pr-subscribers-mlir-llvm

@llvm/pr-subscribers-mlir

Author: Bruno Cardoso Lopes (bcardosolopes)

Changes

Current usage of alignstack is restricted to LLVM pointer types, whereas when it's used in parameters it's possible to use it for other types, see examples like {i8, i8}, [2 x float], etc in llvm/test/CodeGen. This PR lifts the restriction and add testcases.


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

4 Files Affected:

  • (modified) mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp (+8-2)
  • (added) mlir/test/Dialect/LLVMIR/call-param.mlir (+23)
  • (modified) mlir/test/Dialect/LLVMIR/parameter-attrs-invalid.mlir (-5)
  • (modified) mlir/test/Target/LLVMIR/llvmir.mlir (+24)
diff --git a/mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp b/mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp
index 18a70cc64628f..32b5e8bdcebeb 100644
--- a/mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp
+++ b/mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp
@@ -4032,8 +4032,7 @@ LogicalResult LLVMDialect::verifyParameterAttribute(Operation *op,
   // Check an integer attribute that is attached to a pointer value.
   if (name == LLVMDialect::getAlignAttrName() ||
       name == LLVMDialect::getDereferenceableAttrName() ||
-      name == LLVMDialect::getDereferenceableOrNullAttrName() ||
-      name == LLVMDialect::getStackAlignmentAttrName()) {
+      name == LLVMDialect::getDereferenceableOrNullAttrName()) {
     if (failed(checkIntegerAttrType()))
       return failure();
     if (verifyValueType && failed(checkPointerType()))
@@ -4041,6 +4040,13 @@ LogicalResult LLVMDialect::verifyParameterAttribute(Operation *op,
     return success();
   }
 
+  // Check an integer attribute that is attached to a pointer value.
+  if (name == LLVMDialect::getStackAlignmentAttrName()) {
+    if (failed(checkIntegerAttrType()))
+      return failure();
+    return success();
+  }
+
   // Check a unit attribute that can be attached to arbitrary types.
   if (name == LLVMDialect::getNoUndefAttrName() ||
       name == LLVMDialect::getInRegAttrName() ||
diff --git a/mlir/test/Dialect/LLVMIR/call-param.mlir b/mlir/test/Dialect/LLVMIR/call-param.mlir
new file mode 100644
index 0000000000000..f64c233c47625
--- /dev/null
+++ b/mlir/test/Dialect/LLVMIR/call-param.mlir
@@ -0,0 +1,23 @@
+// RUN: mlir-opt %s -split-input-file | FileCheck %s
+
+llvm.func local_unnamed_addr @testfn(!llvm.array<2 x f32> {llvm.alignstack = 8 : i64})
+llvm.func internal @g(%arg0: !llvm.array<2 x f32>) attributes {dso_local} {
+  // CHECK-LABEL: @g
+  // CHECK: llvm.call @testfn(%arg0) : (!llvm.array<2 x f32> {llvm.alignstack = 8 : i64}) -> ()
+  llvm.call @testfn(%arg0) : (!llvm.array<2 x f32> {llvm.alignstack = 8 : i64}) -> ()
+  llvm.return
+}
+llvm.func local_unnamed_addr @testfn2(!llvm.struct<(i8, i8)> {llvm.alignstack = 8 : i64})
+llvm.func internal @h(%arg0: !llvm.struct<(i8, i8)>) attributes {dso_local} {
+  // CHECK-LABEL: @h
+  // CHECK: llvm.call @testfn2(%arg0) : (!llvm.struct<(i8, i8)> {llvm.alignstack = 8 : i64}) -> ()
+  llvm.call @testfn2(%arg0) : (!llvm.struct<(i8, i8)> {llvm.alignstack = 8 : i64}) -> ()
+  llvm.return
+}
+llvm.func local_unnamed_addr @testfn3(i32 {llvm.alignstack = 8 : i64})
+llvm.func internal @i(%arg0: i32) attributes {dso_local} {
+  // CHECK-LABEL: @i
+  // CHECK: llvm.call @testfn3(%arg0) : (i32 {llvm.alignstack = 8 : i64}) -> ()
+  llvm.call @testfn3(%arg0) : (i32 {llvm.alignstack = 8 : i64}) -> ()
+  llvm.return
+}
diff --git a/mlir/test/Dialect/LLVMIR/parameter-attrs-invalid.mlir b/mlir/test/Dialect/LLVMIR/parameter-attrs-invalid.mlir
index 55b1d4faf207f..1cf561ac6c6a5 100644
--- a/mlir/test/Dialect/LLVMIR/parameter-attrs-invalid.mlir
+++ b/mlir/test/Dialect/LLVMIR/parameter-attrs-invalid.mlir
@@ -157,11 +157,6 @@ llvm.func @invalid_returned_attr_type(%0 : i32 {llvm.returned = !llvm.ptr})
 
 // -----
 
-// expected-error@below {{"llvm.alignstack" attribute attached to non-pointer LLVM type}}
-llvm.func @invalid_alignstack_arg_type(%0 : i32 {llvm.alignstack = 10 : i32})
-
-// -----
-
 // expected-error@below {{"llvm.alignstack" should be an integer attribute}}
 llvm.func @invalid_alignstack_attr_type(%0 : i32 {llvm.alignstack = "foo"})
 
diff --git a/mlir/test/Target/LLVMIR/llvmir.mlir b/mlir/test/Target/LLVMIR/llvmir.mlir
index 2476b1b15faaa..89faa6abb6cf7 100644
--- a/mlir/test/Target/LLVMIR/llvmir.mlir
+++ b/mlir/test/Target/LLVMIR/llvmir.mlir
@@ -2842,3 +2842,27 @@ llvm.func @call_alias_func() {
 
 // CHECK-LABEL: @call_alias_func
 // CHECK: call void dso_local_equivalent @alias_func()
+
+// -----
+
+llvm.func local_unnamed_addr @testfn(!llvm.array<2 x f32> {llvm.alignstack = 8 : i64})
+llvm.func internal @g(%arg0: !llvm.array<2 x f32>) attributes {dso_local} {
+  // CHECK-LABEL: @g
+  // CHECK: call void @testfn([2 x float] alignstack(8) %0)
+  llvm.call @testfn(%arg0) : (!llvm.array<2 x f32> {llvm.alignstack = 8 : i64}) -> ()
+  llvm.return
+}
+llvm.func local_unnamed_addr @testfn2(!llvm.struct<(i8, i8)> {llvm.alignstack = 8 : i64})
+llvm.func internal @h(%arg0: !llvm.struct<(i8, i8)>) attributes {dso_local} {
+  // CHECK-LABEL: @h
+  // CHECK: call void @testfn2({ i8, i8 } alignstack(8) %0)
+  llvm.call @testfn2(%arg0) : (!llvm.struct<(i8, i8)> {llvm.alignstack = 8 : i64}) -> ()
+  llvm.return
+}
+llvm.func local_unnamed_addr @testfn3(i32 {llvm.alignstack = 8 : i64})
+llvm.func internal @i(%arg0: i32) attributes {dso_local} {
+  // CHECK-LABEL: @i
+  // CHECK: call void @testfn3(i32 alignstack(8) %0)
+  llvm.call @testfn3(%arg0) : (i32 {llvm.alignstack = 8 : i64}) -> ()
+  llvm.return
+}

Copy link
Contributor

@gysit gysit left a comment

Choose a reason for hiding this comment

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

Thanks!

LGTM

@bcardosolopes bcardosolopes merged commit 08aedf7 into llvm:main Mar 27, 2025
14 checks passed
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