From 8b28500f60b716a353b733a0111aae6a4a5e9854 Mon Sep 17 00:00:00 2001 From: Quentin Colombet Date: Fri, 10 Feb 2023 13:43:33 +0100 Subject: [PATCH] [mlir][MemRef][TransformOps] Fix error reporting for multibuffer Multibuffer will fail to apply on allocs that are used outside of loops. This was properly caught in the current implementation but the way we report it was broken. Notes cannot be emitted on their own, they need to be attached to another main diagnostic. Long story short, change the severity of the report from Note to Error. Differential Revision: https://reviews.llvm.org/D143729 --- .../MemRef/TransformOps/MemRefTransformOps.cpp | 8 ++---- mlir/test/Dialect/MemRef/transform-ops.mlir | 32 +++++++++++++++++++++- 2 files changed, 34 insertions(+), 6 deletions(-) diff --git a/mlir/lib/Dialect/MemRef/TransformOps/MemRefTransformOps.cpp b/mlir/lib/Dialect/MemRef/TransformOps/MemRefTransformOps.cpp index 084f9a8d..1b53686 100644 --- a/mlir/lib/Dialect/MemRef/TransformOps/MemRefTransformOps.cpp +++ b/mlir/lib/Dialect/MemRef/TransformOps/MemRefTransformOps.cpp @@ -25,11 +25,9 @@ DiagnosedSilenceableFailure transform::MemRefMultiBufferOp::applyToOne( memref::AllocOp target, transform::ApplyToEachResultList &results, transform::TransformState &state) { auto newBuffer = memref::multiBuffer(target, getFactor()); - if (failed(newBuffer)) { - Diagnostic diag(target->getLoc(), DiagnosticSeverity::Note); - diag << "op failed to multibuffer"; - return DiagnosedSilenceableFailure::silenceableFailure(std::move(diag)); - } + if (failed(newBuffer)) + return emitSilenceableFailure(target->getLoc()) + << "op failed to multibuffer"; results.push_back(*newBuffer); return DiagnosedSilenceableFailure::success(); diff --git a/mlir/test/Dialect/MemRef/transform-ops.mlir b/mlir/test/Dialect/MemRef/transform-ops.mlir index 9716680..ddb6407 100644 --- a/mlir/test/Dialect/MemRef/transform-ops.mlir +++ b/mlir/test/Dialect/MemRef/transform-ops.mlir @@ -1,4 +1,4 @@ -// RUN: mlir-opt %s -test-transform-dialect-interpreter -verify-diagnostics -allow-unregistered-dialect | FileCheck %s +// RUN: mlir-opt %s -test-transform-dialect-interpreter -verify-diagnostics -allow-unregistered-dialect -split-input-file | FileCheck %s // CHECK-DAG: #[[$MAP0:.*]] = affine_map<(d0) -> ((d0 floordiv 4) mod 2)> // CHECK-DAG: #[[$MAP1:.*]] = affine_map<(d0)[s0] -> (d0 + s0)> @@ -35,3 +35,33 @@ transform.sequence failures(propagate) { // Verify that the returned handle is usable. transform.test_print_remark_at_operand %1, "transformed" : !pdl.operation } + +// ----- + +// Trying to use multibuffer on alloc that are used outside of loops is +// going to fail. +// Check that we emit a proper error for that. +func.func @multi_buffer_uses_outside_of_loop(%in: memref<16xf32>) { + // expected-error @below {{op failed to multibuffer}} + %tmp = memref.alloc() : memref<4xf32> + + "some_outside_loop_use"(%tmp) : (memref<4xf32>) -> () + + %c0 = arith.constant 0 : index + %c4 = arith.constant 4 : index + %c16 = arith.constant 16 : index + + scf.for %i0 = %c0 to %c16 step %c4 { + %1 = memref.subview %in[%i0] [4] [1] : memref<16xf32> to memref<4xf32, affine_map<(d0)[s0] -> (d0 + s0)>> + memref.copy %1, %tmp : memref<4xf32, affine_map<(d0)[s0] -> (d0 + s0)>> to memref<4xf32> + + "some_use"(%tmp) : (memref<4xf32>) ->() + } + return +} + +transform.sequence failures(propagate) { +^bb1(%arg1: !pdl.operation): + %0 = transform.structured.match ops{["memref.alloc"]} in %arg1 : (!pdl.operation) -> !pdl.operation + %1 = transform.memref.multibuffer %0 {factor = 2 : i64} +} -- 2.7.4