Skip to content

[mlir][bufferize] Make buffer-results-to-out-params support only functions that are neither public nor extern #162441

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Oct 8, 2025
Merged

[mlir][bufferize] Make buffer-results-to-out-params support only functions that are neither public nor extern #162441

merged 1 commit into from
Oct 8, 2025

Conversation

@linuxlonelyeagle
Copy link
Member

The callers of public or extern functions are unknown, so their function signatures cannot be changed.

@llvmbot llvmbot added mlir mlir:bufferization Bufferization infrastructure mlir:emitc labels Oct 8, 2025
@llvmbot
Copy link
Member

llvmbot commented Oct 8, 2025 •
edited
Loading

@llvm/pr-subscribers-mlir-emitc

@llvm/pr-subscribers-mlir

Author: lonely eagle (linuxlonelyeagle)

Changes

The callers of public or extern functions are unknown, so their function signatures cannot be changed.

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

6 Files Affected:

  • (modified) mlir/lib/Dialect/Bufferization/Transforms/BufferResultsToOutParams.cpp (+5-2)
  • (modified) mlir/test/Conversion/ConvertToEmitC/tosa.mlir (+2-2)
  • (modified) mlir/test/Dialect/Bufferization/Transforms/one-shot-module-bufferize-out-params.mlir (+8-8)
  • (modified) mlir/test/Transforms/buffer-results-to-out-params-add-result-attr.mlir (+2-2)
  • (modified) mlir/test/Transforms/buffer-results-to-out-params-hosit-static-allocs.mlir (+8-8)
  • (modified) mlir/test/Transforms/buffer-results-to-out-params.mlir (+25-26)
diff --git a/mlir/lib/Dialect/Bufferization/Transforms/BufferResultsToOutParams.cpp b/mlir/lib/Dialect/Bufferization/Transforms/BufferResultsToOutParams.cpp
index 25f941dc16516..b9ee0a4d401f3 100644
--- a/mlir/lib/Dialect/Bufferization/Transforms/BufferResultsToOutParams.cpp
+++ b/mlir/lib/Dialect/Bufferization/Transforms/BufferResultsToOutParams.cpp
@@ -217,6 +217,9 @@ updateCalls(ModuleOp module, const AllocDynamicSizesMap &map,
     }
     if (!options.filterFn(&callee))
       return;
