[Mlir-commits] [mlir] [MLIR] Add support for frame pointers in MLIR (PR #72145)

Radu Salavat llvmlistbot at llvm.org
Mon Dec 4 05:54:35 PST 2023


https://github.com/Radu2k updated https://github.com/llvm/llvm-project/pull/72145

>From 05455399af8dd324c3770b492765dd2cc755e316 Mon Sep 17 00:00:00 2001
From: Radu2k <radusalavat48 at gmail.com>
Date: Mon, 13 Nov 2023 17:57:24 +0000
Subject: [PATCH 1/8] [MLIR] Add support for frame pointers in MLIR

---
 mlir/include/mlir/Dialect/LLVMIR/LLVMEnums.td | 19 ++++++++++++++
 mlir/include/mlir/Dialect/LLVMIR/LLVMOps.td   |  3 ++-
 mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp    | 25 +++++++++++++++++--
 mlir/lib/Target/LLVMIR/ModuleImport.cpp       |  7 ++++++
 mlir/lib/Target/LLVMIR/ModuleTranslation.cpp  |  4 +++
 mlir/test/Dialect/LLVMIR/func.mlir            |  6 +++++
 .../Target/LLVMIR/Import/frame-pointer.ll     |  7 ++++++
 mlir/test/Target/LLVMIR/frame-pointer.mlir    |  8 ++++++
 8 files changed, 76 insertions(+), 3 deletions(-)
 create mode 100644 mlir/test/Target/LLVMIR/Import/frame-pointer.ll
 create mode 100644 mlir/test/Target/LLVMIR/frame-pointer.mlir

diff --git a/mlir/include/mlir/Dialect/LLVMIR/LLVMEnums.td b/mlir/include/mlir/Dialect/LLVMIR/LLVMEnums.td
index f05230526c21f..446850d6dc7bb 100644
--- a/mlir/include/mlir/Dialect/LLVMIR/LLVMEnums.td
+++ b/mlir/include/mlir/Dialect/LLVMIR/LLVMEnums.td
@@ -664,4 +664,23 @@ def ModRefInfoEnum : LLVM_EnumAttr<
   let cppNamespace = "::mlir::LLVM";
 }
 
+//===----------------------------------------------------------------------===//
+// FramePointerAttr
+//===----------------------------------------------------------------------===//
+
+def FramePointerKindNone
+    : LLVM_EnumAttrCase<"None", "none", "None", 0>;
+def FramePointerKindNonLeaf
+    : LLVM_EnumAttrCase<"NonLeaf", "non-leaf", "NonLeaf", 1>;
+def FramePointerKindAll
+    : LLVM_EnumAttrCase<"All", "all", "All", 2>;
+
+def FramePointerKindAttr : LLVM_EnumAttr<
+    "FramePointerKind",
+    "::llvm::FramePointerKind",
+    "LLVM FramePointerKind",
+    [FramePointerKindNone, FramePointerKindNonLeaf, FramePointerKindAll]> {
+  let cppNamespace = "::mlir::LLVM";
+}
+
 #endif // LLVMIR_ENUMS
diff --git a/mlir/include/mlir/Dialect/LLVMIR/LLVMOps.td b/mlir/include/mlir/Dialect/LLVMIR/LLVMOps.td
index 88f4f81735372..370dc2c376a8d 100644
--- a/mlir/include/mlir/Dialect/LLVMIR/LLVMOps.td
+++ b/mlir/include/mlir/Dialect/LLVMIR/LLVMOps.td
@@ -1391,7 +1391,8 @@ def LLVM_LLVMFuncOp : LLVM_Op<"func", [
     OptionalAttr<StrAttr>:$section,
     OptionalAttr<UnnamedAddr>:$unnamed_addr,
     OptionalAttr<I64Attr>:$alignment,
-    OptionalAttr<LLVM_VScaleRangeAttr>:$vscale_range
+    OptionalAttr<LLVM_VScaleRangeAttr>:$vscale_range,
+    OptionalAttr<FramePointerKindAttr>:$frame_pointer
   );
 
   let regions = (region AnyRegion:$body);
diff --git a/mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp b/mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp
index 71860363b8ea5..a831d625c6eb8 100644
--- a/mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp
+++ b/mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp
@@ -2309,7 +2309,7 @@ ParseResult LLVMFuncOp::parse(OpAsmParser &parser, OperationState &result) {
                                    IntegerAttr::get(intTy, minRange),
                                    IntegerAttr::get(intTy, maxRange)));
   }
-  // Parse the optional comdat selector.
+  // Parse the optional comdat selector.:q
   if (succeeded(parser.parseOptionalKeyword("comdat"))) {
     SymbolRefAttr comdat;
     if (parser.parseLParen() || parser.parseAttribute(comdat) ||
@@ -2319,6 +2319,23 @@ ParseResult LLVMFuncOp::parse(OpAsmParser &parser, OperationState &result) {
     result.addAttribute(getComdatAttrName(result.name), comdat);
   }
 
+  // Parse the optional frame_pointer
+  if (succeeded(parser.parseOptionalKeyword("frame_pointer"))) {
+    std::string string;
+    
+    if (parser.parseEqual() || parser.parseString(&string))
+      return failure();
+    if (!LLVM::symbolizeFramePointerKind(string))
+    { 
+      llvm::outs() << "failure: frame-pointer option not recognized: " << string << "\n";
+      return failure();
+    }
+
+    result.addAttribute(getFramePointerAttrName(result.name), 
+        LLVM::FramePointerKindAttr::get(parser.getContext(),
+                                   LLVM::symbolizeFramePointerKind(string).value()));
+  }
+
   if (failed(parser.parseOptionalAttrDictWithKeyword(result.attributes)))
     return failure();
   function_interface_impl::addArgAndResultAttrs(
@@ -2373,13 +2390,17 @@ void LLVMFuncOp::print(OpAsmPrinter &p) {
   // Print the optional comdat selector.
   if (auto comdat = getComdat())
     p << " comdat(" << *comdat << ')';
+  
+  // Print the optional frame pointer option.
+  if (std::optional<FramePointerKind> frame_pointer = getFramePointer())
+    p << " frame_pointer=" << "\"" << stringifyFramePointerKind(frame_pointer.value()) << "\"";
 
   function_interface_impl::printFunctionAttributes(
       p, *this,
       {getFunctionTypeAttrName(), getArgAttrsAttrName(), getResAttrsAttrName(),
        getLinkageAttrName(), getCConvAttrName(), getVisibility_AttrName(),
        getComdatAttrName(), getUnnamedAddrAttrName(),
-       getVscaleRangeAttrName()});
+       getVscaleRangeAttrName(), getFramePointerAttrName()});
 
   // Print the body if this is not an external function.
   Region &body = getBody();
diff --git a/mlir/lib/Target/LLVMIR/ModuleImport.cpp b/mlir/lib/Target/LLVMIR/ModuleImport.cpp
index 9cdc1f45d38fa..518cccf2003c4 100644
--- a/mlir/lib/Target/LLVMIR/ModuleImport.cpp
+++ b/mlir/lib/Target/LLVMIR/ModuleImport.cpp
@@ -1694,6 +1694,13 @@ void ModuleImport::processFunctionAttributes(llvm::Function *func,
         context, IntegerAttr::get(intTy, attr.getVScaleRangeMin()),
         IntegerAttr::get(intTy, attr.getVScaleRangeMax().value_or(0))));
   }
+
+  // Process frame-pointer attribute
+  if (func->hasFnAttribute("frame-pointer")) {
+    llvm::StringRef stringRefFramePointerKind = func->getFnAttribute("frame-pointer").getValueAsString();
+    funcOp.setFramePointerAttr(LLVM::FramePointerKindAttr::get(funcOp.getContext(),
+                                  symbolizeFramePointerKind(stringRefFramePointerKind).value()));
+  }
 }
 
 DictionaryAttr
diff --git a/mlir/lib/Target/LLVMIR/ModuleTranslation.cpp b/mlir/lib/Target/LLVMIR/ModuleTranslation.cpp
index 322843e656276..179f8a6b76fdd 100644
--- a/mlir/lib/Target/LLVMIR/ModuleTranslation.cpp
+++ b/mlir/lib/Target/LLVMIR/ModuleTranslation.cpp
@@ -946,6 +946,10 @@ LogicalResult ModuleTranslation::convertOneFunction(LLVMFuncOp func) {
     llvmFunc->addFnAttr(llvm::Attribute::getWithVScaleRangeArgs(
         getLLVMContext(), attr->getMinRange().getInt(),
         attr->getMaxRange().getInt()));
+  
+  // Add function attribute frame-pointer, if found.
+  if (auto attr = func.getFramePointerAttr()) 
+    llvmFunc->addFnAttr("frame-pointer", stringifyFramePointerKind(attr.getValue()));
 
   // First, create all blocks so we can jump to them.
   llvm::LLVMContext &llvmContext = llvmFunc->getContext();
diff --git a/mlir/test/Dialect/LLVMIR/func.mlir b/mlir/test/Dialect/LLVMIR/func.mlir
index 63e20b1d8fc31..9e68a3dff4bca 100644
--- a/mlir/test/Dialect/LLVMIR/func.mlir
+++ b/mlir/test/Dialect/LLVMIR/func.mlir
@@ -249,6 +249,12 @@ module {
     // CHECK-SAME: vscale_range(1, 2)
     llvm.return
   }
+
+  llvm.func @frame_pointer_roundtrip() frame_pointer="non-leaf" {
+    // CHECK: @frame_pointer_roundtrip()
+    // CHECK-SAME: frame_pointer="non-leaf"
+    llvm.return
+  }
 }
 
 // -----
diff --git a/mlir/test/Target/LLVMIR/Import/frame-pointer.ll b/mlir/test/Target/LLVMIR/Import/frame-pointer.ll
new file mode 100644
index 0000000000000..698771f4ae04c
--- /dev/null
+++ b/mlir/test/Target/LLVMIR/Import/frame-pointer.ll
@@ -0,0 +1,7 @@
+; RUN: mlir-translate -import-llvm -split-input-file %s | FileCheck %s
+
+define void @frame_pointer_func() "frame-pointer"="non-leaf" {
+  ; CHECK: llvm.func @frame_pointer_func()
+  ; CHECK-SAME: frame_pointer="non-leaf"
+  ret void
+}
diff --git a/mlir/test/Target/LLVMIR/frame-pointer.mlir b/mlir/test/Target/LLVMIR/frame-pointer.mlir
new file mode 100644
index 0000000000000..543b6b1af1761
--- /dev/null
+++ b/mlir/test/Target/LLVMIR/frame-pointer.mlir
@@ -0,0 +1,8 @@
+// RUN: mlir-translate -mlir-to-llvmir %s | FileCheck %s
+
+llvm.func @frame_pointer_func() frame_pointer="non-leaf" {
+  // CHECK-LABEL: define void @frame_pointer_func
+  // CHECK: attributes #{{.*}} = { "frame-pointer"="non-leaf" }
+  llvm.return
+}
+

