[Mlir-commits] [mlir] [MLIR][LLVM] Fix CallOp asm parser for attr-dict (PR #74372)

[Billy Zhu]

https://github.com/zyx-billy updated https://github.com/llvm/llvm-project/pull/74372

>From 53d8a021d584551419b97db794a318ac6c29b826 Mon Sep 17 00:00:00 2001
From: Billy Zhu <billyzhu at modular.com>
Date: Mon, 4 Dec 2023 13:30:18 -0800
Subject: [PATCH 1/3] fix attr dict parse

---
 mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp | 6 ++++--
 mlir/test/Target/LLVMIR/llvmir.mlir        | 4 ++++
 2 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp b/mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp
index 28445945f07d6..4c3367336fa79 100644
--- a/mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp
+++ b/mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp
@@ -1219,8 +1219,7 @@ ParseResult CallOp::parse(OpAsmParser &parser, OperationState &result) {
       return failure();
 
   // Parse the function arguments.
-  if (parser.parseOperandList(operands, OpAsmParser::Delimiter::Paren) ||
-      parser.parseOptionalAttrDict(result.attributes))
+  if (parser.parseOperandList(operands, OpAsmParser::Delimiter::Paren))
     return failure();
 
   bool isVarArg = parser.parseOptionalKeyword("vararg").succeeded();
@@ -1232,6 +1231,9 @@ ParseResult CallOp::parse(OpAsmParser &parser, OperationState &result) {
       return failure();
   }
 
+  if (parser.parseOptionalAttrDict(result.attributes))
+    return failure();
+
   // Parse the trailing type list and resolve the operands.
   return parseCallTypeAndResolveOperands(parser, result, isDirect, operands);
 }
diff --git a/mlir/test/Target/LLVMIR/llvmir.mlir b/mlir/test/Target/LLVMIR/llvmir.mlir
index 3f84f9dc5a9b8..3ec8aa0e8bb12 100644
--- a/mlir/test/Target/LLVMIR/llvmir.mlir
+++ b/mlir/test/Target/LLVMIR/llvmir.mlir
@@ -1227,15 +1227,19 @@ llvm.func @varargs(...)
 
 // CHECK-LABEL: define void @varargs_call
 llvm.func @varargs_call(%arg0 : i32) {
+// CHECK:  call void (...) @varargs(i32 %{{.*}})
 // CHECK:  call void (...) @varargs(i32 %{{.*}})
   llvm.call @varargs(%arg0) vararg(!llvm.func<void (...)>) : (i32) -> ()
+  llvm.call @varargs(%arg0) vararg(!llvm.func<void (...)>) {fastmathFlags = #llvm.fastmath<none>} : (i32) -> ()
   llvm.return
 }
 
 // CHECK-LABEL: define void @indirect_varargs_call(ptr %0, i32 %1)
 llvm.func @indirect_varargs_call(%arg0 : !llvm.ptr, %arg1 : i32) {
+// CHECK:  call void (...) %0(i32 %1)
 // CHECK:  call void (...) %0(i32 %1)
   llvm.call %arg0(%arg1) vararg(!llvm.func<void (...)>) : !llvm.ptr, (i32) -> ()
+  llvm.call %arg0(%arg1) vararg(!llvm.func<void (...)>) {fastmathFlags = #llvm.fastmath<none>} : !llvm.ptr, (i32) -> ()
   llvm.return
 }
 

>From 31cbcc9ae8116f1a23dbc9405d46f2749d19de47 Mon Sep 17 00:00:00 2001
From: Billy Zhu <billyzhu at modular.com>
Date: Mon, 4 Dec 2023 22:59:12 -0800
Subject: [PATCH 2/3] make attrs show up in the conversion

---
 mlir/test/Target/LLVMIR/llvmir.mlir | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/mlir/test/Target/LLVMIR/llvmir.mlir b/mlir/test/Target/LLVMIR/llvmir.mlir
index 3ec8aa0e8bb12..d9e7b790dd801 100644
--- a/mlir/test/Target/LLVMIR/llvmir.mlir
+++ b/mlir/test/Target/LLVMIR/llvmir.mlir
@@ -1223,23 +1223,23 @@ llvm.func @dereferenceableornullattr_ret_decl() -> (!llvm.ptr {llvm.dereferencea
 llvm.func @inregattr_ret_decl() -> (!llvm.ptr {llvm.inreg})
 
 // CHECK-LABEL: @varargs(...)
-llvm.func @varargs(...)
+llvm.func @varargs(...) -> f32
 
 // CHECK-LABEL: define void @varargs_call
 llvm.func @varargs_call(%arg0 : i32) {
-// CHECK:  call void (...) @varargs(i32 %{{.*}})
-// CHECK:  call void (...) @varargs(i32 %{{.*}})
-  llvm.call @varargs(%arg0) vararg(!llvm.func<void (...)>) : (i32) -> ()
-  llvm.call @varargs(%arg0) vararg(!llvm.func<void (...)>) {fastmathFlags = #llvm.fastmath<none>} : (i32) -> ()
+// CHECK:  call float (...) @varargs(i32 %{{.*}})
+// CHECK:  call nnan float (...) @varargs(i32 %{{.*}})
+  llvm.call @varargs(%arg0) vararg(!llvm.func<f32 (...)>) : (i32) -> (f32)
+  llvm.call @varargs(%arg0) vararg(!llvm.func<f32 (...)>) {fastmathFlags = #llvm.fastmath<nnan>} : (i32) -> (f32)
   llvm.return
 }
 
 // CHECK-LABEL: define void @indirect_varargs_call(ptr %0, i32 %1)
 llvm.func @indirect_varargs_call(%arg0 : !llvm.ptr, %arg1 : i32) {
-// CHECK:  call void (...) %0(i32 %1)
-// CHECK:  call void (...) %0(i32 %1)
-  llvm.call %arg0(%arg1) vararg(!llvm.func<void (...)>) : !llvm.ptr, (i32) -> ()
-  llvm.call %arg0(%arg1) vararg(!llvm.func<void (...)>) {fastmathFlags = #llvm.fastmath<none>} : !llvm.ptr, (i32) -> ()
+// CHECK:  call float (...) %0(i32 %1)
+// CHECK:  call nnan float (...) %0(i32 %1)
+  llvm.call %arg0(%arg1) vararg(!llvm.func<f32 (...)>) : !llvm.ptr, (i32) -> (f32)
+  llvm.call %arg0(%arg1) vararg(!llvm.func<f32 (...)>) {fastmathFlags = #llvm.fastmath<nnan>} : !llvm.ptr, (i32) -> (f32)
   llvm.return
 }
 

>From 5bfe3090c2867a9ff8d470073dc0d37bbf7d29e8 Mon Sep 17 00:00:00 2001
From: Billy Zhu <billyzhu at modular.com>
Date: Mon, 4 Dec 2023 23:58:04 -0800
Subject: [PATCH 3/3] add roundtrip test

---
 mlir/test/Dialect/LLVMIR/roundtrip.mlir | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/mlir/test/Dialect/LLVMIR/roundtrip.mlir b/mlir/test/Dialect/LLVMIR/roundtrip.mlir
index ee724a482cfb5..42589972ef2ba 100644
--- a/mlir/test/Dialect/LLVMIR/roundtrip.mlir
+++ b/mlir/test/Dialect/LLVMIR/roundtrip.mlir
@@ -73,11 +73,15 @@ func.func @ops(%arg0: i32, %arg1: f32,
 
 // Variadic calls
 // CHECK:  llvm.call @vararg_func(%arg0, %arg0) vararg(!llvm.func<void (i32, ...)>) : (i32, i32) -> ()
+// CHECK:  llvm.call @vararg_func(%arg0, %arg0) vararg(!llvm.func<void (i32, ...)>) {fastmathFlags = #llvm.fastmath<fast>} : (i32, i32) -> ()
 // CHECK:  %[[VARIADIC_FUNC:.*]] = llvm.mlir.addressof @vararg_func : !llvm.ptr
 // CHECK:  llvm.call %[[VARIADIC_FUNC]](%[[I32]], %[[I32]]) vararg(!llvm.func<void (i32, ...)>) : !llvm.ptr, (i32, i32) -> ()
+// CHECK:  llvm.call %[[VARIADIC_FUNC]](%[[I32]], %[[I32]]) vararg(!llvm.func<void (i32, ...)>) {fastmathFlags = #llvm.fastmath<fast>} : !llvm.ptr, (i32, i32) -> ()
   llvm.call @vararg_func(%arg0, %arg0) vararg(!llvm.func<void (i32, ...)>) : (i32, i32) -> ()
+  llvm.call @vararg_func(%arg0, %arg0) vararg(!llvm.func<void (i32, ...)>) {fastmathFlags = #llvm.fastmath<fast>} : (i32, i32) -> ()
   %variadic_func = llvm.mlir.addressof @vararg_func : !llvm.ptr
   llvm.call %variadic_func(%arg0, %arg0) vararg(!llvm.func<void (i32, ...)>) : !llvm.ptr, (i32, i32) -> ()
+  llvm.call %variadic_func(%arg0, %arg0) vararg(!llvm.func<void (i32, ...)>) {fastmathFlags = #llvm.fastmath<fast>} : !llvm.ptr, (i32, i32) -> ()
 
 // Terminator operations and their successors.
 //