+    if (callee.isExternal() || callee.isPublic())
+      return;
+
     SmallVector<Value, 6> replaceWithNewCallResults;
     SmallVector<Value, 6> replaceWithOutParams;
     for (OpResult result : op.getResults()) {
@@ -292,14 +295,14 @@ LogicalResult mlir::bufferization::promoteBufferResultsToOutParams(
   // function.
   AllocDynamicSizesMap map;
   for (auto func : module.getOps<func::FuncOp>()) {
+    if (func.isExternal() || func.isPublic())
+      continue;
     if (!options.filterFn(&func))
       continue;
     SmallVector<BlockArgument, 6> appendedEntryArgs;
     if (failed(
             updateFuncOp(func, appendedEntryArgs, options.addResultAttribute)))
       return failure();
-    if (func.isExternal())
-      continue;
     if (failed(updateReturnOps(func, appendedEntryArgs, map, options))) {
       return failure();
     }
diff --git a/mlir/test/Conversion/ConvertToEmitC/tosa.mlir b/mlir/test/Conversion/ConvertToEmitC/tosa.mlir
index 8ced05eced4b9..158018374e72c 100644
--- a/mlir/test/Conversion/ConvertToEmitC/tosa.mlir
+++ b/mlir/test/Conversion/ConvertToEmitC/tosa.mlir
@@ -20,7 +20,7 @@
 // RUN: mlir-opt --pass-pipeline=%{pipeline} %s | FileCheck %s
 // -----
 
-//      CHECK: emitc.func @main(%[[ARG0:.*]]: !emitc.array<2xf32>, %[[ARG1:.*]]: !emitc.array<2xf32>, %[[RES:.*]]: !emitc.array<2xf32>) {
+//      CHECK: emitc.func private @main(%[[ARG0:.*]]: !emitc.array<2xf32>, %[[ARG1:.*]]: !emitc.array<2xf32>, %[[RES:.*]]: !emitc.array<2xf32>)
 //  CHECK-DAG:   %[[C0:.*]] = "emitc.constant"() <{value = 0 : index}> : () -> !emitc.size_t
 //  CHECK-DAG:   %[[C1:.*]] = "emitc.constant"() <{value = 1 : index}> : () -> !emitc.size_t
 //  CHECK-DAG:   %[[C2:.*]] = "emitc.constant"() <{value = 2 : index}> : () -> !emitc.size_t
@@ -35,7 +35,7 @@
 // CHECK-NEXT:   }
 // CHECK-NEXT:   return
 // CHECK-NEXT: }
-func.func @main(%arg0: tensor<2xf32>, %arg1: tensor<2xf32>) -> tensor<2xf32> {
+func.func private @main(%arg0: tensor<2xf32>, %arg1: tensor<2xf32>) -> tensor<2xf32> {
   %0 = tosa.add %arg0, %arg1 : (tensor<2xf32>, tensor<2xf32>) -> tensor<2xf32>
   return %0 : tensor<2xf32>
 }
diff --git a/mlir/test/Dialect/Bufferization/Transforms/one-shot-module-bufferize-out-params.mlir b/mlir/test/Dialect/Bufferization/Transforms/one-shot-module-bufferize-out-params.mlir
index 9cf44c335d551..75e9a8926ad15 100644
--- a/mlir/test/Dialect/Bufferization/Transforms/one-shot-module-bufferize-out-params.mlir
+++ b/mlir/test/Dialect/Bufferization/Transforms/one-shot-module-bufferize-out-params.mlir
@@ -7,7 +7,7 @@
 
 // Note: This bufferization is not very efficient yet, but it works.
 
-// CHECK-LABEL: func @callee(
+// CHECK-LABEL: func private @callee(
 //  CHECK-SAME:              %[[arg0:.*]]: memref<5xf32, strided<[?], offset: ?>>,
 //  CHECK-SAME:              %[[arg1:.*]]: memref<5xf32, strided<[?], offset: ?>>) {
 // This alloc is not needed, but it is inserted due to the out-of-place
@@ -21,7 +21,7 @@
 //       CHECK:   return
 //       CHECK: }
 
-// CHECK-NO-LAYOUT-LABEL: func @callee(
+// CHECK-NO-LAYOUT-LABEL: func private @callee(
 //  CHECK-NO-LAYOUT-SAME:     %[[arg0:.*]]: memref<5xf32>,
 //  CHECK-NO-LAYOUT-SAME:     %[[arg1:.*]]: memref<5xf32>) {
 //       CHECK-NO-LAYOUT:   %[[alloc:.*]] = memref.alloc() {{.*}} : memref<5xf32>
@@ -29,13 +29,13 @@
 //       CHECK-NO-LAYOUT:   memref.store {{.*}}, %[[alloc]]
 //       CHECK-NO-LAYOUT:   memref.copy %[[alloc]], %[[arg1]]
 
-// CHECK-BASELINE-LABEL: func @callee(
+// CHECK-BASELINE-LABEL: func private @callee(
 //  CHECK-BASELINE-SAME:     %[[arg0:.*]]: memref<5xf32, strided<[?], offset: ?>>) -> memref<5xf32> {
 //       CHECK-BASELINE:   %[[alloc:.*]] = memref.alloc() {{.*}} : memref<5xf32>
 //       CHECK-BASELINE:   memref.copy %[[arg0]], %[[alloc]]
 //       CHECK-BASELINE:   memref.store {{.*}}, %[[alloc]]
 //       CHECK-BASELINE:   return %[[alloc]]
-func.func @callee(%t: tensor<5xf32>) -> (tensor<5xf32>, tensor<5xf32>) {
+func.func private @callee(%t: tensor<5xf32>) -> (tensor<5xf32>, tensor<5xf32>) {
   %c0 = arith.constant 0 : index
   %cst = arith.constant 8.0 : f32
   // This must bufferize out-of-place.
@@ -68,7 +68,7 @@ func.func @main(%t: tensor<5xf32>) -> (f32, f32) {
 
 // -----
 
-// CHECK-LABEL: func @callee(
+// CHECK-LABEL: func private @callee(
 //  CHECK-SAME:     %{{.*}}: index,
 //  CHECK-SAME:     %[[r:.*]]: memref<2x5xf32, strided<[?, ?], offset: ?>>) {
 //       CHECK:   %[[alloc:.*]] = memref.alloc() {{.*}} : memref<10x20xf32>
@@ -76,7 +76,7 @@ func.func @main(%t: tensor<5xf32>) -> (f32, f32) {
 //       CHECK:   %[[casted:.*]] = memref.cast %[[subview]]
 //       CHECK:   memref.copy %[[casted]], %[[r]]
 
-// CHECK-NO-LAYOUT-LABEL: func @callee(
+// CHECK-NO-LAYOUT-LABEL: func private @callee(
 //  CHECK-NO-LAYOUT-SAME:              %{{.*}}: index,
 //  CHECK-NO-LAYOUT-SAME:              %[[r:.*]]: memref<2x5xf32>) {
 //       CHECK-NO-LAYOUT:   %[[alloc:.*]] = memref.alloc() {{.*}} : memref<10x20xf32>
@@ -88,12 +88,12 @@ func.func @main(%t: tensor<5xf32>) -> (f32, f32) {
 //       CHECK-NO-LAYOUT:   memref.copy %[[subview]], %[[alloc2]]
 //       CHECK-NO-LAYOUT:   memref.copy %[[alloc2]], %[[r]]
 
-// CHECK-BASELINE-LABEL: func @callee(
+// CHECK-BASELINE-LABEL: func private @callee(
 //  CHECK-BASELINE-SAME:     %{{.*}}: index) -> memref<2x5xf32, strided<[20, 1], offset: ?>> {
 //       CHECK-BASELINE:   %[[alloc:.*]] = memref.alloc() {{.*}} : memref<10x20xf32>
 //       CHECK-BASELINE:   %[[subview:.*]] = memref.subview %[[alloc]]
 //       CHECK-BASELINE:   return %[[subview]]
-func.func @callee(%idx: index) -> tensor<2x5xf32> {
+func.func private @callee(%idx: index) -> tensor<2x5xf32> {
   %0 = bufferization.alloc_tensor() : tensor<10x20xf32>
   %1 = tensor.extract_slice %0[%idx, %idx][2, 5][1, 1] : tensor<10x20xf32> to tensor<2x5xf32>
   return %1 : tensor<2x5xf32>
diff --git a/mlir/test/Transforms/buffer-results-to-out-params-add-result-attr.mlir b/mlir/test/Transforms/buffer-results-to-out-params-add-result-attr.mlir
index f4a95c73e2953..8faacdb00d556 100644
--- a/mlir/test/Transforms/buffer-results-to-out-params-add-result-attr.mlir
+++ b/mlir/test/Transforms/buffer-results-to-out-params-add-result-attr.mlir
@@ -1,7 +1,7 @@
 // RUN: mlir-opt -p 'builtin.module(buffer-results-to-out-params{add-result-attr})' -split-input-file -verify-diagnostics %s | FileCheck %s
 
 // CHECK-LABEL: @basic({{.*}}: memref<f32> {bufferize.result})
-func.func @basic() -> (memref<f32>) {
+func.func private @basic() -> (memref<f32>) {
   %0 = "test.source"() : () -> (memref<f32>)
   return %0 : memref<f32>
 }
@@ -11,7 +11,7 @@ func.func @basic() -> (memref<f32>) {
 // CHECK-LABEL: multiple_results
 // CHECK-SAME:  memref<1xf32> {bufferize.result}
 // CHECK-SAME:  memref<2xf32> {bufferize.result}
-func.func @multiple_results() -> (memref<1xf32>, memref<2xf32>) {
+func.func private @multiple_results() -> (memref<1xf32>, memref<2xf32>) {
   %0, %1 = "test.source"() : () -> (memref<1xf32>, memref<2xf32>)
   return %0, %1 : memref<1xf32>, memref<2xf32>
 }
diff --git a/mlir/test/Transforms/buffer-results-to-out-params-hosit-static-allocs.mlir b/mlir/test/Transforms/buffer-results-to-out-params-hosit-static-allocs.mlir
index 2783836a09e16..97fb9f45f57f8 100644
--- a/mlir/test/Transforms/buffer-results-to-out-params-hosit-static-allocs.mlir
+++ b/mlir/test/Transforms/buffer-results-to-out-params-hosit-static-allocs.mlir
@@ -1,36 +1,36 @@
 // RUN: mlir-opt -allow-unregistered-dialect -p 'builtin.module(buffer-results-to-out-params{hoist-static-allocs})'  %s | FileCheck %s
 
-// CHECK-LABEL:   func @basic(
+// CHECK-LABEL:   func private @basic(
 // CHECK-SAME:                %[[ARG:.*]]: memref<8x64xf32>) {
 // CHECK-NOT:        memref.alloc()
 // CHECK:           "test.source"(%[[ARG]])  : (memref<8x64xf32>) -> ()
 // CHECK:           return
 // CHECK:         }
-func.func @basic() -> (memref<8x64xf32>) {
+func.func private @basic() -> (memref<8x64xf32>) {
   %b = memref.alloc() : memref<8x64xf32>
   "test.source"(%b)  : (memref<8x64xf32>) -> ()
   return %b : memref<8x64xf32>
 }
 
-// CHECK-LABEL:   func @basic_no_change(
+// CHECK-LABEL:   func private @basic_no_change(
 // CHECK-SAME:                %[[ARG:.*]]: memref<f32>) {
 // CHECK:           %[[RESULT:.*]] = "test.source"() : () -> memref<f32>
 // CHECK:           memref.copy %[[RESULT]], %[[ARG]]  : memref<f32> to memref<f32>
 // CHECK:           return
 // CHECK:         }
-func.func @basic_no_change() -> (memref<f32>) {
+func.func private @basic_no_change() -> (memref<f32>) {
   %0 = "test.source"() : () -> (memref<f32>)
   return %0 : memref<f32>
 }
 
-// CHECK-LABEL:   func @basic_dynamic(
+// CHECK-LABEL:   func private @basic_dynamic(
 // CHECK-SAME:                %[[D:.*]]: index, %[[ARG:.*]]: memref<?xf32>) {
 // CHECK:           %[[RESULT:.*]] = memref.alloc(%[[D]]) : memref<?xf32>
 // CHECK:           "test.source"(%[[RESULT]])  : (memref<?xf32>) -> ()
 // CHECK:           memref.copy %[[RESULT]], %[[ARG]]
 // CHECK:           return
 // CHECK:         }
-func.func @basic_dynamic(%d: index) -> (memref<?xf32>) {
+func.func private @basic_dynamic(%d: index) -> (memref<?xf32>) {
   %b = memref.alloc(%d) : memref<?xf32>
   "test.source"(%b)  : (memref<?xf32>) -> ()
   return %b : memref<?xf32>
@@ -39,13 +39,13 @@ func.func @basic_dynamic(%d: index) -> (memref<?xf32>) {
 // -----
 
 // no change due to writing to func args
-// CHECK-LABEL:   func @return_arg(
+// CHECK-LABEL:   func private @return_arg(
 // CHECK-SAME:        %[[ARG0:.*]]: memref<128x256xf32>, %[[ARG1:.*]]: memref<128x256xf32>, %[[ARG2:.*]]: memref<128x256xf32>) {
 // CHECK:           "test.source"(%[[ARG0]], %[[ARG1]])
 // CHECK:           memref.copy
 // CHECK:           return
 // CHECK:         }
-func.func @return_arg(%arg0: memref<128x256xf32>, %arg1: memref<128x256xf32>) -> memref<128x256xf32> {
+func.func private @return_arg(%arg0: memref<128x256xf32>, %arg1: memref<128x256xf32>) -> memref<128x256xf32> {
   "test.source"(%arg0, %arg1)  : (memref<128x256xf32>, memref<128x256xf32>) -> ()
   return %arg0 : memref<128x256xf32>
 }
diff --git a/mlir/test/Transforms/buffer-results-to-out-params.mlir b/mlir/test/Transforms/buffer-results-to-out-params.mlir
index ad0235fb3bb1c..1462e120fad24 100644
--- a/mlir/test/Transforms/buffer-results-to-out-params.mlir
+++ b/mlir/test/Transforms/buffer-results-to-out-params.mlir
@@ -1,29 +1,29 @@
 // RUN: mlir-opt -buffer-results-to-out-params -split-input-file -verify-diagnostics %s | FileCheck %s
 
-// CHECK-LABEL:   func @basic(
+// CHECK-LABEL:   func private @basic(
 // CHECK-SAME:                %[[ARG:.*]]: memref<f32>) {
 // CHECK:           %[[RESULT:.*]] = "test.source"() : () -> memref<f32>
 // CHECK:           memref.copy %[[RESULT]], %[[ARG]]  : memref<f32> to memref<f32>
 // CHECK:           return
 // CHECK:         }
-func.func @basic() -> (memref<f32>) {
+func.func private @basic() -> (memref<f32>) {
   %0 = "test.source"() : () -> (memref<f32>)
   return %0 : memref<f32>
 }
 
-// CHECK-LABEL:   func @presence_of_existing_arguments(
+// CHECK-LABEL:   func private @presence_of_existing_arguments(
 // CHECK-SAME:                                         %[[ARG0:.*]]: memref<1xf32>,
 // CHECK-SAME:                                         %[[ARG1:.*]]: memref<2xf32>) {
 // CHECK:           %[[RESULT:.*]] = "test.source"() : () -> memref<2xf32>
 // CHECK:           memref.copy %[[RESULT]], %[[ARG1]]  : memref<2xf32> to memref<2xf32>
 // CHECK:           return
 // CHECK:         }
-func.func @presence_of_existing_arguments(%arg0: memref<1xf32>) -> (memref<2xf32>) {
+func.func private @presence_of_existing_arguments(%arg0: memref<1xf32>) -> (memref<2xf32>) {
   %0 = "test.source"() : () -> (memref<2xf32>)
   return %0 : memref<2xf32>
 }
 
-// CHECK-LABEL:   func @multiple_results(
+// CHECK-LABEL:   func private @multiple_results(
 // CHECK-SAME:                           %[[ARG0:.*]]: memref<1xf32>,
 // CHECK-SAME:                           %[[ARG1:.*]]: memref<2xf32>) {
 // CHECK:           %[[RESULTS:.*]]:2 = "test.source"() : () -> (memref<1xf32>, memref<2xf32>)
@@ -31,37 +31,36 @@ func.func @presence_of_existing_arguments(%arg0: memref<1xf32>) -> (memref<2xf32
 // CHECK:           memref.copy %[[RESULTS]]#1, %[[ARG1]]  : memref<2xf32> to memref<2xf32>
 // CHECK:           return
 // CHECK:         }
-func.func @multiple_results() -> (memref<1xf32>, memref<2xf32>) {
+func.func private @multiple_results() -> (memref<1xf32>, memref<2xf32>) {
   %0, %1 = "test.source"() : () -> (memref<1xf32>, memref<2xf32>)
   return %0, %1 : memref<1xf32>, memref<2xf32>
 }
 
-// CHECK-LABEL:   func @non_memref_types(
+// CHECK-LABEL:   func private @non_memref_types(
 // CHECK-SAME:                           %[[OUTPARAM:.*]]: memref<f32>) -> (i1, i32) {
 // CHECK:           %[[RESULT1:.*]]:3 = "test.source"() : () -> (i1, memref<f32>, i32)
 // CHECK:           memref.copy %[[RESULT1]]#1, %[[OUTPARAM]]  : memref<f32> to memref<f32>
 // CHECK:           return %[[RESULT1]]#0, %[[RESULT1]]#2 : i1, i32
 // CHECK:         }
-func.func @non_memref_types() -> (i1, memref<f32>, i32) {
+func.func private @non_memref_types() -> (i1, memref<f32>, i32) {
   %0, %1, %2 = "test.source"() : () -> (i1, memref<f32>, i32)
   return %0, %1, %2 : i1, memref<f32>, i32
 }
 
-// CHECK: func private @external_function(memref<f32>)
+// CHECK: func private @external_function() -> memref<f32>
 func.func private @external_function() -> (memref<f32>)
-// CHECK: func private @result_attrs(memref<f32> {test.some_attr})
+// CHECK: func private @result_attrs() -> (memref<f32> {test.some_attr})
 func.func private @result_attrs() -> (memref<f32> {test.some_attr})
-// CHECK: func private @mixed_result_attrs(memref<1xf32>, memref<2xf32> {test.some_attr}, memref<3xf32>)
+// CHECK: func private @mixed_result_attrs() -> (memref<1xf32>, memref<2xf32> {test.some_attr}, memref<3xf32>)
 func.func private @mixed_result_attrs() -> (memref<1xf32>, memref<2xf32> {test.some_attr}, memref<3xf32>)
 
 // -----
 
-// CHECK-LABEL: func private @callee(memref<1xf32>)
+// CHECK-LABEL: func private @callee() -> memref<1xf32>
 func.func private @callee() -> memref<1xf32>
 
 // CHECK-LABEL:   func @call_basic() {
-// CHECK:           %[[OUTPARAM:.*]] = memref.alloc() : memref<1xf32>
-// CHECK:           call @callee(%[[OUTPARAM]]) : (memref<1xf32>) -> ()
+// CHECK:           %[[OUTPARAM:.*]] = call @callee() : () -> memref<1xf32> 
 // CHECK:           "test.sink"(%[[OUTPARAM]]) : (memref<1xf32>) -> ()
 // CHECK:           return
 // CHECK:         }
@@ -73,14 +72,12 @@ func.func @call_basic() {
 
 // -----
 
-// CHECK-LABEL: func private @callee(memref<1xf32>, memref<2xf32>)
+// CHECK-LABEL: func private @callee() -> (memref<1xf32>, memref<2xf32>)
 func.func private @callee() -> (memref<1xf32>, memref<2xf32>)
 
 // CHECK-LABEL:   func @call_multiple_result() {
-// CHECK:           %[[RESULT0:.*]] = memref.alloc() : memref<1xf32>
-// CHECK:           %[[RESULT1:.*]] = memref.alloc() : memref<2xf32>
-// CHECK:           call @callee(%[[RESULT0]], %[[RESULT1]]) : (memref<1xf32>, memref<2xf32>) -> ()
-// CHECK:           "test.sink"(%[[RESULT0]], %[[RESULT1]]) : (memref<1xf32>, memref<2xf32>) -> ()
+// CHECK:           %[[RESULTS:.*]]:2 = call @callee() : () -> (memref<1xf32>, memref<2xf32>)
+// CHECK:           "test.sink"(%[[RESULTS]]#0, %[[RESULTS]]#1) : (memref<1xf32>, memref<2xf32>) -> ()
 // CHECK:         }
 func.func @call_multiple_result() {
   %0, %1 = call @callee() : () -> (memref<1xf32>, memref<2xf32>)
@@ -89,13 +86,12 @@ func.func @call_multiple_result() {
 
 // -----
 
-// CHECK-LABEL: func private @callee(memref<1xf32>) -> (i1, i32)
+// CHECK-LABEL: func private @callee() -> (i1, memref<1xf32>, i32)
 func.func private @callee() -> (i1, memref<1xf32>, i32)
 
 // CHECK-LABEL:   func @call_non_memref_result() {
-// CHECK:           %[[RESULT0:.*]] = memref.alloc() : memref<1xf32>
-// CHECK:           %[[NON_MEMREF_RESULTS:.*]]:2 = call @callee(%[[RESULT0]]) : (memref<1xf32>) -> (i1, i32)
-// CHECK:           "test.sink"(%[[NON_MEMREF_RESULTS]]#0, %[[RESULT0]], %[[NON_MEMREF_RESULTS]]#1) : (i1, memref<1xf32>, i32) -> ()
+// CHECK:           %[[RESULTS:.*]]:3 = call @callee() : () -> (i1, memref<1xf32>, i32)
+// CHECK:           "test.sink"(%[[RESULTS]]#0, %[[RESULTS]]#1, %[[RESULTS]]#2) : (i1, memref<1xf32>, i32) -> ()
 // CHECK:         }
 func.func @call_non_memref_result() {
   %0, %1, %2 = call @callee() : () -> (i1, memref<1xf32>, i32)
@@ -104,10 +100,13 @@ func.func @call_non_memref_result() {
 
 // -----
 
-func.func private @callee() -> (memref<?xf32>)
+func.func private @callee(%size: index) -> (memref<?xf32>) {
+  %alloc = memref.alloc(%size) : memref<?xf32>
+  return %alloc : memref<?xf32>
+}
 
-func.func @call_non_memref_result() {
+func.func @call_non_memref_result(%size: index) {
   // expected-error @+1 {{cannot create out param for dynamically shaped result}}
-  %0 = call @callee() : () -> (memref<?xf32>)
+  %0 = call @callee(%size) : (index) -> (memref<?xf32>)
   "test.sink"(%0) : (memref<?xf32>) -> ()
 }

@llvmbot
Copy link
Member

llvmbot commented Oct 8, 2025

@llvm/pr-subscribers-mlir-bufferization

Author: lonely eagle (linuxlonelyeagle)

Changes

The callers of public or extern functions are unknown, so their function signatures cannot be changed.

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

6 Files Affected:

  • (modified) mlir/lib/Dialect/Bufferization/Transforms/BufferResultsToOutParams.cpp (+5-2)
  • (modified) mlir/test/Conversion/ConvertToEmitC/tosa.mlir (+2-2)
  • (modified) mlir/test/Dialect/Bufferization/Transforms/one-shot-module-bufferize-out-params.mlir (+8-8)
  • (modified) mlir/test/Transforms/buffer-results-to-out-params-add-result-attr.mlir (+2-2)
  • (modified) mlir/test/Transforms/buffer-results-to-out-params-hosit-static-allocs.mlir (+8-8)
  • (modified) mlir/test/Transforms/buffer-results-to-out-params.mlir (+25-26)
diff --git a/mlir/lib/Dialect/Bufferization/Transforms/BufferResultsToOutParams.cpp b/mlir/lib/Dialect/Bufferization/Transforms/BufferResultsToOutParams.cpp
index 25f941dc16516..b9ee0a4d401f3 100644
--- a/mlir/lib/Dialect/Bufferization/Transforms/BufferResultsToOutParams.cpp
+++ b/mlir/lib/Dialect/Bufferization/Transforms/BufferResultsToOutParams.cpp
@@ -217,6 +217,9 @@ updateCalls(ModuleOp module, const AllocDynamicSizesMap &map,
     }
     if (!options.filterFn(&callee))
       return;
+    if (callee.isExternal() || callee.isPublic())
+      return;
+
     SmallVector<Value, 6> replaceWithNewCallResults;
     SmallVector<Value, 6> replaceWithOutParams;
     for (OpResult result : op.getResults()) {
@@ -292,14 +295,14 @@ LogicalResult mlir::bufferization::promoteBufferResultsToOutParams(
   // function.
   AllocDynamicSizesMap map;
   for (auto func : module.getOps<func::FuncOp>()) {
+    if (func.isExternal() || func.isPublic())
+      continue;
     if (!options.filterFn(&func))
       continue;
     SmallVector<BlockArgument, 6> appendedEntryArgs;
     if (failed(
             updateFuncOp(func, appendedEntryArgs, options.addResultAttribute)))
       return failure();
-    if (func.isExternal())
-      continue;
     if (failed(updateReturnOps(func, appendedEntryArgs, map, options))) {
       return failure();
     }
diff --git a/mlir/test/Conversion/ConvertToEmitC/tosa.mlir b/mlir/test/Conversion/ConvertToEmitC/tosa.mlir
index 8ced05eced4b9..158018374e72c 100644
--- a/mlir/test/Conversion/ConvertToEmitC/tosa.mlir
+++ b/mlir/test/Conversion/ConvertToEmitC/tosa.mlir
@@ -20,7 +20,7 @@
 // RUN: mlir-opt --pass-pipeline=%{pipeline} %s | FileCheck %s
 // -----
 
-//      CHECK: emitc.func @main(%[[ARG0:.*]]: !emitc.array<2xf32>, %[[ARG1:.*]]: !emitc.array<2xf32>, %[[RES:.*]]: !emitc.array<2xf32>) {
+//      CHECK: emitc.func private @main(%[[ARG0:.*]]: !emitc.array<2xf32>, %[[ARG1:.*]]: !emitc.array<2xf32>, %[[RES:.*]]: !emitc.array<2xf32>)
 //  CHECK-DAG:   %[[C0:.*]] = "emitc.constant"() <{value = 0 : index}> : () -> !emitc.size_t
 //  CHECK-DAG:   %[[C1:.*]] = "emitc.constant"() <{value = 1 : index}> : () -> !emitc.size_t
 //  CHECK-DAG:   %[[C2:.*]] = "emitc.constant"() <{value = 2 : index}> : () -> !emitc.size_t
@@ -35,7 +35,7 @@
 // CHECK-NEXT:   }
 // CHECK-NEXT:   return
 // CHECK-NEXT: }
-func.func @main(%arg0: tensor<2xf32>, %arg1: tensor<2xf32>) -> tensor<2xf32> {
+func.func private @main(%arg0: tensor<2xf32>, %arg1: tensor<2xf32>) -> tensor<2xf32> {
   %0 = tosa.add %arg0, %arg1 : (tensor<2xf32>, tensor<2xf32>) -> tensor<2xf32>
   return %0 : tensor<2xf32>
 }
diff --git a/mlir/test/Dialect/Bufferization/Transforms/one-shot-module-bufferize-out-params.mlir b/mlir/test/Dialect/Bufferization/Transforms/one-shot-module-bufferize-out-params.mlir
index 9cf44c335d551..75e9a8926ad15 100644
--- a/mlir/test/Dialect/Bufferization/Transforms/one-shot-module-bufferize-out-params.mlir
+++ b/mlir/test/Dialect/Bufferization/Transforms/one-shot-module-bufferize-out-params.mlir
@@ -7,7 +7,7 @@
 
 // Note: This bufferization is not very efficient yet, but it works.
 
-// CHECK-LABEL: func @callee(
+// CHECK-LABEL: func private @callee(
 //  CHECK-SAME:              %[[arg0:.*]]: memref<5xf32, strided<[?], offset: ?>>,
 //  CHECK-SAME:              %[[arg1:.*]]: memref<5xf32, strided<[?], offset: ?>>) {
 // This alloc is not needed, but it is inserted due to the out-of-place
@@ -21,7 +21,7 @@
 //       CHECK:   return
 //       CHECK: }
 
-// CHECK-NO-LAYOUT-LABEL: func @callee(
+// CHECK-NO-LAYOUT-LABEL: func private @callee(
 //  CHECK-NO-LAYOUT-SAME:     %[[arg0:.*]]: memref<5xf32>,
 //  CHECK-NO-LAYOUT-SAME:     %[[arg1:.*]]: memref<5xf32>) {
 //       CHECK-NO-LAYOUT:   %[[alloc:.*]] = memref.alloc() {{.*}} : memref<5xf32>
@@ -29,13 +29,13 @@
 //       CHECK-NO-LAYOUT:   memref.store {{.*}}, %[[alloc]]
 //       CHECK-NO-LAYOUT:   memref.copy %[[alloc]], %[[arg1]]
 
-// CHECK-BASELINE-LABEL: func @callee(
+// CHECK-BASELINE-LABEL: func private @callee(
 //  CHECK-BASELINE-SAME:     %[[arg0:.*]]: memref<5xf32, strided<[?], offset: ?>>) -> memref<5xf32> {
 //       CHECK-BASELINE:   %[[alloc:.*]] = memref.alloc() {{.*}} : memref<5xf32>
 //       CHECK-BASELINE:   memref.copy %[[arg0]], %[[alloc]]
 //       CHECK-BASELINE:   memref.store {{.*}}, %[[alloc]]
 //       CHECK-BASELINE:   return %[[alloc]]
-func.func @callee(%t: tensor<5xf32>) -> (tensor<5xf32>, tensor<5xf32>) {
+func.func private @callee(%t: tensor<5xf32>) -> (tensor<5xf32>, tensor<5xf32>) {
   %c0 = arith.constant 0 : index
   %cst = arith.constant 8.0 : f32
   // This must bufferize out-of-place.
@@ -68,7 +68,7 @@ func.func @main(%t: tensor<5xf32>) -> (f32, f32) {
 
 // -----
 
-// CHECK-LABEL: func @callee(
+// CHECK-LABEL: func private @callee(
 //  CHECK-SAME:     %{{.*}}: index,
 //  CHECK-SAME:     %[[r:.*]]: memref<2x5xf32, strided<[?, ?], offset: ?>>) {
 //       CHECK:   %[[alloc:.*]] = memref.alloc() {{.*}} : memref<10x20xf32>
@@ -76,7 +76,7 @@ func.func @main(%t: tensor<5xf32>) -> (f32, f32) {
 //       CHECK:   %[[casted:.*]] = memref.cast %[[subview]]
 //       CHECK:   memref.copy %[[casted]], %[[r]]
 
-// CHECK-NO-LAYOUT-LABEL: func @callee(
+// CHECK-NO-LAYOUT-LABEL: func private @callee(
 //  CHECK-NO-LAYOUT-SAME:              %{{.*}}: index,
 //  CHECK-NO-LAYOUT-SAME:              %[[r:.*]]: memref<2x5xf32>) {
 //       CHECK-NO-LAYOUT:   %[[alloc:.*]] = memref.alloc() {{.*}} : memref<10x20xf32>
@@ -88,12 +88,12 @@ func.func @main(%t: tensor<5xf32>) -> (f32, f32) {
 //       CHECK-NO-LAYOUT:   memref.copy %[[subview]], %[[alloc2]]
 //       CHECK-NO-LAYOUT:   memref.copy %[[alloc2]], %[[r]]
 
-// CHECK-BASELINE-LABEL: func @callee(
+// CHECK-BASELINE-LABEL: func private @callee(
 //  CHECK-BASELINE-SAME:     %{{.*}}: index) -> memref<2x5xf32, strided<[20, 1], offset: ?>> {
 //       CHECK-BASELINE:   %[[alloc:.*]] = memref.alloc() {{.*}} : memref<10x20xf32>
 //       CHECK-BASELINE:   %[[subview:.*]] = memref.subview %[[alloc]]
 //       CHECK-BASELINE:   return %[[subview]]
-func.func @callee(%idx: index) -> tensor<2x5xf32> {
+func.func private @callee(%idx: index) -> tensor<2x5xf32> {
   %0 = bufferization.alloc_tensor() : tensor<10x20xf32>
   %1 = tensor.extract_slice %0[%idx, %idx][2, 5][1, 1] : tensor<10x20xf32> to tensor<2x5xf32>
   return %1 : tensor<2x5xf32>
diff --git a/mlir/test/Transforms/buffer-results-to-out-params-add-result-attr.mlir b/mlir/test/Transforms/buffer-results-to-out-params-add-result-attr.mlir
index f4a95c73e2953..8faacdb00d556 100644
--- a/mlir/test/Transforms/buffer-results-to-out-params-add-result-attr.mlir
+++ b/mlir/test/Transforms/buffer-results-to-out-params-add-result-attr.mlir
@@ -1,7 +1,7 @@
 // RUN: mlir-opt -p 'builtin.module(buffer-results-to-out-params{add-result-attr})' -split-input-file -verify-diagnostics %s | FileCheck %s
 
 // CHECK-LABEL: @basic({{.*}}: memref<f32> {bufferize.result})
-func.func @basic() -> (memref<f32>) {
+func.func private @basic() -> (memref<f32>) {
   %0 = "test.source"() : () -> (memref<f32>)
   return %0 : memref<f32>
 }
@@ -11,7 +11,7 @@ func.func @basic() -> (memref<f32>) {
 // CHECK-LABEL: multiple_results
 // CHECK-SAME:  memref<1xf32> {bufferize.result}
 // CHECK-SAME:  memref<2xf32> {bufferize.result}
-func.func @multiple_results() -> (memref<1xf32>, memref<2xf32>) {
+func.func private @multiple_results() -> (memref<1xf32>, memref<2xf32>) {
   %0, %1 = "test.source"() : () -> (memref<1xf32>, memref<2xf32>)
   return %0, %1 : memref<1xf32>, memref<2xf32>
 }
diff --git a/mlir/test/Transforms/buffer-results-to-out-params-hosit-static-allocs.mlir b/mlir/test/Transforms/buffer-results-to-out-params-hosit-static-allocs.mlir
index 2783836a09e16..97fb9f45f57f8 100644
--- a/mlir/test/Transforms/buffer-results-to-out-params-hosit-static-allocs.mlir
+++ b/mlir/test/Transforms/buffer-results-to-out-params-hosit-static-allocs.mlir
@@ -1,36 +1,36 @@
 // RUN: mlir-opt -allow-unregistered-dialect -p 'builtin.module(buffer-results-to-out-params{hoist-static-allocs})'  %s | FileCheck %s
 
-// CHECK-LABEL:   func @basic(
+// CHECK-LABEL:   func private @basic(
 // CHECK-SAME:                %[[ARG:.*]]: memref<8x64xf32>) {
 // CHECK-NOT:        memref.alloc()
 // CHECK:           "test.source"(%[[ARG]])  : (memref<8x64xf32>) -> ()
 // CHECK:           return
 // CHECK:         }
-func.func @basic() -> (memref<8x64xf32>) {
+func.func private @basic() -> (memref<8x64xf32>) {
   %b = memref.alloc() : memref<8x64xf32>
   "test.source"(%b)  : (memref<8x64xf32>) -> ()
   return %b : memref<8x64xf32>
 }
 
-// CHECK-LABEL:   func @basic_no_change(
+// CHECK-LABEL:   func private @basic_no_change(
 // CHECK-SAME:                %[[ARG:.*]]: memref<f32>) {
 // CHECK:           %[[RESULT:.*]] = "test.source"() : () -> memref<f32>
 // CHECK:           memref.copy %[[RESULT]], %[[ARG]]  : memref<f32> to memref<f32>
 // CHECK:           return
 // CHECK:         }
-func.func @basic_no_change() -> (memref<f32>) {
+func.func private @basic_no_change() -> (memref<f32>) {
   %0 = "test.source"() : () -> (memref<f32>)
   return %0 : memref<f32>
 }
 
-// CHECK-LABEL:   func @basic_dynamic(
+// CHECK-LABEL:   func private @basic_dynamic(
 // CHECK-SAME:                %[[D:.*]]: index, %[[ARG:.*]]: memref<?xf32>) {
 // CHECK:           %[[RESULT:.*]] = memref.alloc(%[[D]]) : memref<?xf32>
 // CHECK:           "test.source"(%[[RESULT]])  : (memref<?xf32>) -> ()
 // CHECK:           memref.copy %[[RESULT]], %[[ARG]]
 // CHECK:           return
 // CHECK:         }
-func.func @basic_dynamic(%d: index) -> (memref<?xf32>) {
+func.func private @basic_dynamic(%d: index) -> (memref<?xf32>) {
   %b = memref.alloc(%d) : memref<?xf32>
   "test.source"(%b)  : (memref<?xf32>) -> ()
   return %b : memref<?xf32>
@@ -39,13 +39,13 @@ func.func @basic_dynamic(%d: index) -> (memref<?xf32>) {
 // -----
 
 // no change due to writing to func args
-// CHECK-LABEL:   func @return_arg(
+// CHECK-LABEL:   func private @return_arg(
 // CHECK-SAME:        %[[ARG0:.*]]: memref<128x256xf32>, %[[ARG1:.*]]: memref<128x256xf32>, %[[ARG2:.*]]: memref<128x256xf32>) {
 // CHECK:           "test.source"(%[[ARG0]], %[[ARG1]])
 // CHECK:           memref.copy
 // CHECK:           return
 // CHECK:         }
-func.func @return_arg(%arg0: memref<128x256xf32>, %arg1: memref<128x256xf32>) -> memref<128x256xf32> {
+func.func private @return_arg(%arg0: memref<128x256xf32>, %arg1: memref<128x256xf32>) -> memref<128x256xf32> {
   "test.source"(%arg0, %arg1)  : (memref<128x256xf32>, memref<128x256xf32>) -> ()
   return %arg0 : memref<128x256xf32>
 }
diff --git a/mlir/test/Transforms/buffer-results-to-out-params.mlir b/mlir/test/Transforms/buffer-results-to-out-params.mlir
index ad0235fb3bb1c..1462e120fad24 100644
--- a/mlir/test/Transforms/buffer-results-to-out-params.mlir
+++ b/mlir/test/Transforms/buffer-results-to-out-params.mlir
@@ -1,29 +1,29 @@
 // RUN: mlir-opt -buffer-results-to-out-params -split-input-file -verify-diagnostics %s | FileCheck %s
 
-// CHECK-LABEL:   func @basic(
+// CHECK-LABEL:   func private @basic(
 // CHECK-SAME:                %[[ARG:.*]]: memref<f32>) {
 // CHECK:           %[[RESULT:.*]] = "test.source"() : () -> memref<f32>
 // CHECK:           memref.copy %[[RESULT]], %[[ARG]]  : memref<f32> to memref<f32>
 // CHECK:           return
 // CHECK:         }
-func.func @basic() -> (memref<f32>) {
+func.func private @basic() -> (memref<f32>) {
   %0 = "test.source"() : () -> (memref<f32>)
   return %0 : memref<f32>
 }
 
-// CHECK-LABEL:   func @presence_of_existing_arguments(
+// CHECK-LABEL:   func private @presence_of_existing_arguments(
 // CHECK-SAME:                                         %[[ARG0:.*]]: memref<1xf32>,
 // CHECK-SAME:                                         %[[ARG1:.*]]: memref<2xf32>) {
 // CHECK:           %[[RESULT:.*]] = "test.source"() : () -> memref<2xf32>
 // CHECK:           memref.copy %[[RESULT]], %[[ARG1]]  : memref<2xf32> to memref<2xf32>
 // CHECK:           return
 // CHECK:         }
-func.func @presence_of_existing_arguments(%arg0: memref<1xf32>) -> (memref<2xf32>) {
+func.func private @presence_of_existing_arguments(%arg0: memref<1xf32>) -> (memref<2xf32>) {
   %0 = "test.source"() : () -> (memref<2xf32>)
   return %0 : memref<2xf32>
 }
 
-// CHECK-LABEL:   func @multiple_results(
+// CHECK-LABEL:   func private @multiple_results(
 // CHECK-SAME:                           %[[ARG0:.*]]: memref<1xf32>,
 // CHECK-SAME:                           %[[ARG1:.*]]: memref<2xf32>) {
 // CHECK:           %[[RESULTS:.*]]:2 = "test.source"() : () -> (memref<1xf32>, memref<2xf32>)
@@ -31,37 +31,36 @@ func.func @presence_of_existing_arguments(%arg0: memref<1xf32>) -> (memref<2xf32
 // CHECK:           memref.copy %[[RESULTS]]#1, %[[ARG1]]  : memref<2xf32> to memref<2xf32>
 // CHECK:           return
 // CHECK:         }
-func.func @multiple_results() -> (memref<1xf32>, memref<2xf32>) {
+func.func private @multiple_results() -> (memref<1xf32>, memref<2xf32>) {
   %0, %1 = "test.source"() : () -> (memref<1xf32>, memref<2xf32>)
   return %0, %1 : memref<1xf32>, memref<2xf32>
 }
 
-// CHECK-LABEL:   func @non_memref_types(
+// CHECK-LABEL:   func private @non_memref_types(
 // CHECK-SAME:                           %[[OUTPARAM:.*]]: memref<f32>) -> (i1, i32) {
 // CHECK:           %[[RESULT1:.*]]:3 = "test.source"() : () -> (i1, memref<f32>, i32)
 // CHECK:           memref.copy %[[RESULT1]]#1, %[[OUTPARAM]]  : memref<f32> to memref<f32>
 // CHECK:           return %[[RESULT1]]#0, %[[RESULT1]]#2 : i1, i32
 // CHECK:         }
-func.func @non_memref_types() -> (i1, memref<f32>, i32) {
+func.func private @non_memref_types() -> (i1, memref<f32>, i32) {
   %0, %1, %2 = "test.source"() : () -> (i1, memref<f32>, i32)
   return %0, %1, %2 : i1, memref<f32>, i32
 }
 
-// CHECK: func private @external_function(memref<f32>)
+// CHECK: func private @external_function() -> memref<f32>
 func.func private @external_function() -> (memref<f32>)
-// CHECK: func private @result_attrs(memref<f32> {test.some_attr})
+// CHECK: func private @result_attrs() -> (memref<f32> {test.some_attr})
 func.func private @result_attrs() -> (memref<f32> {test.some_attr})
-// CHECK: func private @mixed_result_attrs(memref<1xf32>, memref<2xf32> {test.some_attr}, memref<3xf32>)
+// CHECK: func private @mixed_result_attrs() -> (memref<1xf32>, memref<2xf32> {test.some_attr}, memref<3xf32>)
 func.func private @mixed_result_attrs() -> (memref<1xf32>, memref<2xf32> {test.some_attr}, memref<3xf32>)
 
 // -----
 
-// CHECK-LABEL: func private @callee(memref<1xf32>)
+// CHECK-LABEL: func private @callee() -> memref<1xf32>
 func.func private @callee() -> memref<1xf32>
 
 // CHECK-LABEL:   func @call_basic() {
-// CHECK:           %[[OUTPARAM:.*]] = memref.alloc() : memref<1xf32>
-// CHECK:           call @callee(%[[OUTPARAM]]) : (memref<1xf32>) -> ()
+// CHECK:           %[[OUTPARAM:.*]] = call @callee() : () -> memref<1xf32> 
 // CHECK:           "test.sink"(%[[OUTPARAM]]) : (memref<1xf32>) -> ()
 // CHECK:           return
 // CHECK:         }
@@ -73,14 +72,12 @@ func.func @call_basic() {
 
 // -----
 
-// CHECK-LABEL: func private @callee(memref<1xf32>, memref<2xf32>)
+// CHECK-LABEL: func private @callee() -> (memref<1xf32>, memref<2xf32>)
 func.func private @callee() -> (memref<1xf32>, memref<2xf32>)
 
 // CHECK-LABEL:   func @call_multiple_result() {
-// CHECK:           %[[RESULT0:.*]] = memref.alloc() : memref<1xf32>
-// CHECK:           %[[RESULT1:.*]] = memref.alloc() : memref<2xf32>
-// CHECK:           call @callee(%[[RESULT0]], %[[RESULT1]]) : (memref<1xf32>, memref<2xf32>) -> ()
-// CHECK:           "test.sink"(%[[RESULT0]], %[[RESULT1]]) : (memref<1xf32>, memref<2xf32>) -> ()
+// CHECK:           %[[RESULTS:.*]]:2 = call @callee() : () -> (memref<1xf32>, memref<2xf32>)
+// CHECK:           "test.sink"(%[[RESULTS]]#0, %[[RESULTS]]#1) : (memref<1xf32>, memref<2xf32>) -> ()
 // CHECK:         }
 func.func @call_multiple_result() {
   %0, %1 = call @callee() : () -> (memref<1xf32>, memref<2xf32>)
@@ -89,13 +86,12 @@ func.func @call_multiple_result() {
 
 // -----
 
-// CHECK-LABEL: func private @callee(memref<1xf32>) -> (i1, i32)
+// CHECK-LABEL: func private @callee() -> (i1, memref<1xf32>, i32)
 func.func private @callee() -> (i1, memref<1xf32>, i32)
 
 // CHECK-LABEL:   func @call_non_memref_result() {
-// CHECK:           %[[RESULT0:.*]] = memref.alloc() : memref<1xf32>
-// CHECK:           %[[NON_MEMREF_RESULTS:.*]]:2 = call @callee(%[[RESULT0]]) : (memref<1xf32>) -> (i1, i32)
-// CHECK:           "test.sink"(%[[NON_MEMREF_RESULTS]]#0, %[[RESULT0]], %[[NON_MEMREF_RESULTS]]#1) : (i1, memref<1xf32>, i32) -> ()
+// CHECK:           %[[RESULTS:.*]]:3 = call @callee() : () -> (i1, memref<1xf32>, i32)
+// CHECK:           "test.sink"(%[[RESULTS]]#0, %[[RESULTS]]#1, %[[RESULTS]]#2) : (i1, memref<1xf32>, i32) -> ()
 // CHECK:         }
 func.func @call_non_memref_result() {
   %0, %1, %2 = call @callee() : () -> (i1, memref<1xf32>, i32)
@@ -104,10 +100,13 @@ func.func @call_non_memref_result() {
 
 // -----
 
-func.func private @callee() -> (memref<?xf32>)
+func.func private @callee(%size: index) -> (memref<?xf32>) {
+  %alloc = memref.alloc(%size) : memref<?xf32>
+  return %alloc : memref<?xf32>
+}
 
-func.func @call_non_memref_result() {
+func.func @call_non_memref_result(%size: index) {
   // expected-error @+1 {{cannot create out param for dynamically shaped result}}
-  %0 = call @callee() : () -> (memref<?xf32>)
+  %0 = call @callee(%size) : (index) -> (memref<?xf32>)
   "test.sink"(%0) : (memref<?xf32>) -> ()
 }

@linuxlonelyeagle linuxlonelyeagle changed the title [mlir][bufferize] Make buffer-results-to-out-params support only functions that are neither private nor extern [mlir][bufferize] Make buffer-results-to-out-params support only functions that are neither public nor extern Oct 8, 2025
@linuxlonelyeagle linuxlonelyeagle merged commit 71586a6 into llvm:main Oct 8, 2025
13 checks passed
@linuxlonelyeagle
Copy link
Member Author

@matthias-springer thank you.

svkeerthy pushed a commit that referenced this pull request Oct 9, 2025
@linuxlonelyeagle @svkeerthy
[mlir][bufferize] Make buffer-results-to-out-params support only func…
30cd06f 
…tions that are neither public nor extern (#162441)

The callers of public or extern functions are unknown, so their function
signatures cannot be changed.
clingfei pushed a commit to clingfei/llvm-project that referenced this pull request Oct 10, 2025
@linuxlonelyeagle @clingfei
[mlir][bufferize] Make buffer-results-to-out-params support only func…
dde68d3 
…tions that are neither public nor extern (llvm#162441)

The callers of public or extern functions are unknown, so their function
signatures cannot be changed.
@nuudlman
Copy link
Contributor

nuudlman commented Oct 15, 2025 •
edited
Loading

The documentation of this pass implies that it is used for enforcing calling conventions:

Some calling conventions prefer to pass output memrefs as "out params". The
conversion to this calling convention must be done as an atomic
transformation of the entire program (hence this is a module pass).

Our internal target uses this pass to enforce a valid calling convention for MLIR, and I believe that many users of EmitC will also be using this as iirc EmitC doesn't support functions that return memrefs. To be clear, this change completely breaks our downstream target.

See https://godbolt.org/z/n8bvv4oTa for an example of this change breaking EmitC export (Compiler Explorer doesn't have this change merged, so I removed this pass to be equivalent).

Given that this pass explicitly modifies the calling convention, l don't believe that the reasoning for this change is valid. The presence of a calling convention implies that all functions must abide by its rules, and thus if a platform/target requires out-params, it is perfectly valid to change the function signatures of public or extern functions so that they comply with calling convention.

Considering that this breaks EmitC and at least one downstream target, I believe that this should be reverted.

@linuxlonelyeagle
Copy link
Member Author

The documentation of this pass implies that it is used for enforcing calling conventions:

Some calling conventions prefer to pass output memrefs as "out params". The
conversion to this calling convention must be done as an atomic
transformation of the entire program (hence this is a module pass).

Our internal target uses this pass to enforce a valid calling convention for MLIR, and I believe that many users of EmitC will also be using this as iirc EmitC doesn't support functions that return memrefs. To be clear, this change completely breaks our downstream target.

See https://godbolt.org/z/n8bvv4oTa for an example of this change breaking EmitC export (Compiler Explorer doesn't have this change merged, so I removed this pass to be equivalent).

Given that this pass explicitly modifies the calling convention, l don't believe that the reasoning for this change is valid. The presence of a calling convention implies that all functions must abide by its rules, and thus if a platform/target requires out-params, it is perfectly valid to change the function signatures of public or extern functions so that they comply with calling convention.

Considering that this breaks EmitC and at least one downstream target, I believe that this should be reverted.

cc: @matthias-springer @joker-eph @ftynse What are you thinking?

@linuxlonelyeagle
Copy link
Member Author

I think the current changes make sense.The reason lies in the fact that during DeadCodeAnalysis, public and extern functions have unknown function calls, so we cannot modify the function signatures.

For the given case, https://godbolt.org/z/n8bvv4oTa, You should make the function private.

@nuudlman
Copy link
Contributor

I'm not sure I follow this line of reasoning. Privatization is a much more significant breaking API change as compared to a transform that one-shot changes the calling convention.

There are multiple arguments against this:

  1. EmitC is supposedly usable end to end with upstream unmodified MLIR. There is no un-privatize pass in upstream MLIR (at least that I am aware of), so this workaround is not usable without writing code. Assuming that the privatized function survives to export (since private functions are liable to get elided), it no longer has any API since it is private.
  2. This pass documents that it is only valid for cases where all of the relevant MLIR is visible in the module so that it can do a one-shot conversion. Regardless, calling conventions are enforced globally, so any unknown callers must also expect this change.
  3. symbol-privatize is an invalid pass by your line of reasoning, as there may be unknown callers that rely on the presence of public functions that are made private

@nuudlman
Copy link
Contributor

nuudlman commented Oct 15, 2025 •
edited
Loading

More broadly, I am not convinced that the argument that public function signatures cannot be changed holds water, as this would be a breaking change to the Func dialect and MLIR as a whole.

The symbol visibility documentation (https://mlir.llvm.org/docs/SymbolsAndSymbolTables/#symbol-visibility)

The symbol may be referenced from outside of the visible IR. We cannot assume that all of the uses of this symbol are observable.

does state that visibility of public symbols is unknown, however this does not state that public symbols are guaranteed to be immutable. As far as I can tell, the only evidence for this claim is this and similar PRs.

More importantly, public symbol immutability would be a very significant breaking change to MLIR as a whole.
There is existing precedent that public symbols can be modified if the user wishes so or as part of lowering:

  • The aforementioned symbol-privatize completely removes public symbols
  • Bufferization can completely change the types of public symbols
  • Target conversion likewise completely changes the types of public symbols (e.g. LLVM spreads memrefs, EmitC converts to multi-dim arrays)

I am in complete agreement about modification of public symbols when it comes to optimization/non user invoked changes, like in dead code analysis/elimination. However, bufferizing to out params is a lowering pass, so public symbol modification is perfectly acceptable.

@nuudlman
Copy link
Contributor

ping @matthias-springer @joker-eph @ftynse again. I believe under the revert guidelines this should have been reverted as soon as there was a breakage report.

@matthias-springer
Copy link
Member

How about adding a pass option (and flag to BufferResultsToOutParamsOpts) that allows users to opt into modifying external/public functions?

@matthias-springer
Copy link
Member

matthias-springer commented Oct 20, 2025 •
edited
Loading

The reason lies in the fact that during DeadCodeAnalysis, public and extern functions have unknown function calls, so we cannot modify the function signatures.

There are a few other passes that behave similarly: e.g., the ownership-based buffer deallocation pass changes function signatures only if the function is private:

    The `private-function-dynamic-ownership` pass option allows the pass to add
    additional arguments to private functions to dynamically give ownership of
    MemRefs to callees. This can enable earlier deallocations and allows the
    pass to by-pass the function boundary ABI and thus potentially leading to
    fewer MemRef clones being inserted.

@nuudlman
Copy link
Contributor

I will go ahead and send a PR with the flag added later today

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@matthias-springer matthias-springer matthias-springer approved these changes

@joker-eph joker-eph Awaiting requested review from joker-eph

@ftynse ftynse Awaiting requested review from ftynse

@krzysz00 krzysz00 Awaiting requested review from krzysz00

Assignees

No one assigned

Labels

mlir:bufferization Bufferization infrastructure mlir:emitc mlir

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@linuxlonelyeagle @llvmbot @nuudlman @matthias-springer