>From e9fe1785080bcde525c6978f23af9e6abf900c89 Mon Sep 17 00:00:00 2001
From: Radu Salavat <radusalavat48 at gmail.com>
Date: Tue, 14 Nov 2023 11:52:28 +0000
Subject: [PATCH 2/8] Update mlir/test/Target/LLVMIR/frame-pointer.mlir
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Co-authored-by: Markus Böck <markus.boeck02 at gmail.com>
---
 mlir/test/Target/LLVMIR/frame-pointer.mlir | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/mlir/test/Target/LLVMIR/frame-pointer.mlir b/mlir/test/Target/LLVMIR/frame-pointer.mlir
index 543b6b1af1761..f69cd18e0577f 100644
--- a/mlir/test/Target/LLVMIR/frame-pointer.mlir
+++ b/mlir/test/Target/LLVMIR/frame-pointer.mlir
@@ -1,8 +1,8 @@
 // RUN: mlir-translate -mlir-to-llvmir %s | FileCheck %s
 
 llvm.func @frame_pointer_func() frame_pointer="non-leaf" {
-  // CHECK-LABEL: define void @frame_pointer_func
-  // CHECK: attributes #{{.*}} = { "frame-pointer"="non-leaf" }
+  // CHECK-LABEL: define void @frame_pointer_func() #[[ATTRS:\d+]]
+  // CHECK: attributes #[[ATTRS]] = { "frame-pointer"="non-leaf" }
   llvm.return
 }
 

>From 72eb83b32251ec2cd85bac296f172d5660b685b9 Mon Sep 17 00:00:00 2001
From: Radu2k <radusalavat48 at gmail.com>
Date: Tue, 14 Nov 2023 23:22:57 +0000
Subject: [PATCH 3/8] Removed special syntax for handling the frame pointer
 attribute for llvm.func op parser and printer, spelled out 'auto' and
 enhanced the mlir-to-llvmir check by adding ATTRS as variable

---
 mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp    | 23 +------------------
 mlir/lib/Target/LLVMIR/ModuleTranslation.cpp  |  2 +-
 mlir/test/Dialect/LLVMIR/func.mlir            |  4 ++--
 .../Target/LLVMIR/Import/frame-pointer.ll     |  3 ++-
 mlir/test/Target/LLVMIR/frame-pointer.mlir    |  5 ++--
 5 files changed, 9 insertions(+), 28 deletions(-)

diff --git a/mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp b/mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp
index a831d625c6eb8..988e6e77a3b84 100644
--- a/mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp
+++ b/mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp
@@ -2319,23 +2319,6 @@ ParseResult LLVMFuncOp::parse(OpAsmParser &parser, OperationState &result) {
     result.addAttribute(getComdatAttrName(result.name), comdat);
   }
 
-  // Parse the optional frame_pointer
-  if (succeeded(parser.parseOptionalKeyword("frame_pointer"))) {
-    std::string string;
-    
-    if (parser.parseEqual() || parser.parseString(&string))
-      return failure();
-    if (!LLVM::symbolizeFramePointerKind(string))
-    { 
-      llvm::outs() << "failure: frame-pointer option not recognized: " << string << "\n";
-      return failure();
-    }
-
-    result.addAttribute(getFramePointerAttrName(result.name), 
-        LLVM::FramePointerKindAttr::get(parser.getContext(),
-                                   LLVM::symbolizeFramePointerKind(string).value()));
-  }
-
   if (failed(parser.parseOptionalAttrDictWithKeyword(result.attributes)))
     return failure();
   function_interface_impl::addArgAndResultAttrs(
@@ -2390,17 +2373,13 @@ void LLVMFuncOp::print(OpAsmPrinter &p) {
   // Print the optional comdat selector.
   if (auto comdat = getComdat())
     p << " comdat(" << *comdat << ')';
-  
-  // Print the optional frame pointer option.
-  if (std::optional<FramePointerKind> frame_pointer = getFramePointer())
-    p << " frame_pointer=" << "\"" << stringifyFramePointerKind(frame_pointer.value()) << "\"";
 
   function_interface_impl::printFunctionAttributes(
       p, *this,
       {getFunctionTypeAttrName(), getArgAttrsAttrName(), getResAttrsAttrName(),
        getLinkageAttrName(), getCConvAttrName(), getVisibility_AttrName(),
        getComdatAttrName(), getUnnamedAddrAttrName(),
-       getVscaleRangeAttrName(), getFramePointerAttrName()});
+       getVscaleRangeAttrName()});
 
   // Print the body if this is not an external function.
   Region &body = getBody();
diff --git a/mlir/lib/Target/LLVMIR/ModuleTranslation.cpp b/mlir/lib/Target/LLVMIR/ModuleTranslation.cpp
index 179f8a6b76fdd..c15f33ff928c4 100644
--- a/mlir/lib/Target/LLVMIR/ModuleTranslation.cpp
+++ b/mlir/lib/Target/LLVMIR/ModuleTranslation.cpp
@@ -948,7 +948,7 @@ LogicalResult ModuleTranslation::convertOneFunction(LLVMFuncOp func) {
         attr->getMaxRange().getInt()));
   
   // Add function attribute frame-pointer, if found.
-  if (auto attr = func.getFramePointerAttr()) 
+  if (mlir::LLVM::FramePointerKindAttr attr = func.getFramePointerAttr())
     llvmFunc->addFnAttr("frame-pointer", stringifyFramePointerKind(attr.getValue()));
 
   // First, create all blocks so we can jump to them.
