From c6c158323db8075b1b06921ee5c6a2cc1fb5c78b Mon Sep 17 00:00:00 2001 From: Kevin Gleason Date: Tue, 1 Oct 2024 14:50:39 -0400 Subject: [PATCH] Improve forward incompatibility error messaging (#2569) Now failure to `--deserialize` a portable artifact will include: 1. The op that failed. 2. The version that the portable artifact was serialized for. 3. The current version of StableHLO. This should help debugging compat issues at a glance. Added a forward incompatible hypothetical feature `vhlo.constant_v99`, serialized for `StableHLO_v2.0.0`, which emulates a current version of StableHLO trying to parse a future operation that doesn't currently exist. ``` $ stablehlo-translate --deserialize file.mlirbc unregistered operation 'vhlo.constant_v99' found in dialect ('vhlo') that does not allow unknown operations note: in bytecode version 6 produced by: StableHLO_v2.0.0 failed to deserialize portable artifact using StableHLO_v1.7.5 ``` --- stablehlo/dialect/Serialization.cpp | 5 +++++ stablehlo/tests/vhlo/invalid_vhlo_future.mlir | 14 ++++++++++++++ stablehlo/tests/vhlo/invalid_vhlo_future.mlir.bc | Bin 0 -> 243 bytes .../vhlo_to_version_downgrade_invalid.0_10_0.mlir | 1 + .../vhlo_to_version_downgrade_invalid.0_15_0.mlir | 1 + .../vhlo_to_version_downgrade_invalid.0_16_0.mlir | 6 ++++++ .../vhlo_to_version_downgrade_invalid.0_17_0.mlir | 1 + .../vhlo_to_version_downgrade_invalid.0_18_0.mlir | 1 + .../vhlo_to_version_downgrade_invalid.0_9_0.mlir | 2 ++ .../vhlo_to_version_downgrade_invalid.1_1_0.mlir | 2 ++ .../vhlo_to_version_downgrade_invalid.1_2_0.mlir | 2 ++ .../vhlo_to_version_downgrade_invalid.1_3_0.mlir | 1 + .../vhlo_to_version_downgrade_invalid.1_4_0.mlir | 4 +++- .../vhlo_to_version_downgrade_invalid.1_5_0.mlir | 3 +++ .../vhlo_to_version_downgrade_invalid.1_6_0.mlir | 2 ++ stablehlo/transforms/VhloToVersion.cpp | 5 ++++- 16 files changed, 48 insertions(+), 2 deletions(-) create mode 100644 stablehlo/tests/vhlo/invalid_vhlo_future.mlir create mode 100644 stablehlo/tests/vhlo/invalid_vhlo_future.mlir.bc diff --git a/stablehlo/dialect/Serialization.cpp b/stablehlo/dialect/Serialization.cpp index 8ec16a0989..3da045dad9 100644 --- a/stablehlo/dialect/Serialization.cpp +++ b/stablehlo/dialect/Serialization.cpp @@ -17,6 +17,8 @@ limitations under the License. #include "mlir/Bytecode/BytecodeWriter.h" #include "mlir/IR/BuiltinOps.h" +#include "mlir/IR/Diagnostics.h" +#include "mlir/IR/Location.h" #include "mlir/IR/MLIRContext.h" #include "mlir/IR/OwningOpRef.h" #include "mlir/Parser/Parser.h" @@ -71,6 +73,9 @@ OwningOpRef deserializePortableArtifact(StringRef sourceStr, context->loadDialect(); auto module = parseSourceString(sourceStr, context); if (!module) { + emitError(UnknownLoc::get(context)) + << "failed to deserialize portable artifact using StableHLO_v" + << vhlo::Version::getCurrentVersion(); return nullptr; } diff --git a/stablehlo/tests/vhlo/invalid_vhlo_future.mlir b/stablehlo/tests/vhlo/invalid_vhlo_future.mlir new file mode 100644 index 0000000000..946c5ffbce --- /dev/null +++ b/stablehlo/tests/vhlo/invalid_vhlo_future.mlir @@ -0,0 +1,14 @@ +// RUN: not stablehlo-translate --deserialize %s.bc --verify-diagnostics 2>&1 | FileCheck %s +// +// Note: This file is not valid to parse since VHLO doesn't support unknown +// operations, but is kept around to help visualize the bytecode file in test. +// The bytecode file should not break, as it is a portable artifact with full +// backward compatibility. + +// CHECK: error: unregistered operation 'vhlo.constant_v99' found in dialect ('vhlo') that does not allow unknown operations +// CHECK: note: in bytecode version 6 produced by: StableHLO_v2.0.0 +// CHECK: error: failed to deserialize portable artifact using StableHLO_v{{.*}} +vhlo.func_v1 @main() -> (!vhlo.tensor_v1) { + %0 = "vhlo.constant_v99"() <{value = #vhlo.tensor_v1 : tensor>}> : () -> !vhlo.tensor_v1 + "vhlo.return_v1"(%0) : (!vhlo.tensor_v1) -> () +} {arg_attrs = #vhlo.array_v1<[]>, res_attrs = #vhlo.array_v1<[]>, sym_visibility = #vhlo.string_v1<"public">} diff --git a/stablehlo/tests/vhlo/invalid_vhlo_future.mlir.bc b/stablehlo/tests/vhlo/invalid_vhlo_future.mlir.bc new file mode 100644 index 0000000000000000000000000000000000000000..73c356962bcd7ad20bba079920f91b302d1625e7 GIT binary patch literal 243 zcmWlU&1!={6h`mu^>#80GKk0!f@INEDVU#KRwZ3^k) -> tensor { %0 = stablehlo.add %arg0, %arg0 : tensor diff --git a/stablehlo/tests/vhlo/vhlo_to_version_downgrade_invalid.0_15_0.mlir b/stablehlo/tests/vhlo/vhlo_to_version_downgrade_invalid.0_15_0.mlir index d3417f65cb..2d70a8fb6c 100644 --- a/stablehlo/tests/vhlo/vhlo_to_version_downgrade_invalid.0_15_0.mlir +++ b/stablehlo/tests/vhlo/vhlo_to_version_downgrade_invalid.0_15_0.mlir @@ -1,5 +1,6 @@ // RUN: stablehlo-opt --stablehlo-legalize-to-vhlo --vhlo-to-version='target=0.15.0' --verify-diagnostics --split-input-file %s +// expected-error @-3 {{failed to convert VHLO to v0.15.0}} func.func @default_collective_broadcast(%arg0: tensor<16x8xf32>) -> tensor<16x8xf32> { // expected-error @+1 {{failed to legalize operation 'vhlo.collective_broadcast_v1' that was explicitly marked illegal}} %0 = "stablehlo.collective_broadcast"(%arg0) { diff --git a/stablehlo/tests/vhlo/vhlo_to_version_downgrade_invalid.0_16_0.mlir b/stablehlo/tests/vhlo/vhlo_to_version_downgrade_invalid.0_16_0.mlir index 2d4f0816dc..3ee34ec4a9 100644 --- a/stablehlo/tests/vhlo/vhlo_to_version_downgrade_invalid.0_16_0.mlir +++ b/stablehlo/tests/vhlo/vhlo_to_version_downgrade_invalid.0_16_0.mlir @@ -1,5 +1,6 @@ // RUN: stablehlo-opt --stablehlo-legalize-to-vhlo --vhlo-to-version='target=0.16.0' --verify-diagnostics --split-input-file %s +// expected-error @-3 {{failed to convert VHLO to v0.16.0}} func.func @reduce_with_promotable_types(%arg0: tensor<4x4xf32>, %arg1 : tensor) -> (tensor<4xf64>) { @@ -17,6 +18,7 @@ func.func @reduce_with_promotable_types(%arg0: tensor<4x4xf32>, %arg1 : tensor) -> tensor { // expected-error @+1 {{failed to legalize operation 'vhlo.all_reduce_v2' that was explicitly marked illegal}} @@ -34,6 +36,7 @@ func.func @all_reduce_with_promotable_types(%operand: tensor) -> tensor) -> tensor<4x4xf64> { // expected-error @+1 {{failed to legalize operation 'vhlo.reduce_scatter_v1' that was explicitly marked illegal}} @@ -50,6 +53,7 @@ func.func @reduce_scatter_with_promotable_types(%data: tensor<4x16xf32>) -> tens // ----- +// expected-error @-3 {{failed to convert VHLO to v0.16.0}} func.func @reduce_window_with_promotable_types(%arg0: tensor<4x2xf32>, %arg1: tensor<4x2xf32>, %init0: tensor, %init1: tensor) -> (tensor<2x2xf64>, tensor<2x2xf32>) { @@ -72,6 +76,7 @@ func.func @reduce_window_with_promotable_types(%arg0: tensor<4x2xf32>, // ----- +// expected-error @-3 {{failed to convert VHLO to v0.16.0}} func.func @scatter_with_promotable_types(%input_tensor: tensor<200x100x300xf32>, %scatter_indices: tensor<10x2xi32>, %updates: tensor<10x300xf32>) -> tensor<200x100x300xf64> { @@ -97,6 +102,7 @@ func.func @scatter_with_promotable_types(%input_tensor: tensor<200x100x300xf32>, // ----- +// expected-error @-3 {{failed to convert VHLO to v0.16.0}} func.func @select_and_scatter_with_promotable_types( %arg0: tensor<10x24x24x64xf32>, %arg1: tensor<10x12x12x64xf32>) -> () { diff --git a/stablehlo/tests/vhlo/vhlo_to_version_downgrade_invalid.0_17_0.mlir b/stablehlo/tests/vhlo/vhlo_to_version_downgrade_invalid.0_17_0.mlir index 914e339b95..351677c4d7 100644 --- a/stablehlo/tests/vhlo/vhlo_to_version_downgrade_invalid.0_17_0.mlir +++ b/stablehlo/tests/vhlo/vhlo_to_version_downgrade_invalid.0_17_0.mlir @@ -1,5 +1,6 @@ // RUN: stablehlo-opt --stablehlo-legalize-to-vhlo --vhlo-to-version='target=0.17.0' --verify-diagnostics --split-input-file %s +// expected-error @-3 {{failed to convert VHLO to v0.17.0}} // expected-error @+1 {{failed to legalize operation 'vhlo.func_v1' that was explicitly marked illegal}} func.func @type_per_axis_quantization(%arg0: tensor<2x!quant.uniform>) -> tensor<2x!quant.uniform> { %0 = stablehlo.add %arg0, %arg0 : tensor<2x!quant.uniform> diff --git a/stablehlo/tests/vhlo/vhlo_to_version_downgrade_invalid.0_18_0.mlir b/stablehlo/tests/vhlo/vhlo_to_version_downgrade_invalid.0_18_0.mlir index 602ddbf2bf..2455092bdc 100644 --- a/stablehlo/tests/vhlo/vhlo_to_version_downgrade_invalid.0_18_0.mlir +++ b/stablehlo/tests/vhlo/vhlo_to_version_downgrade_invalid.0_18_0.mlir @@ -1,5 +1,6 @@ // RUN: stablehlo-opt --stablehlo-legalize-to-vhlo --vhlo-to-version='target=0.18.0' --verify-diagnostics --split-input-file %s +// expected-error @-3 {{failed to convert VHLO to v0.18.0}} func.func @composite(%arg0: tensor) -> tensor { // expected-error @+1 {{failed to legalize operation 'vhlo.composite_v1' that was explicitly marked illegal}} %0 = "stablehlo.composite"(%arg0) { diff --git a/stablehlo/tests/vhlo/vhlo_to_version_downgrade_invalid.0_9_0.mlir b/stablehlo/tests/vhlo/vhlo_to_version_downgrade_invalid.0_9_0.mlir index 23585f1c24..fb15f3b0f3 100644 --- a/stablehlo/tests/vhlo/vhlo_to_version_downgrade_invalid.0_9_0.mlir +++ b/stablehlo/tests/vhlo/vhlo_to_version_downgrade_invalid.0_9_0.mlir @@ -1,5 +1,6 @@ // RUN: stablehlo-opt --stablehlo-legalize-to-vhlo --vhlo-to-version='target=0.9.0' --verify-diagnostics --split-input-file %s +// expected-error @-3 {{failed to convert VHLO to v0.9.0}} // expected-error @+1 {{failed to legalize operation 'vhlo.func_v1' that was explicitly marked illegal}} func.func @type_fp8_E5M2FNUZ(%arg0: tensor) -> tensor { %0 = stablehlo.add %arg0, %arg0 : tensor @@ -8,6 +9,7 @@ func.func @type_fp8_E5M2FNUZ(%arg0: tensor) -> tensor { // ----- +// expected-error @-3 {{failed to convert VHLO to v0.9.0}} // expected-error @+1 {{failed to legalize operation 'vhlo.func_v1' that was explicitly marked illegal}} func.func @type_fp8_E4M3FNUZ(%arg0: tensor) -> tensor { %0 = stablehlo.add %arg0, %arg0 : tensor diff --git a/stablehlo/tests/vhlo/vhlo_to_version_downgrade_invalid.1_1_0.mlir b/stablehlo/tests/vhlo/vhlo_to_version_downgrade_invalid.1_1_0.mlir index e2d876ff9a..c1347a8058 100644 --- a/stablehlo/tests/vhlo/vhlo_to_version_downgrade_invalid.1_1_0.mlir +++ b/stablehlo/tests/vhlo/vhlo_to_version_downgrade_invalid.1_1_0.mlir @@ -1,5 +1,6 @@ // RUN: stablehlo-opt --stablehlo-legalize-to-vhlo --vhlo-to-version='target=1.1.0' --verify-diagnostics --split-input-file %s +// expected-error @-3 {{failed to convert VHLO to v1.1.0}} // expected-error @+1 {{failed to legalize operation 'vhlo.func_v1' that was explicitly marked illegal}} func.func @type_i2(%arg0: tensor) -> tensor { %0 = stablehlo.add %arg0, %arg0 : tensor @@ -8,6 +9,7 @@ func.func @type_i2(%arg0: tensor) -> tensor { // ----- +// expected-error @-3 {{failed to convert VHLO to v1.1.0}} // expected-error @+1 {{failed to legalize operation 'vhlo.func_v1' that was explicitly marked illegal}} func.func @type_ui2(%arg0: tensor) -> tensor { %0 = stablehlo.add %arg0, %arg0 : tensor diff --git a/stablehlo/tests/vhlo/vhlo_to_version_downgrade_invalid.1_2_0.mlir b/stablehlo/tests/vhlo/vhlo_to_version_downgrade_invalid.1_2_0.mlir index f1314f92c6..a1fbe3d539 100644 --- a/stablehlo/tests/vhlo/vhlo_to_version_downgrade_invalid.1_2_0.mlir +++ b/stablehlo/tests/vhlo/vhlo_to_version_downgrade_invalid.1_2_0.mlir @@ -1,5 +1,6 @@ // RUN: stablehlo-opt --stablehlo-legalize-to-vhlo --vhlo-to-version='target=1.2.0' --verify-diagnostics --split-input-file %s +// expected-error @-3 {{failed to convert VHLO to v1.2.0}} func.func @custom_call_dictionary_attr(%arg0: tensor) -> tensor { // expected-error @+1 {{failed to legalize operation 'vhlo.custom_call_v1' that was explicitly marked illegal}} %0 = "stablehlo.custom_call"(%arg0) { @@ -12,6 +13,7 @@ func.func @custom_call_dictionary_attr(%arg0: tensor) -> tensor { // ----- +// expected-error @-3 {{failed to convert VHLO to v1.2.0}} func.func @custom_call_dictionary_attr(%arg0: tensor) -> tensor { // expected-error @+1 {{failed to legalize operation 'vhlo.custom_call_v1' that was explicitly marked illegal}} %0 = "stablehlo.custom_call"(%arg0) { diff --git a/stablehlo/tests/vhlo/vhlo_to_version_downgrade_invalid.1_3_0.mlir b/stablehlo/tests/vhlo/vhlo_to_version_downgrade_invalid.1_3_0.mlir index f8b749e69a..18ad2b4359 100644 --- a/stablehlo/tests/vhlo/vhlo_to_version_downgrade_invalid.1_3_0.mlir +++ b/stablehlo/tests/vhlo/vhlo_to_version_downgrade_invalid.1_3_0.mlir @@ -1,5 +1,6 @@ // RUN: stablehlo-opt --stablehlo-legalize-to-vhlo --vhlo-to-version='target=1.3.0' --verify-diagnostics --split-input-file %s +// expected-error @-3 {{failed to convert VHLO to v1.3.0}} func.func @op_tan(%arg0: tensor) -> tensor { // expected-error @+1 {{failed to legalize operation 'vhlo.tan_v1' that was explicitly marked illegal}} %0 = "stablehlo.tan"(%arg0) : (tensor) -> tensor diff --git a/stablehlo/tests/vhlo/vhlo_to_version_downgrade_invalid.1_4_0.mlir b/stablehlo/tests/vhlo/vhlo_to_version_downgrade_invalid.1_4_0.mlir index c1b337a965..b6a497e6f6 100644 --- a/stablehlo/tests/vhlo/vhlo_to_version_downgrade_invalid.1_4_0.mlir +++ b/stablehlo/tests/vhlo/vhlo_to_version_downgrade_invalid.1_4_0.mlir @@ -1,5 +1,6 @@ // RUN: stablehlo-opt --stablehlo-legalize-to-vhlo --vhlo-to-version='target=1.4.0' --verify-diagnostics --split-input-file %s +// expected-error @-3 {{failed to convert VHLO to v1.4.0}} func.func @all_reduce_variadic(%arg0: tensor, %arg1: tensor) -> (tensor, tensor) { // expected-error @+1 {{failed to legalize operation 'vhlo.all_reduce_v2' that was explicitly marked illegal}} %0:2 = "stablehlo.all_reduce"(%arg0, %arg1) ({ @@ -14,7 +15,7 @@ func.func @all_reduce_variadic(%arg0: tensor, %arg1: tensor) -> (tenso // ----- - +// expected-error @-3 {{failed to convert VHLO to v1.4.0}} func.func @all_gather_variadic(%arg0: tensor<16x8xf32>, %arg1: tensor<16x8xf32>) -> (tensor<16x16xf32>, tensor<16x16xf32>) { // expected-error @+1 {{failed to legalize operation 'vhlo.all_gather_v2' that was explicitly marked illegal}} %0:2 = "stablehlo.all_gather"(%arg0, %arg1) { @@ -26,6 +27,7 @@ func.func @all_gather_variadic(%arg0: tensor<16x8xf32>, %arg1: tensor<16x8xf32>) // ----- +// expected-error @-3 {{failed to convert VHLO to v1.4.0}} func.func @all_to_all_variadic(%arg0: tensor<4x16xf32>, %arg1: tensor<5x16xf32>) -> (tensor<16x4xf32>, tensor<20x4xf32>) { // expected-error @+1 {{failed to legalize operation 'vhlo.all_to_all_v2' that was explicitly marked illegal}} %0:2 = "stablehlo.all_to_all"(%arg0, %arg1) { diff --git a/stablehlo/tests/vhlo/vhlo_to_version_downgrade_invalid.1_5_0.mlir b/stablehlo/tests/vhlo/vhlo_to_version_downgrade_invalid.1_5_0.mlir index f9e003c3bb..23592af1fc 100644 --- a/stablehlo/tests/vhlo/vhlo_to_version_downgrade_invalid.1_5_0.mlir +++ b/stablehlo/tests/vhlo/vhlo_to_version_downgrade_invalid.1_5_0.mlir @@ -1,5 +1,6 @@ // RUN: stablehlo-opt --stablehlo-legalize-to-vhlo --vhlo-to-version='target=1.5.0' --verify-diagnostics --split-input-file %s +// expected-error @-3 {{failed to convert VHLO to v1.5.0}} func.func @dot_general_algorithm(%arg0: tensor<2x2x2xi64>, %arg1: tensor<2x2x2xi64>) -> tensor<2x2x2xi64> { // expected-error @+1 {{failed to legalize operation 'vhlo.dot_general_v2' that was explicitly marked illegal}} %0 = "stablehlo.dot_general"(%arg0, %arg1) <{ @@ -11,6 +12,7 @@ func.func @dot_general_algorithm(%arg0: tensor<2x2x2xi64>, %arg1: tensor<2x2x2xi // ----- +// expected-error @-3 {{failed to convert VHLO to v1.5.0}} // expected-error @+1 {{failed to legalize operation 'vhlo.func_v1' that was explicitly marked illegal}} func.func @none_type() attributes {stablehlo.attr = none } { return @@ -18,6 +20,7 @@ func.func @none_type() attributes {stablehlo.attr = none } { // ----- +// expected-error @-3 {{failed to convert VHLO to v1.5.0}} // expected-error @+1 {{failed to legalize operation 'vhlo.func_v1' that was explicitly marked illegal}} func.func @tf32_type() attributes {stablehlo.attr = tf32 } { return diff --git a/stablehlo/tests/vhlo/vhlo_to_version_downgrade_invalid.1_6_0.mlir b/stablehlo/tests/vhlo/vhlo_to_version_downgrade_invalid.1_6_0.mlir index c093a0b990..26ff88b8a7 100644 --- a/stablehlo/tests/vhlo/vhlo_to_version_downgrade_invalid.1_6_0.mlir +++ b/stablehlo/tests/vhlo/vhlo_to_version_downgrade_invalid.1_6_0.mlir @@ -1,5 +1,6 @@ // RUN: stablehlo-opt --stablehlo-legalize-to-vhlo --vhlo-to-version='target=1.6.0' --verify-diagnostics --split-input-file %s +// expected-error @-3 {{failed to convert VHLO to v1.6.0}} // expected-error @+1 {{failed to legalize operation 'vhlo.func_v1' that was explicitly marked illegal}} func.func @type_f8E4M3(%arg0: tensor) -> tensor { %0 = stablehlo.add %arg0, %arg0 : tensor @@ -8,6 +9,7 @@ func.func @type_f8E4M3(%arg0: tensor) -> tensor { // ----- +// expected-error @-3 {{failed to convert VHLO to v1.6.0}} // expected-error @+1 {{failed to legalize operation 'vhlo.func_v1' that was explicitly marked illegal}} func.func @type_f8E3M4(%arg0: tensor) -> tensor { %0 = stablehlo.add %arg0, %arg0 : tensor diff --git a/stablehlo/transforms/VhloToVersion.cpp b/stablehlo/transforms/VhloToVersion.cpp index 5d0e0ec9aa..02b956617e 100644 --- a/stablehlo/transforms/VhloToVersion.cpp +++ b/stablehlo/transforms/VhloToVersion.cpp @@ -258,8 +258,11 @@ struct VhloToVersionPass : public VhloToVersionPassBase { }); // Conversions within VHLO may fail if new features or ops are used. - if (failed(applyPartialConversion(getOperation(), target, patterns))) + if (failed(applyPartialConversion(getOperation(), target, patterns))) { + getOperation()->emitError() + << "failed to convert VHLO to v" << targetVersion; return signalPassFailure(); + } } private: