Fix handling of integer template argument in emitc.call_opaque

[patel-vimal]

Integer attributes supplied to emitc.call_opaque as arguments were treated as index into the operands list. This should be the case only for the normal arguments but not for the template arguments which can't refer to SSA values. This commit updates the handling of template arguments in mlir-to-cpp by removing special handling of integer attributes.

[llvmbot]

@llvm/pr-subscribers-mlir

@llvm/pr-subscribers-mlir-emitc

Author: Vimal (patel-vimal)

Changes

Integer attributes supplied to emitc.call_opaque as arguments were treated as index into the operands list. This should be the case only for the normal arguments but not for the template arguments which can't refer to SSA values. This commit updates the handling of template arguments in mlir-to-cpp by removing special handling of integer attributes.

---

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

2 Files Affected:

• (modified) mlir/lib/Target/Cpp/TranslateToCpp.cpp (+16-8)
• (modified) mlir/test/Target/Cpp/common-cpp.mlir (+6)

diff --git a/mlir/lib/Target/Cpp/TranslateToCpp.cpp b/mlir/lib/Target/Cpp/TranslateToCpp.cpp
index 0c4975a13d301..5abc112ab8c7a 100644
--- a/mlir/lib/Target/Cpp/TranslateToCpp.cpp
+++ b/mlir/lib/Target/Cpp/TranslateToCpp.cpp
@@ -692,6 +692,22 @@ static LogicalResult printOperation(CppEmitter &emitter,
     return failure();
   os << callOpaqueOp.getCallee();
 
+  // Template arguments can't refer to SSA values and as such the template
+  // arguments which are supplied in form of attributes can be emitted as is. We
+  // don't need to handle integer attributes specially like we do for arguments
+  // - see below.
+  auto emitTemplateArgs = [&](Attribute attr) -> LogicalResult {
+    return emitter.emitAttribute(op.getLoc(), attr);
+  };
+
+  if (callOpaqueOp.getTemplateArgs()) {
+    os << "<";
+    if (failed(interleaveCommaWithError(*callOpaqueOp.getTemplateArgs(), os,
+                                        emitTemplateArgs)))
+      return failure();
+    os << ">";
+  }
+
   auto emitArgs = [&](Attribute attr) -> LogicalResult {
     if (auto t = dyn_cast<IntegerAttr>(attr)) {
       // Index attributes are treated specially as operand index.
@@ -711,14 +727,6 @@ static LogicalResult printOperation(CppEmitter &emitter,
     return success();
   };
 
-  if (callOpaqueOp.getTemplateArgs()) {
-    os << "<";
-    if (failed(interleaveCommaWithError(*callOpaqueOp.getTemplateArgs(), os,
-                                        emitArgs)))
-      return failure();
-    os << ">";
-  }
-
   os << "(";
 
   LogicalResult emittedArgs =
diff --git a/mlir/test/Target/Cpp/common-cpp.mlir b/mlir/test/Target/Cpp/common-cpp.mlir
index 45fef618621cc..960d7c824e9e0 100644
--- a/mlir/test/Target/Cpp/common-cpp.mlir
+++ b/mlir/test/Target/Cpp/common-cpp.mlir
@@ -109,3 +109,9 @@ func.func @apply() -> !emitc.ptr<i32> {
 func.func @array_type(%arg0: !emitc.array<3xi32>, %arg1: !emitc.array<10x20xf32>) {
   return
 }
+
+// CHECK: call_opaque_with_template_arg
+func.func @call_opaque_with_template_arg() {
+  emitc.call_opaque "init_tile"() {template_args = [512 : index]} : () -> ()
+  return
+}

[patel-vimal]

@EugeneZelenko A gentle request to add reviewers when you get a chance.

[jacquesguan]

LGTM, except the check lines in test.

[patel-vimal]

LGTM, except the check lines in test.

Sure. I'll add the check lines for the output.

[patel-vimal]

@jacquesguan Thanks for the review. I've handled the review comments. I don't have write access. Could you please merge this?

[jacquesguan]

@jacquesguan Thanks for the review. I've handled the review comments. I don't have write access. Could you please merge this?

No problem, I will merge this.