diff --git a/mlir/test/Dialect/LLVMIR/func.mlir b/mlir/test/Dialect/LLVMIR/func.mlir
index 9e68a3dff4bca..ab9889c7b2790 100644
--- a/mlir/test/Dialect/LLVMIR/func.mlir
+++ b/mlir/test/Dialect/LLVMIR/func.mlir
@@ -250,9 +250,9 @@ module {
     llvm.return
   }
 
-  llvm.func @frame_pointer_roundtrip() frame_pointer="non-leaf" {
+  llvm.func @frame_pointer_roundtrip() attributes {frame_pointer = 1 : i64} {
     // CHECK: @frame_pointer_roundtrip()
-    // CHECK-SAME: frame_pointer="non-leaf"
+    // CHECK-SAME: attributes {frame_pointer = 1 : i64}
     llvm.return
   }
 }
diff --git a/mlir/test/Target/LLVMIR/Import/frame-pointer.ll b/mlir/test/Target/LLVMIR/Import/frame-pointer.ll
index 698771f4ae04c..8f8eec66913ce 100644
--- a/mlir/test/Target/LLVMIR/Import/frame-pointer.ll
+++ b/mlir/test/Target/LLVMIR/Import/frame-pointer.ll
@@ -2,6 +2,7 @@
 
 define void @frame_pointer_func() "frame-pointer"="non-leaf" {
   ; CHECK: llvm.func @frame_pointer_func()
-  ; CHECK-SAME: frame_pointer="non-leaf"
+  ; CHECK-SAME: attributes {frame_pointer = 1 : i64}   
+  
   ret void
 }
diff --git a/mlir/test/Target/LLVMIR/frame-pointer.mlir b/mlir/test/Target/LLVMIR/frame-pointer.mlir
index f69cd18e0577f..39057b28d5b28 100644
--- a/mlir/test/Target/LLVMIR/frame-pointer.mlir
+++ b/mlir/test/Target/LLVMIR/frame-pointer.mlir
@@ -1,7 +1,8 @@
 // RUN: mlir-translate -mlir-to-llvmir %s | FileCheck %s
 
-llvm.func @frame_pointer_func() frame_pointer="non-leaf" {
-  // CHECK-LABEL: define void @frame_pointer_func() #[[ATTRS:\d+]]
+llvm.func @frame_pointer_func() attributes {frame_pointer = 1 : i64}  {
+  // CHECK-LABEL: define void @frame_pointer_func() 
+  // CHECK-SAME: #[[ATTRS:[0-9]+]]
   // CHECK: attributes #[[ATTRS]] = { "frame-pointer"="non-leaf" }
   llvm.return
 }

>From ff312286d5dad7a296b27df0678dca514ea197ee Mon Sep 17 00:00:00 2001
From: Radu2k <radusalavat48 at gmail.com>
Date: Thu, 23 Nov 2023 18:08:42 +0000
Subject: [PATCH 4/8] Adjust MLIR implementation to format 'frame-pointer'
 appropriately

---
 mlir/include/mlir/Dialect/LLVMIR/LLVMAttrDefs.td |  9 +++++++++
 mlir/include/mlir/Dialect/LLVMIR/LLVMEnums.td    |  5 ++---
 mlir/include/mlir/Dialect/LLVMIR/LLVMOps.td      |  1 +
 mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp       |  2 +-
 mlir/lib/Target/LLVMIR/ModuleImport.cpp          | 10 ++++++----
 mlir/lib/Target/LLVMIR/ModuleTranslation.cpp     |  6 ++++--
 mlir/test/Target/LLVMIR/frame-pointer.mlir       |  2 +-
 7 files changed, 24 insertions(+), 11 deletions(-)

diff --git a/mlir/include/mlir/Dialect/LLVMIR/LLVMAttrDefs.td b/mlir/include/mlir/Dialect/LLVMIR/LLVMAttrDefs.td
index 9e35bf1ba9777..a812d9590e9c4 100644
--- a/mlir/include/mlir/Dialect/LLVMIR/LLVMAttrDefs.td
+++ b/mlir/include/mlir/Dialect/LLVMIR/LLVMAttrDefs.td
@@ -47,6 +47,15 @@ def LinkageAttr : LLVM_Attr<"Linkage", "linkage"> {
   let assemblyFormat = "`<` $linkage `>`";
 }
 
+//===----------------------------------------------------------------------===//
+// FramePointerKindAttr
+//===----------------------------------------------------------------------===//
+
+def FramePointerKindAttr : LLVM_Attr<"FramePointerKind", ""> {
+  let parameters = (ins "framePointerKind::FramePointerKind":$framePointerKind);
+  let assemblyFormat = "$framePointerKind";
+}
+
 //===----------------------------------------------------------------------===//
 // Loop Attributes
 //===----------------------------------------------------------------------===//
diff --git a/mlir/include/mlir/Dialect/LLVMIR/LLVMEnums.td b/mlir/include/mlir/Dialect/LLVMIR/LLVMEnums.td
index 446850d6dc7bb..41161549fe525 100644
--- a/mlir/include/mlir/Dialect/LLVMIR/LLVMEnums.td
+++ b/mlir/include/mlir/Dialect/LLVMIR/LLVMEnums.td
@@ -587,7 +587,6 @@ def Linkage : DialectAttr<
           "::mlir::LLVM::LinkageAttr::get($_builder.getContext(), $0)";
 }
 
-
 //===----------------------------------------------------------------------===//
 // Comdat
 //===----------------------------------------------------------------------===//
@@ -675,12 +674,12 @@ def FramePointerKindNonLeaf
 def FramePointerKindAll
     : LLVM_EnumAttrCase<"All", "all", "All", 2>;
 
