[mlir][spirv] Verify matching of entry block arguments and function signature

[hiraditya]

Fixes: #132894

[llvmbot]

@llvm/pr-subscribers-mlir-spirv

@llvm/pr-subscribers-mlir

Author: AdityaK (hiraditya)

Changes

Fixes: #132894

---

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

2 Files Affected:

• (modified) mlir/lib/Interfaces/CallInterfaces.cpp (+3-2)
• (added) mlir/test/mlir-opt/print-region.mlir (+17)

diff --git a/mlir/lib/Interfaces/CallInterfaces.cpp b/mlir/lib/Interfaces/CallInterfaces.cpp
index e8ed4b339a0cb..815b906d7d0f2 100644
--- a/mlir/lib/Interfaces/CallInterfaces.cpp
+++ b/mlir/lib/Interfaces/CallInterfaces.cpp
@@ -116,9 +116,10 @@ void call_interface_impl::printFunctionSignature(
 
     if (!isExternal) {
       ArrayRef<NamedAttribute> attrs;
-      if (argAttrs)
+      if (argAttrs) {
         attrs = llvm::cast<DictionaryAttr>(argAttrs[i]).getValue();
-      p.printRegionArgument(body->getArgument(i), attrs);
+        p.printRegionArgument(body->getArgument(i), attrs);
+      }
     } else {
       p.printType(argTypes[i]);
       if (argAttrs)
diff --git a/mlir/test/mlir-opt/print-region.mlir b/mlir/test/mlir-opt/print-region.mlir
new file mode 100644
index 0000000000000..678457defff57
--- /dev/null
+++ b/mlir/test/mlir-opt/print-region.mlir
@@ -0,0 +1,17 @@
+// Bug: https://github.com/llvm/llvm-project/issues/132894
+
+// RUN: mlir-opt %s | FileCheck
+
+// CHECK: module {
+// CHECK:   spirv.func @f(f32) "None" {
+// CHECK:     %c0 = arith.constant 0 : index
+// CHECK:     spirv.Return
+// CHECK:   }
+// CHECK: }
+
+module {
+  spirv.func @f(f32) "None" {
+    %c0 = arith.constant 0 : index
+    spirv.Return
+  }
+}

[CoTinker]

The FunctionOpInterface implement the verifyBody and check the arguments number.

llvm-project/mlir/include/mlir/Interfaces/FunctionInterfaces.td

Lines 86 to 99 in dad86f5

	"::llvm::LogicalResult", "verifyBody", (ins),
	/*methodBody=*/[{}], /*defaultImplementation=*/[{
	if ($_op.isExternal())
	return success();
	ArrayRef<Type> fnInputTypes = $_op.getArgumentTypes();
	// NOTE: This should just be $_op.front() but access generically
	// because the interface methods defined here may be shadowed in
	// arbitrary ways. https://github.com/llvm/llvm-project/issues/54807
	Block &entryBlock = $_op->getRegion(0).front();
	unsigned numArguments = fnInputTypes.size();
	if (entryBlock.getNumArguments() != numArguments)
	return $_op.emitOpError("entry block must have ")
	<< numArguments << " arguments to match function signature";

The spirv.func op override the verifyBody funciton, but not check the arguments, causing this crash.

llvm-project/mlir/lib/Dialect/SPIRV/IR/SPIRVOps.cpp

Line 1022 in dad86f5

LogicalResult spirv::FuncOp::verifyBody() {

[github-actions]

✅ With the latest revision this PR passed the C/C++ code formatter.

[joker-eph]

LGTM, @kuhar any more comments here?

[kuhar]

Thanks for fixing this

[kuhar]

LGTM