From 0e3ddbac918facd2c32bb0606120893c98e0f947 Mon Sep 17 00:00:00 2001 From: Sean Silva Date: Thu, 25 Aug 2022 22:18:19 +0000 Subject: [PATCH] Remove VerifyInvariantsBeforeBackendLowering LowerToBackendContract now checks all this consistently. --- .../TorchConversion/Transforms/Passes.h | 3 - .../TorchConversion/Transforms/Passes.td | 21 ----- .../TorchConversion/Transforms/CMakeLists.txt | 3 +- .../TorchConversion/Transforms/Passes.cpp | 12 --- .../VerifyInvariantsBeforeBackendLowering.cpp | 86 ------------------- ...fy-invariants-before-backend-lowering.mlir | 36 -------- utils/bazel/torch-mlir-overlay/BUILD.bazel | 1 - 7 files changed, 1 insertion(+), 161 deletions(-) delete mode 100644 lib/Dialect/TorchConversion/Transforms/VerifyInvariantsBeforeBackendLowering.cpp delete mode 100644 test/Dialect/TorchConversion/verify-invariants-before-backend-lowering.mlir diff --git a/include/torch-mlir/Dialect/TorchConversion/Transforms/Passes.h b/include/torch-mlir/Dialect/TorchConversion/Transforms/Passes.h index b83857f9c..c4008dec4 100644 --- a/include/torch-mlir/Dialect/TorchConversion/Transforms/Passes.h +++ b/include/torch-mlir/Dialect/TorchConversion/Transforms/Passes.h @@ -41,9 +41,6 @@ void createTorchBackendToMhloBackendPipeline( const torch::Torch::TorchLoweringPipelineOptions &options); #endif -std::unique_ptr> -createVerifyInvariantsBeforeBackendLoweringPass(); - std::unique_ptr> createFuncBackendTypeConversionPass(); std::unique_ptr> diff --git a/include/torch-mlir/Dialect/TorchConversion/Transforms/Passes.td b/include/torch-mlir/Dialect/TorchConversion/Transforms/Passes.td index cbadd0b92..8afd9850b 100644 --- a/include/torch-mlir/Dialect/TorchConversion/Transforms/Passes.td +++ b/include/torch-mlir/Dialect/TorchConversion/Transforms/Passes.td @@ -12,27 +12,6 @@ include "mlir/Pass/PassBase.td" -def VerifyInvariantsBeforeBackendLowering - : Pass<"torch-verify-invariants-before-backend-lowering", "ModuleOp"> { - let summary = "Verify invariants required by backend lowering"; - let constructor = - "mlir::torch::TorchConversion::createVerifyInvariantsBeforeBackendLoweringPass()"; - let description = [{ - This pass checks any invariants needed by the process of lowering the - `torch` dialect to the linalg-on-tensors backend contract. - - The most important invariant is that all tensors should be ranked and have - a known dtype. It is useful to catch this early because it usually - represents a simple bug in RefineTypes, but can manifest as many different - kinds of obscure symptoms during lowering. - - TODO: This pass should probably be phrased as checking the - "torch backend contract" and moved to that dialect once we have more - substantial definition definition around what that layer is from an - "allowlist" perspective. - }]; -} - def FuncBackendTypeConversion : Pass<"torch-func-backend-type-conversion", "ModuleOp"> { let summary = "Convert functions to operate on builtin tensors"; let constructor = "mlir::torch::TorchConversion::createFuncBackendTypeConversionPass()"; diff --git a/lib/Dialect/TorchConversion/Transforms/CMakeLists.txt b/lib/Dialect/TorchConversion/Transforms/CMakeLists.txt index e0a4e4fa2..f8c3373cb 100644 --- a/lib/Dialect/TorchConversion/Transforms/CMakeLists.txt +++ b/lib/Dialect/TorchConversion/Transforms/CMakeLists.txt @@ -18,7 +18,6 @@ add_mlir_library(TorchMLIRTorchConversionPasses BackendTypeConversion.cpp BackendTypeConversionPasses.cpp Passes.cpp - VerifyInvariantsBeforeBackendLowering.cpp VerifyLinalgOnTensorsBackendContract.cpp VerifyTosaBackendContract.cpp @@ -34,4 +33,4 @@ add_mlir_library(TorchMLIRTorchConversionPasses LINK_LIBS PUBLIC ${LinkedLibs} -) \ No newline at end of file +) diff --git a/lib/Dialect/TorchConversion/Transforms/Passes.cpp b/lib/Dialect/TorchConversion/Transforms/Passes.cpp index 2aec9567d..b2f012f03 100644 --- a/lib/Dialect/TorchConversion/Transforms/Passes.cpp +++ b/lib/Dialect/TorchConversion/Transforms/Passes.cpp @@ -63,10 +63,6 @@ void mlir::torch::registerTorchConversionPasses() { void TorchConversion::createTorchBackendToLinalgOnTensorsBackendPipeline( OpPassManager &pm, const Torch::TorchLoweringPipelineOptions &options) { - // Check some invariants to catch errors in a clear way. - pm.addPass( - TorchConversion::createVerifyInvariantsBeforeBackendLoweringPass()); - // Lower to linalg + guards which is the input to codegen backends. // We do this first as it tends to involve pattern-matching against constants, // (e.g. dimensions which must be constant in a ranked programming model) @@ -101,10 +97,6 @@ void TorchConversion::createTorchBackendToLinalgOnTensorsBackendPipeline( void TorchConversion::createTorchBackendToTosaBackendPipeline( OpPassManager &pm, const Torch::TorchLoweringPipelineOptions &options) { - // Check some invariants to catch errors in a clear way. - pm.addPass( - TorchConversion::createVerifyInvariantsBeforeBackendLoweringPass()); - pm.addNestedPass(createConvertTorchToTosaPass()); // Perform rank broadcasting so TosaToLinalg pass works pm.addNestedPass(createTosaMakeBroadcastablePass()); @@ -130,10 +122,6 @@ void TorchConversion::createTorchBackendToTosaBackendPipeline( #ifdef TORCH_MLIR_ENABLE_MHLO void TorchConversion::createTorchBackendToMhloBackendPipeline( OpPassManager &pm, const Torch::TorchLoweringPipelineOptions &options) { - // Check some invariants to catch errors in a clear way. - pm.addPass( - TorchConversion::createVerifyInvariantsBeforeBackendLoweringPass()); - pm.addNestedPass(createConvertTorchToMhloPass()); // Clean up any non-canonical code introduced above.. diff --git a/lib/Dialect/TorchConversion/Transforms/VerifyInvariantsBeforeBackendLowering.cpp b/lib/Dialect/TorchConversion/Transforms/VerifyInvariantsBeforeBackendLowering.cpp deleted file mode 100644 index 85861457b..000000000 --- a/lib/Dialect/TorchConversion/Transforms/VerifyInvariantsBeforeBackendLowering.cpp +++ /dev/null @@ -1,86 +0,0 @@ -//===- VerifyInvariantsBeforeBackendLowering.cpp -----------------*- C++-*-===// -// -// This file is licensed under the Apache License v2.0 with LLVM Exceptions. -// See https://llvm.org/LICENSE.txt for license information. -// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception -// Also available under a BSD-style license. See LICENSE. -// -//===----------------------------------------------------------------------===// - -#include "PassDetail.h" - -#include "mlir/IR/Builders.h" -#include "mlir/IR/BuiltinOps.h" -#include "torch-mlir/Dialect/Torch/IR/TorchOps.h" -#include "torch-mlir/Dialect/TorchConversion/IR/TorchConversionOps.h" -#include "torch-mlir/Dialect/TorchConversion/Transforms/Passes.h" - -using namespace mlir; -using namespace mlir::torch; -using namespace mlir::torch::TorchConversion; -using namespace mlir::torch; - -static LogicalResult checkValueInvariants(Operation *errorReportOp, Value v) { - // TODO: Make this an allowlist instead of a denylist. - // TODO: Make this stricter. - auto type = v.getType(); - if (auto valueTensorType = type.dyn_cast()) { - if (!valueTensorType.hasDtype() || !valueTensorType.hasSizes()) - return errorReportOp->emitError() - .append("unsupported by backend lowering: tensor with unknown rank " - "or dtype") - .attachNote() - .append("this is likely due to a missing shape transfer function in " - "shape_lib_gen.py or missing case in RefineTypes"); - } - return success(); -} - -namespace { - -class VerifyInvariantsBeforeBackendLoweringPass - : public VerifyInvariantsBeforeBackendLoweringBase< - VerifyInvariantsBeforeBackendLoweringPass> { - void runOnOperation() override { - if (getOperation() - .walk([](Torch::GlobalSlotModuleInitializerOp op) { - op.emitError() - << "unsupported by backend lowering: module initializers"; - return WalkResult::interrupt(); - }) - .wasInterrupted()) - return signalPassFailure(); - auto walkResult = getOperation().walk([&](Block *block) { - // Check invariants on all the Value's in the program. - // That is, check all BlockArgument's and OpResult's. - for (BlockArgument arg : block->getArguments()) - if (failed(checkValueInvariants(block->getParentOp(), arg))) - return WalkResult::interrupt(); - - for (Operation &op : *block) { - if (isa(op)) { - op.emitError() - .append("unsupported by backend lowering: `torch.operator` op") - .attachNote() - .append("this is likely due to a missing op that needs to be " - "generated by torch_ods_gen.py"); - return WalkResult::interrupt(); - } - - for (OpResult result : op.getResults()) - if (failed(checkValueInvariants(&op, result))) - return WalkResult::interrupt(); - } - return WalkResult::advance(); - }); - if (walkResult.wasInterrupted()) - return signalPassFailure(); - } -}; - -} // namespace - -std::unique_ptr> mlir::torch::TorchConversion:: - createVerifyInvariantsBeforeBackendLoweringPass() { - return std::make_unique(); -} diff --git a/test/Dialect/TorchConversion/verify-invariants-before-backend-lowering.mlir b/test/Dialect/TorchConversion/verify-invariants-before-backend-lowering.mlir deleted file mode 100644 index ef918eacd..000000000 --- a/test/Dialect/TorchConversion/verify-invariants-before-backend-lowering.mlir +++ /dev/null @@ -1,36 +0,0 @@ -// RUN: torch-mlir-opt -split-input-file -verify-diagnostics %s -torch-verify-invariants-before-backend-lowering - -// ----- - -func.func @unknown_rank(%arg0: !torch.vtensor<[],f32>) { - // expected-error@+2 {{unsupported by backend lowering: tensor with unknown rank or dtype}} - // expected-note@+1 {{this is likely due to a missing shape transfer function in shape_lib_gen.py}} - %0 = torch.aten.mul.Tensor %arg0, %arg0 : !torch.vtensor<[],f32>, !torch.vtensor<[],f32> -> !torch.vtensor<*,f32> - return -} - -// ----- - -func.func @unknown_dtype(%arg0: !torch.vtensor<[],f32>) { - // expected-error@+2 {{unsupported by backend lowering: tensor with unknown rank or dtype}} - // expected-note@+1 {{this is likely due to a missing shape transfer function in shape_lib_gen.py}} - %0 = torch.aten.mul.Tensor %arg0, %arg0 : !torch.vtensor<[],f32>, !torch.vtensor<[],f32> -> !torch.vtensor<[],unk> - return -} - -// ----- - -func.func @unresolved_operator(%arg0: !torch.vtensor<[],f32>, %arg1: !torch.int) { - // expected-error@+2 {{unsupported by backend lowering: `torch.operator` op}} - // expected-note@+1 {{this is likely due to a missing op that needs to be generated by torch_ods_gen.py}} - torch.operator "aten.mul.Scalar"(%arg0, %arg1) : (!torch.vtensor<[],f32>, !torch.int) -> !torch.vtensor<[],f32> - return -} - -// ----- - -// expected-error@+1 {{unsupported by backend lowering: module initializers}} -torch.global_slot.module_initializer { - torch.initialize.global_slots [ - ] -} diff --git a/utils/bazel/torch-mlir-overlay/BUILD.bazel b/utils/bazel/torch-mlir-overlay/BUILD.bazel index ee3ca0f09..6704c54bf 100644 --- a/utils/bazel/torch-mlir-overlay/BUILD.bazel +++ b/utils/bazel/torch-mlir-overlay/BUILD.bazel @@ -500,7 +500,6 @@ cc_library( "lib/Dialect/TorchConversion/Transforms/BackendTypeConversionPasses.cpp", "lib/Dialect/TorchConversion/Transforms/PassDetail.h", "lib/Dialect/TorchConversion/Transforms/Passes.cpp", - "lib/Dialect/TorchConversion/Transforms/VerifyInvariantsBeforeBackendLowering.cpp", "lib/Dialect/TorchConversion/Transforms/VerifyLinalgOnTensorsBackendContract.cpp", "lib/Dialect/TorchConversion/Transforms/VerifyTosaBackendContract.cpp", ],