-def FramePointerKindAttr : LLVM_EnumAttr<
+def FramePointerKindEnum : LLVM_EnumAttr<
     "FramePointerKind",
     "::llvm::FramePointerKind",
     "LLVM FramePointerKind",
     [FramePointerKindNone, FramePointerKindNonLeaf, FramePointerKindAll]> {
-  let cppNamespace = "::mlir::LLVM";
+  let cppNamespace = "::mlir::LLVM::framePointerKind";
 }
 
 #endif // LLVMIR_ENUMS
diff --git a/mlir/include/mlir/Dialect/LLVMIR/LLVMOps.td b/mlir/include/mlir/Dialect/LLVMIR/LLVMOps.td
index 370dc2c376a8d..94f2f802fcf22 100644
--- a/mlir/include/mlir/Dialect/LLVMIR/LLVMOps.td
+++ b/mlir/include/mlir/Dialect/LLVMIR/LLVMOps.td
@@ -1392,6 +1392,7 @@ def LLVM_LLVMFuncOp : LLVM_Op<"func", [
     OptionalAttr<UnnamedAddr>:$unnamed_addr,
     OptionalAttr<I64Attr>:$alignment,
     OptionalAttr<LLVM_VScaleRangeAttr>:$vscale_range,
+    //DefaultValuedAttr<FramePointerKindAttr, "mlir::LLVM::FramePointerKind::None">:$frame_pointer
     OptionalAttr<FramePointerKindAttr>:$frame_pointer
   );
 
diff --git a/mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp b/mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp
index 988e6e77a3b84..71860363b8ea5 100644
--- a/mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp
+++ b/mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp
@@ -2309,7 +2309,7 @@ ParseResult LLVMFuncOp::parse(OpAsmParser &parser, OperationState &result) {
                                    IntegerAttr::get(intTy, minRange),
                                    IntegerAttr::get(intTy, maxRange)));
   }
-  // Parse the optional comdat selector.:q
+  // Parse the optional comdat selector.
   if (succeeded(parser.parseOptionalKeyword("comdat"))) {
     SymbolRefAttr comdat;
     if (parser.parseLParen() || parser.parseAttribute(comdat) ||
diff --git a/mlir/lib/Target/LLVMIR/ModuleImport.cpp b/mlir/lib/Target/LLVMIR/ModuleImport.cpp
index 518cccf2003c4..46385801be59a 100644
--- a/mlir/lib/Target/LLVMIR/ModuleImport.cpp
+++ b/mlir/lib/Target/LLVMIR/ModuleImport.cpp
@@ -1695,11 +1695,13 @@ void ModuleImport::processFunctionAttributes(llvm::Function *func,
         IntegerAttr::get(intTy, attr.getVScaleRangeMax().value_or(0))));
   }
 
-  // Process frame-pointer attribute
+  // Process frame-pointer attribute.
   if (func->hasFnAttribute("frame-pointer")) {
-    llvm::StringRef stringRefFramePointerKind = func->getFnAttribute("frame-pointer").getValueAsString();
-    funcOp.setFramePointerAttr(LLVM::FramePointerKindAttr::get(funcOp.getContext(),
-                                  symbolizeFramePointerKind(stringRefFramePointerKind).value()));
+    StringRef stringRefFramePointerKind = func->getFnAttribute("frame-pointer").getValueAsString();
+    funcOp.setFramePointerAttr(LLVM::FramePointerKindAttr::get(
+        funcOp.getContext(), LLVM::framePointerKind::symbolizeFramePointerKind(
+                                 stringRefFramePointerKind)
+                                 .value()));
   }
 }
 
diff --git a/mlir/lib/Target/LLVMIR/ModuleTranslation.cpp b/mlir/lib/Target/LLVMIR/ModuleTranslation.cpp
index c15f33ff928c4..fc8db57284628 100644
--- a/mlir/lib/Target/LLVMIR/ModuleTranslation.cpp
+++ b/mlir/lib/Target/LLVMIR/ModuleTranslation.cpp
@@ -948,8 +948,10 @@ LogicalResult ModuleTranslation::convertOneFunction(LLVMFuncOp func) {
         attr->getMaxRange().getInt()));
   
   // Add function attribute frame-pointer, if found.
-  if (mlir::LLVM::FramePointerKindAttr attr = func.getFramePointerAttr())
-    llvmFunc->addFnAttr("frame-pointer", stringifyFramePointerKind(attr.getValue()));
+  if (FramePointerKindAttr attr = func.getFramePointerAttr())
+    llvmFunc->addFnAttr("frame-pointer",
+                        LLVM::framePointerKind::stringifyFramePointerKind(
+                            (attr.getFramePointerKind())));
 
   // First, create all blocks so we can jump to them.
   llvm::LLVMContext &llvmContext = llvmFunc->getContext();
diff --git a/mlir/test/Target/LLVMIR/frame-pointer.mlir b/mlir/test/Target/LLVMIR/frame-pointer.mlir
index 39057b28d5b28..8f3390db950bd 100644
--- a/mlir/test/Target/LLVMIR/frame-pointer.mlir
+++ b/mlir/test/Target/LLVMIR/frame-pointer.mlir
@@ -1,6 +1,6 @@
 // RUN: mlir-translate -mlir-to-llvmir %s | FileCheck %s
 
-llvm.func @frame_pointer_func() attributes {frame_pointer = 1 : i64}  {
+llvm.func @frame_pointer_func() attributes {frame_pointer = #llvm< "non-leaf">}  {
   // CHECK-LABEL: define void @frame_pointer_func() 
   // CHECK-SAME: #[[ATTRS:[0-9]+]]
   // CHECK: attributes #[[ATTRS]] = { "frame-pointer"="non-leaf" }

>From ea39cca0231f125e4ce7b46aad1342135035dd94 Mon Sep 17 00:00:00 2001
From: Radu2k <radusalavat48 at gmail.com>
Date: Fri, 1 Dec 2023 20:45:03 +0000
Subject: [PATCH 5/8] Change LLVM Attr definition, adapt tests structure and
 make appropriate format changes

---
 mlir/include/mlir/Dialect/LLVMIR/LLVMAttrDefs.td | 4 ++--
 mlir/lib/Target/LLVMIR/ModuleImport.cpp          | 3 ++-
 mlir/lib/Target/LLVMIR/ModuleTranslation.cpp     | 2 +-
 mlir/test/Dialect/LLVMIR/func.mlir               | 4 ++--
 mlir/test/Target/LLVMIR/Import/frame-pointer.ll  | 2 +-
 mlir/test/Target/LLVMIR/frame-pointer.mlir       | 2 +-
 6 files changed, 9 insertions(+), 8 deletions(-)

diff --git a/mlir/include/mlir/Dialect/LLVMIR/LLVMAttrDefs.td b/mlir/include/mlir/Dialect/LLVMIR/LLVMAttrDefs.td
index a812d9590e9c4..8182e245c0f78 100644
--- a/mlir/include/mlir/Dialect/LLVMIR/LLVMAttrDefs.td
+++ b/mlir/include/mlir/Dialect/LLVMIR/LLVMAttrDefs.td
@@ -51,9 +51,9 @@ def LinkageAttr : LLVM_Attr<"Linkage", "linkage"> {
 // FramePointerKindAttr
 //===----------------------------------------------------------------------===//
 
-def FramePointerKindAttr : LLVM_Attr<"FramePointerKind", ""> {
+def FramePointerKindAttr : LLVM_Attr<"FramePointerKind", "framePointerKind"> {
   let parameters = (ins "framePointerKind::FramePointerKind":$framePointerKind);
-  let assemblyFormat = "$framePointerKind";
+  let assemblyFormat = "`<` $framePointerKind `>`";
 }
 
 //===----------------------------------------------------------------------===//
diff --git a/mlir/lib/Target/LLVMIR/ModuleImport.cpp b/mlir/lib/Target/LLVMIR/ModuleImport.cpp
index 46385801be59a..943c81ac5b5d7 100644
--- a/mlir/lib/Target/LLVMIR/ModuleImport.cpp
+++ b/mlir/lib/Target/LLVMIR/ModuleImport.cpp
@@ -1697,7 +1697,8 @@ void ModuleImport::processFunctionAttributes(llvm::Function *func,
 
   // Process frame-pointer attribute.
   if (func->hasFnAttribute("frame-pointer")) {
-    StringRef stringRefFramePointerKind = func->getFnAttribute("frame-pointer").getValueAsString();
+    StringRef stringRefFramePointerKind =
+        func->getFnAttribute("frame-pointer").getValueAsString();
     funcOp.setFramePointerAttr(LLVM::FramePointerKindAttr::get(
         funcOp.getContext(), LLVM::framePointerKind::symbolizeFramePointerKind(
                                  stringRefFramePointerKind)
diff --git a/mlir/lib/Target/LLVMIR/ModuleTranslation.cpp b/mlir/lib/Target/LLVMIR/ModuleTranslation.cpp
index fc8db57284628..992b61b1af15a 100644
--- a/mlir/lib/Target/LLVMIR/ModuleTranslation.cpp
+++ b/mlir/lib/Target/LLVMIR/ModuleTranslation.cpp
@@ -946,7 +946,7 @@ LogicalResult ModuleTranslation::convertOneFunction(LLVMFuncOp func) {
     llvmFunc->addFnAttr(llvm::Attribute::getWithVScaleRangeArgs(
         getLLVMContext(), attr->getMinRange().getInt(),
         attr->getMaxRange().getInt()));
-  
+
   // Add function attribute frame-pointer, if found.
   if (FramePointerKindAttr attr = func.getFramePointerAttr())
     llvmFunc->addFnAttr("frame-pointer",
diff --git a/mlir/test/Dialect/LLVMIR/func.mlir b/mlir/test/Dialect/LLVMIR/func.mlir
index ab9889c7b2790..ec3cb5f503925 100644
--- a/mlir/test/Dialect/LLVMIR/func.mlir
+++ b/mlir/test/Dialect/LLVMIR/func.mlir
@@ -250,9 +250,9 @@ module {
     llvm.return
   }
 
-  llvm.func @frame_pointer_roundtrip() attributes {frame_pointer = 1 : i64} {
+  llvm.func @frame_pointer_roundtrip() attributes {frame_pointer = #llvm.framePointerKind<"non-leaf">} {
     // CHECK: @frame_pointer_roundtrip()
-    // CHECK-SAME: attributes {frame_pointer = 1 : i64}
+    // CHECK-SAME: attributes {frame_pointer = #llvm.framePointerKind<"non-leaf">}
     llvm.return
   }
 }
diff --git a/mlir/test/Target/LLVMIR/Import/frame-pointer.ll b/mlir/test/Target/LLVMIR/Import/frame-pointer.ll
index 8f8eec66913ce..a51cc8581db9c 100644
--- a/mlir/test/Target/LLVMIR/Import/frame-pointer.ll
+++ b/mlir/test/Target/LLVMIR/Import/frame-pointer.ll
@@ -2,7 +2,7 @@
 
 define void @frame_pointer_func() "frame-pointer"="non-leaf" {
   ; CHECK: llvm.func @frame_pointer_func()
-  ; CHECK-SAME: attributes {frame_pointer = 1 : i64}   
+  ; CHECK-SAME: attributes {frame_pointer = #llvm.framePointerKind<"non-leaf">}
   
   ret void
 }
diff --git a/mlir/test/Target/LLVMIR/frame-pointer.mlir b/mlir/test/Target/LLVMIR/frame-pointer.mlir
index 8f3390db950bd..3ee347eeebb01 100644
--- a/mlir/test/Target/LLVMIR/frame-pointer.mlir
+++ b/mlir/test/Target/LLVMIR/frame-pointer.mlir
@@ -1,6 +1,6 @@
 // RUN: mlir-translate -mlir-to-llvmir %s | FileCheck %s
 
-llvm.func @frame_pointer_func() attributes {frame_pointer = #llvm< "non-leaf">}  {
+llvm.func @frame_pointer_func() attributes {frame_pointer = #llvm.framePointerKind<"non-leaf">}  {
   // CHECK-LABEL: define void @frame_pointer_func() 
   // CHECK-SAME: #[[ATTRS:[0-9]+]]
   // CHECK: attributes #[[ATTRS]] = { "frame-pointer"="non-leaf" }

>From 69beb1dc45bdc52eaadf2280fab17204b3a403ac Mon Sep 17 00:00:00 2001
From: Radu2k <radu.salavat at arm.com>
Date: Mon, 4 Dec 2023 09:43:17 +0000
Subject: [PATCH 6/8] Change tests structure

---
 mlir/include/mlir/Dialect/LLVMIR/LLVMEnums.td   | 2 +-
 mlir/test/Dialect/LLVMIR/func.mlir              | 4 ++--
 mlir/test/Target/LLVMIR/Import/frame-pointer.ll | 5 +++--
 mlir/test/Target/LLVMIR/frame-pointer.mlir      | 7 +++----
 4 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/mlir/include/mlir/Dialect/LLVMIR/LLVMEnums.td b/mlir/include/mlir/Dialect/LLVMIR/LLVMEnums.td
index 41161549fe525..3b5984498cf83 100644
--- a/mlir/include/mlir/Dialect/LLVMIR/LLVMEnums.td
+++ b/mlir/include/mlir/Dialect/LLVMIR/LLVMEnums.td
@@ -664,7 +664,7 @@ def ModRefInfoEnum : LLVM_EnumAttr<
 }
 
 //===----------------------------------------------------------------------===//
-// FramePointerAttr
+// FramePointerKind
 //===----------------------------------------------------------------------===//
 
 def FramePointerKindNone
diff --git a/mlir/test/Dialect/LLVMIR/func.mlir b/mlir/test/Dialect/LLVMIR/func.mlir
index ec3cb5f503925..d09df07676122 100644
--- a/mlir/test/Dialect/LLVMIR/func.mlir
+++ b/mlir/test/Dialect/LLVMIR/func.mlir
@@ -250,9 +250,9 @@ module {
     llvm.return
   }
 
+  // CHECK-LABEL: @frame_pointer_roundtrip()
+  // CHECK-SAME: attributes {frame_pointer = #llvm.framePointerKind<"non-leaf">}
   llvm.func @frame_pointer_roundtrip() attributes {frame_pointer = #llvm.framePointerKind<"non-leaf">} {
-    // CHECK: @frame_pointer_roundtrip()
-    // CHECK-SAME: attributes {frame_pointer = #llvm.framePointerKind<"non-leaf">}
     llvm.return
   }
 }
diff --git a/mlir/test/Target/LLVMIR/Import/frame-pointer.ll b/mlir/test/Target/LLVMIR/Import/frame-pointer.ll
index a51cc8581db9c..1f3fa0e54b79f 100644
--- a/mlir/test/Target/LLVMIR/Import/frame-pointer.ll
+++ b/mlir/test/Target/LLVMIR/Import/frame-pointer.ll
@@ -1,8 +1,9 @@
 ; RUN: mlir-translate -import-llvm -split-input-file %s | FileCheck %s
 
+; CHECK-LABEL: llvm.func @frame_pointer_func
+; CHECK-SAME: attributes {frame_pointer = #llvm.framePointerKind<"non-leaf">}
+
 define void @frame_pointer_func() "frame-pointer"="non-leaf" {
-  ; CHECK: llvm.func @frame_pointer_func()
-  ; CHECK-SAME: attributes {frame_pointer = #llvm.framePointerKind<"non-leaf">}
   
   ret void
 }
diff --git a/mlir/test/Target/LLVMIR/frame-pointer.mlir b/mlir/test/Target/LLVMIR/frame-pointer.mlir
index 3ee347eeebb01..5224e97a40805 100644
--- a/mlir/test/Target/LLVMIR/frame-pointer.mlir
+++ b/mlir/test/Target/LLVMIR/frame-pointer.mlir
@@ -1,9 +1,8 @@
 // RUN: mlir-translate -mlir-to-llvmir %s | FileCheck %s
 
+// CHECK-LABEL: define void @frame_pointer_func() 
+// CHECK-SAME: #[[ATTRS:[0-9]+]]
 llvm.func @frame_pointer_func() attributes {frame_pointer = #llvm.framePointerKind<"non-leaf">}  {
-  // CHECK-LABEL: define void @frame_pointer_func() 
-  // CHECK-SAME: #[[ATTRS:[0-9]+]]
-  // CHECK: attributes #[[ATTRS]] = { "frame-pointer"="non-leaf" }
   llvm.return
 }
-
+// CHECK: attributes #[[ATTRS]] = { "frame-pointer"="non-leaf" }

>From 19f2ede4166d15ef324e0f710035e76010a2d9cd Mon Sep 17 00:00:00 2001
From: Radu2k <radu.salavat at arm.com>
Date: Mon, 4 Dec 2023 12:09:36 +0000
Subject: [PATCH 7/8] Make appropriate changes after rebase

---
 mlir/lib/Target/LLVMIR/ModuleImport.cpp         | 1 +
 mlir/test/Target/LLVMIR/Import/frame-pointer.ll | 2 +-
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/mlir/lib/Target/LLVMIR/ModuleImport.cpp b/mlir/lib/Target/LLVMIR/ModuleImport.cpp
index 943c81ac5b5d7..d43ddc008a8f3 100644
--- a/mlir/lib/Target/LLVMIR/ModuleImport.cpp
+++ b/mlir/lib/Target/LLVMIR/ModuleImport.cpp
@@ -1614,6 +1614,7 @@ static constexpr std::array ExplicitAttributes{
     StringLiteral("aarch64_pstate_sm_body"),
     StringLiteral("aarch64_pstate_za_new"),
     StringLiteral("vscale_range"),
+    StringLiteral("frame_pointer"),
 };
 
 static void processPassthroughAttrs(llvm::Function *func, LLVMFuncOp funcOp) {
diff --git a/mlir/test/Target/LLVMIR/Import/frame-pointer.ll b/mlir/test/Target/LLVMIR/Import/frame-pointer.ll
index 1f3fa0e54b79f..c6836106672fc 100644
--- a/mlir/test/Target/LLVMIR/Import/frame-pointer.ll
+++ b/mlir/test/Target/LLVMIR/Import/frame-pointer.ll
@@ -1,7 +1,7 @@
 ; RUN: mlir-translate -import-llvm -split-input-file %s | FileCheck %s
 
 ; CHECK-LABEL: llvm.func @frame_pointer_func
-; CHECK-SAME: attributes {frame_pointer = #llvm.framePointerKind<"non-leaf">}
+; CHECK-SAME: attributes {frame_pointer = #llvm.framePointerKind<"non-leaf">
 
 define void @frame_pointer_func() "frame-pointer"="non-leaf" {
   

>From 446a16bc1b08f9ae19a46bac7576a2d18f956146 Mon Sep 17 00:00:00 2001
From: Radu2k <radu.salavat at arm.com>
Date: Mon, 4 Dec 2023 13:50:10 +0000
Subject: [PATCH 8/8] Add 'frame-pointer' to ExplicitAttributes and adjust test

---
 mlir/include/mlir/Dialect/LLVMIR/LLVMOps.td     | 1 -
 mlir/lib/Target/LLVMIR/ModuleImport.cpp         | 2 +-
 mlir/test/Target/LLVMIR/Import/frame-pointer.ll | 2 +-
 3 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/mlir/include/mlir/Dialect/LLVMIR/LLVMOps.td b/mlir/include/mlir/Dialect/LLVMIR/LLVMOps.td
index 94f2f802fcf22..370dc2c376a8d 100644
--- a/mlir/include/mlir/Dialect/LLVMIR/LLVMOps.td
+++ b/mlir/include/mlir/Dialect/LLVMIR/LLVMOps.td
@@ -1392,7 +1392,6 @@ def LLVM_LLVMFuncOp : LLVM_Op<"func", [
     OptionalAttr<UnnamedAddr>:$unnamed_addr,
     OptionalAttr<I64Attr>:$alignment,
     OptionalAttr<LLVM_VScaleRangeAttr>:$vscale_range,
-    //DefaultValuedAttr<FramePointerKindAttr, "mlir::LLVM::FramePointerKind::None">:$frame_pointer
     OptionalAttr<FramePointerKindAttr>:$frame_pointer
   );
 
diff --git a/mlir/lib/Target/LLVMIR/ModuleImport.cpp b/mlir/lib/Target/LLVMIR/ModuleImport.cpp
index d43ddc008a8f3..36b74d0cf4c1e 100644
--- a/mlir/lib/Target/LLVMIR/ModuleImport.cpp
+++ b/mlir/lib/Target/LLVMIR/ModuleImport.cpp
@@ -1614,7 +1614,7 @@ static constexpr std::array ExplicitAttributes{
     StringLiteral("aarch64_pstate_sm_body"),
     StringLiteral("aarch64_pstate_za_new"),
     StringLiteral("vscale_range"),
-    StringLiteral("frame_pointer"),
+    StringLiteral("frame-pointer"),
 };
 
 static void processPassthroughAttrs(llvm::Function *func, LLVMFuncOp funcOp) {
diff --git a/mlir/test/Target/LLVMIR/Import/frame-pointer.ll b/mlir/test/Target/LLVMIR/Import/frame-pointer.ll
index c6836106672fc..1f3fa0e54b79f 100644
--- a/mlir/test/Target/LLVMIR/Import/frame-pointer.ll
+++ b/mlir/test/Target/LLVMIR/Import/frame-pointer.ll
@@ -1,7 +1,7 @@
 ; RUN: mlir-translate -import-llvm -split-input-file %s | FileCheck %s
 
 ; CHECK-LABEL: llvm.func @frame_pointer_func
-; CHECK-SAME: attributes {frame_pointer = #llvm.framePointerKind<"non-leaf">
+; CHECK-SAME: attributes {frame_pointer = #llvm.framePointerKind<"non-leaf">}
 
 define void @frame_pointer_func() "frame-pointer"="non-leaf" {
   



More information about the Mlir-commits mailing list