Improve forward incompatibility error messaging (#2569)

GleasonK · web-flow · commit c6c158323db8 · 2024-10-01T14:50:39.000-04:00
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
```
diff --git 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<ModuleOp> deserializePortableArtifact(StringRef sourceStr,
   context->loadDialect<vhlo::VhloDialect>();
   auto module = parseSourceString<ModuleOp>(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
@@ -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<!vhlo.f32_v1>) {
+  %0 = "vhlo.constant_v99"() <{value = #vhlo.tensor_v1<dense<1.000000e+00> : tensor<f32>>}> : () -> !vhlo.tensor_v1<!vhlo.f32_v1>
+  "vhlo.return_v1"(%0) : (!vhlo.tensor_v1<!vhlo.f32_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
diff --git a/stablehlo/tests/vhlo/vhlo_to_version_downgrade_invalid.0_10_0.mlir b/stablehlo/tests/vhlo/vhlo_to_version_downgrade_invalid.0_10_0.mlir
@@ -1,5 +1,6 @@
 // RUN: stablehlo-opt --stablehlo-legalize-to-vhlo --vhlo-to-version='target=0.10.0' --verify-diagnostics --split-input-file %s
 
+// expected-error @-3 {{failed to convert VHLO to v0.10.0}}
 // expected-error @+1 {{failed to legalize operation 'vhlo.func_v1' that was explicitly marked illegal}}
 func.func @type_fp8_E4M3B11FNUZ(%arg0: tensor<f8E4M3B11FNUZ>) -> tensor<f8E4M3B11FNUZ> {
   %0 = stablehlo.add %arg0, %arg0 : tensor<f8E4M3B11FNUZ>
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
@@ -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
@@ -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<f32>)
     -> (tensor<4xf64>) {
 
@@ -17,6 +18,7 @@ func.func @reduce_with_promotable_types(%arg0: tensor<4x4xf32>, %arg1 : tensor<f
 
 // -----
 
+// expected-error @-3 {{failed to convert VHLO to v0.16.0}}
 func.func @all_reduce_with_promotable_types(%operand: tensor<f32>) -> tensor<f64> {
 
   // 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<f32>) -> tensor<f64
 
 // -----
 
+// expected-error @-3 {{failed to convert VHLO to v0.16.0}}
 func.func @reduce_scatter_with_promotable_types(%data: tensor<4x16xf32>) -> 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<f32>, %init1: tensor<f32>) ->
     (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
@@ -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<i8:f32:0, {34.0:16, 34.0:16}>>) -> tensor<2x!quant.uniform<i8:f32:0, {34.0:16, 34.0:16}>> {
   %0 = stablehlo.add %arg0, %arg0 : tensor<2x!quant.uniform<i8:f32:0, {34.0:16, 34.0:16}>>
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
@@ -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<f32>) -> tensor<f32> {
   // 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
@@ -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<f8E5M2FNUZ>) -> tensor<f8E5M2FNUZ> {
   %0 = stablehlo.add %arg0, %arg0 : tensor<f8E5M2FNUZ>
@@ -8,6 +9,7 @@ func.func @type_fp8_E5M2FNUZ(%arg0: tensor<f8E5M2FNUZ>) -> tensor<f8E5M2FNUZ> {
 
 // -----
 
+// 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<f8E4M3FNUZ>) -> tensor<f8E4M3FNUZ> {
   %0 = stablehlo.add %arg0, %arg0 : tensor<f8E4M3FNUZ>
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
@@ -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<i2>) -> tensor<i2> {
   %0 = stablehlo.add %arg0, %arg0 : tensor<i2>
@@ -8,6 +9,7 @@ func.func @type_i2(%arg0: tensor<i2>) -> tensor<i2> {
 
 // -----
 
+// 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<ui2>) -> tensor<ui2> {
   %0 = stablehlo.add %arg0, %arg0 : tensor<ui2>
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
@@ -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<f32>) -> tensor<f32> {
 // 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<f32>) -> tensor<f32> {
 
 // -----
 
+// expected-error @-3 {{failed to convert VHLO to v1.2.0}}
 func.func @custom_call_dictionary_attr(%arg0: tensor<f32>) -> tensor<f32> {
 // 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
@@ -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<f32>) -> tensor<f32> {
 // expected-error @+1 {{failed to legalize operation 'vhlo.tan_v1' that was explicitly marked illegal}}
   %0 = "stablehlo.tan"(%arg0) : (tensor<f32>) -> tensor<f32>
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
@@ -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<f32>, %arg1: tensor<f32>) -> (tensor<f32>, tensor<f32>) {
   // 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<f32>, %arg1: tensor<f32>) -> (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
@@ -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,13 +12,15 @@ 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
 }
 
 // -----
 
+// 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
@@ -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<f8E4M3>) -> tensor<f8E4M3> {
   %0 = stablehlo.add %arg0, %arg0 : tensor<f8E4M3>
@@ -8,6 +9,7 @@ func.func @type_f8E4M3(%arg0: tensor<f8E4M3>) -> tensor<f8E4M3> {
 
 // -----
 
+// 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<f8E3M4>) -> tensor<f8E3M4> {
   %0 = stablehlo.add %arg0, %arg0 : tensor<f8E3M4>
diff --git a/stablehlo/transforms/VhloToVersion.cpp b/stablehlo/transforms/VhloToVersion.cpp
@@ -258,8 +258,11 @@ struct VhloToVersionPass : public VhloToVersionPassBase<VhloToVersionPass> {
         });
 
     // 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:
