Fix stablehlo_legalize_to_vhlo.0_19_0.mlir bytecode (#2077)

Also, use same header in all tests, and avoid command substitutions since they don't work on Windows: #1764 ### Why does `0_19_0` have a different header? I followed the instructions in https://github.com/openxla/stablehlo/blob/main/docs/vhlo.md, except for the part that says `Replace RUN commands`. That is, I copied `stablehlo_legalize_to_vhlo.mlir`, but I forgot to update the RUN commands to match the other versioned tests. IMO, we should avoid duplicating the RUN commands across tests and documentation, better to just instruct developers to copy the latest versioned test and add their tests in there. ### Why was the bytecode incorrect? I serialized the bytecode a while ago. During review, I ended up changing the order of some attributes, which caused the bytecode to be invalid (e.g. the `name` attribute showed up where `composite_attributes` should be).
openxla · Mar 7, 2024 · 6909631 · 6909631
1 parent d85ebed
commit 6909631
Show file tree

Hide file tree

Showing 13 changed files with 74 additions and 33 deletions.
diff --git a/docs/vhlo.md b/docs/vhlo.md
@@ -133,33 +133,35 @@ negative unit tests of the feature, as well as compatibility unit tests.
 
 Compatibility unit testing involves updating [stablehlo_legalize_to_vhlo.mlir](https://github.com/openxla/stablehlo/blob/main/stablehlo/tests/vhlo/stablehlo_legalize_to_vhlo.mlir)
 to ensure that StableHLO round trips with the latest version of VHLO, as well
-as any additional forward or backward compatibility tests required.
-
-A few examples:
-
-* Backward compatibility, positive tests: [vhlo_to_version_upgrade.mlir](https://github.com/openxla/stablehlo/blob/6886b59f6cd4369674e7e3beff61301c145176e2/stablehlo/tests/vhlo_to_version_upgrade.mlir#L2)
-* Forward compatibility, positive tests: [vhlo_to_version_downgrade.mlir](https://github.com/openxla/stablehlo/blob/6886b59f6cd4369674e7e3beff61301c145176e2/stablehlo/tests/vhlo_to_version_downgrade.mlir#L1)
-* Forward compatibility, negative tests: [vhlo_to_version_downgrade_invalid.0_9_0.mlir](https://github.com/openxla/stablehlo/blob/main/stablehlo/tests/vhlo/vhlo_to_version_downgrade_invalid.0_9_0.mlir)
+as any additional forward or backward compatibility tests required. For example,
+if adding a new op at version `X` with `Y = X - 1`, add a test file like
+`vhlo_to_version_downgrade_invalid.0_Y_0.mlir` that shows the op is unsupported
+before version `X`. If adding a new version of an op, add a test file like
+`vhlo_to_version_downgrade.0_Y_0.mlir` that shows that the op can be downgraded
+successfully.
+
+See
+[vhlo_to_version_downgrade_invalid.0_17_0.mlir](https://github.com/openxla/stablehlo/blob/main/stablehlo/tests/vhlo/vhlo_to_version_downgrade_invalid.0_17_0.mlir)
+and
+[vhlo_to_version_downgrade_invalid.0_18_0.mlir](https://github.com/openxla/stablehlo/blob/main/stablehlo/tests/vhlo/vhlo_to_version_downgrade_invalid.0_18_0.mlir)
+for examples of downgrade tests.
+
+If your op has default attributes, include tests that show that the defaults are
+serialized and deserialized correctly.
 
 ### Add Versioned Serialization Test
 
-After adding a test point to `stablehlo_legalize_to_vhlo.mlir`, create a
-versioned copy of the file named `stablehlo_legalize_to_vhlo.0_X_0.mlir` as
-follows, along with a bytecode version of said file with a `.0_X_0.mlir.bc`
-extension. Add [proper FileCheck lines](https://github.com/openxla/stablehlo/blob/main/stablehlo/tests/vhlo/stablehlo_legalize_to_vhlo.0_9_0.mlir#L1-L3)
-for forward and backward compatibility testing.
+After adding tests to `stablehlo_legalize_to_vhlo.mlir`, copy the versioned test
+file with the largest version into a new file at the new version, and add the
+new tests to that file as well. You will also need to create an associated
+bytecode file using `stablehlo-translate`:
 
 ```bash
-$ export TARGET_VERSION=0.X.0
-$ export TARGET_FILENAME=${TARGET_VERSION//./_}
-$ cp stablehlo/tests/vhlo/stablehlo_legalize_to_vhlo.mlir stablehlo/tests/vhlo/stablehlo_legalize_to_vhlo.$TARGET_FILENAME.mlir
-$ build/bin/stablehlo-translate --serialize --target=$TARGET_VERSION --strip-debuginfo stablehlo/tests/vhlo/stablehlo_legalize_to_vhlo.$TARGET_FILENAME.mlir > stablehlo/tests/vhlo/stablehlo_legalize_to_vhlo.$TARGET_FILENAME.mlir.bc
-
-# Replace RUN commands in stablehlo/tests/vhlo/stablehlo_legalize_to_vhlo.0_X_0.mlir with the following for 0.X.0:
-// RUN: stablehlo-opt --mlir-print-op-generic %s.bc | FileCheck %s
-// RUN: stablehlo-translate --deserialize %s.bc | stablehlo-translate --serialize --target=0.X.0 | stablehlo-opt --mlir-print-op-generic | FileCheck %s
-// RUN: diff <(stablehlo-translate --deserialize %s.bc | stablehlo-opt) <(stablehlo-opt --strip-debuginfo %s)
-// RUN: diff %s.bc <(stablehlo-translate --serialize --target=0.X.0 --strip-debuginfo %s)
+export TARGET_VERSION=0.X.0
+export TARGET_FILENAME=${TARGET_VERSION//./_}
+stablehlo-translate --serialize --target=$TARGET_VERSION --strip-debuginfo stablehlo/tests/vhlo/stablehlo_legalize_to_vhlo.$TARGET_FILENAME.mlir > stablehlo/tests/vhlo/stablehlo_legalize_to_vhlo.$TARGET_FILENAME.mlir.bc
 ```
 
-_Example of versioned test in [#1430](https://github.com/openxla/stablehlo/pull/1430)._
+_See [#1856](https://github.com/openxla/stablehlo/pull/1856) and
+[#1986](https://github.com/openxla/stablehlo/pull/1986) for examples of
+adding new VHLO tests._
diff --git a/stablehlo/tests/vhlo/stablehlo_legalize_to_vhlo.0_10_0.mlir b/stablehlo/tests/vhlo/stablehlo_legalize_to_vhlo.0_10_0.mlir
@@ -5,6 +5,10 @@
 // RUN: diff %t.0 %t.1
 // RUN: stablehlo-translate --serialize --target=0.10.0 --strip-debuginfo %s > %t.2
 // RUN: diff %s.bc %t.2
+// RUN: stablehlo-opt --stablehlo-legalize-to-vhlo -emit-bytecode -debug-only=vhlo-bytecode %s 2>&1 | FileCheck --check-prefix=CHECK-WARN %s
+// RUN: stablehlo-opt --stablehlo-legalize-to-vhlo -emit-bytecode %s | stablehlo-opt -debug-only=vhlo-bytecode 2>&1 | FileCheck --check-prefix=CHECK-WARN %s
+
+// CHECK-WARN-NOT: Not Implemented
 
 // ============ ATTRIBUTES ============
 

diff --git a/stablehlo/tests/vhlo/stablehlo_legalize_to_vhlo.0_11_0.mlir b/stablehlo/tests/vhlo/stablehlo_legalize_to_vhlo.0_11_0.mlir
@@ -5,6 +5,10 @@
 // RUN: diff %t.0 %t.1
 // RUN: stablehlo-translate --serialize --target=0.11.0 --strip-debuginfo %s > %t.2
 // RUN: diff %s.bc %t.2
+// RUN: stablehlo-opt --stablehlo-legalize-to-vhlo -emit-bytecode -debug-only=vhlo-bytecode %s 2>&1 | FileCheck --check-prefix=CHECK-WARN %s
+// RUN: stablehlo-opt --stablehlo-legalize-to-vhlo -emit-bytecode %s | stablehlo-opt -debug-only=vhlo-bytecode 2>&1 | FileCheck --check-prefix=CHECK-WARN %s
+
+// CHECK-WARN-NOT: Not Implemented
 
 // ============ ATTRIBUTES ============
 

diff --git a/stablehlo/tests/vhlo/stablehlo_legalize_to_vhlo.0_12_0.mlir b/stablehlo/tests/vhlo/stablehlo_legalize_to_vhlo.0_12_0.mlir
@@ -5,6 +5,10 @@
 // RUN: diff %t.0 %t.1
 // RUN: stablehlo-translate --serialize --target=0.12.0 --strip-debuginfo %s > %t.2
 // RUN: diff %s.bc %t.2
+// RUN: stablehlo-opt --stablehlo-legalize-to-vhlo -emit-bytecode -debug-only=vhlo-bytecode %s 2>&1 | FileCheck --check-prefix=CHECK-WARN %s
+// RUN: stablehlo-opt --stablehlo-legalize-to-vhlo -emit-bytecode %s | stablehlo-opt -debug-only=vhlo-bytecode 2>&1 | FileCheck --check-prefix=CHECK-WARN %s
+
+// CHECK-WARN-NOT: Not Implemented
 
 // ============ ATTRIBUTES ============
 

diff --git a/stablehlo/tests/vhlo/stablehlo_legalize_to_vhlo.0_13_0.mlir b/stablehlo/tests/vhlo/stablehlo_legalize_to_vhlo.0_13_0.mlir
@@ -5,6 +5,10 @@
 // RUN: diff %t.0 %t.1
 // RUN: stablehlo-translate --serialize --target=0.13.0 --strip-debuginfo %s > %t.2
 // RUN: diff %s.bc %t.2
+// RUN: stablehlo-opt --stablehlo-legalize-to-vhlo -emit-bytecode -debug-only=vhlo-bytecode %s 2>&1 | FileCheck --check-prefix=CHECK-WARN %s
+// RUN: stablehlo-opt --stablehlo-legalize-to-vhlo -emit-bytecode %s | stablehlo-opt -debug-only=vhlo-bytecode 2>&1 | FileCheck --check-prefix=CHECK-WARN %s
+
+// CHECK-WARN-NOT: Not Implemented
 
 // ============ ATTRIBUTES ============
 

diff --git a/stablehlo/tests/vhlo/stablehlo_legalize_to_vhlo.0_14_0.mlir b/stablehlo/tests/vhlo/stablehlo_legalize_to_vhlo.0_14_0.mlir
@@ -5,6 +5,10 @@
 // RUN: diff %t.0 %t.1
 // RUN: stablehlo-translate --serialize --target=0.14.0 --strip-debuginfo %s > %t.2
 // RUN: diff %s.bc %t.2
+// RUN: stablehlo-opt --stablehlo-legalize-to-vhlo -emit-bytecode -debug-only=vhlo-bytecode %s 2>&1 | FileCheck --check-prefix=CHECK-WARN %s
+// RUN: stablehlo-opt --stablehlo-legalize-to-vhlo -emit-bytecode %s | stablehlo-opt -debug-only=vhlo-bytecode 2>&1 | FileCheck --check-prefix=CHECK-WARN %s
+
+// CHECK-WARN-NOT: Not Implemented
 
 // ============ ATTRIBUTES ============
 

diff --git a/stablehlo/tests/vhlo/stablehlo_legalize_to_vhlo.0_15_0.mlir b/stablehlo/tests/vhlo/stablehlo_legalize_to_vhlo.0_15_0.mlir
@@ -5,6 +5,8 @@
 // RUN: diff %t.0 %t.1
 // RUN: stablehlo-translate --serialize --target=0.15.0 --strip-debuginfo %s > %t.2
 // RUN: diff %s.bc %t.2
+// RUN: stablehlo-opt --stablehlo-legalize-to-vhlo -emit-bytecode -debug-only=vhlo-bytecode %s 2>&1 | FileCheck --check-prefix=CHECK-WARN %s
+// RUN: stablehlo-opt --stablehlo-legalize-to-vhlo -emit-bytecode %s | stablehlo-opt -debug-only=vhlo-bytecode 2>&1 | FileCheck --check-prefix=CHECK-WARN %s
 
 // CHECK-WARN-NOT: Not Implemented
 

diff --git a/stablehlo/tests/vhlo/stablehlo_legalize_to_vhlo.0_16_0.mlir b/stablehlo/tests/vhlo/stablehlo_legalize_to_vhlo.0_16_0.mlir
@@ -5,7 +5,10 @@
 // RUN: diff %t.0 %t.1
 // RUN: stablehlo-translate --serialize --target=0.16.0 --strip-debuginfo %s > %t.2
 // RUN: diff %s.bc %t.2
+// RUN: stablehlo-opt --stablehlo-legalize-to-vhlo -emit-bytecode -debug-only=vhlo-bytecode %s 2>&1 | FileCheck --check-prefix=CHECK-WARN %s
+// RUN: stablehlo-opt --stablehlo-legalize-to-vhlo -emit-bytecode %s | stablehlo-opt -debug-only=vhlo-bytecode 2>&1 | FileCheck --check-prefix=CHECK-WARN %s
 
+// CHECK-WARN-NOT: Not Implemented
 
 // ============ ATTRIBUTES ============
 

diff --git a/stablehlo/tests/vhlo/stablehlo_legalize_to_vhlo.0_17_0.mlir b/stablehlo/tests/vhlo/stablehlo_legalize_to_vhlo.0_17_0.mlir
@@ -1,7 +1,12 @@
 // RUN: stablehlo-opt --mlir-print-op-generic %s.bc | FileCheck %s
 // RUN: stablehlo-translate --deserialize %s.bc | stablehlo-translate --serialize --target=0.17.0 | stablehlo-opt --mlir-print-op-generic | FileCheck %s
-// RUN: diff <(stablehlo-translate --deserialize %s.bc | stablehlo-opt) <(stablehlo-opt --strip-debuginfo %s)
-// RUN: diff %s.bc <(stablehlo-translate --serialize --target=0.17.0 --strip-debuginfo %s)
+// RUN: stablehlo-translate --deserialize %s.bc | stablehlo-opt > %t.0
+// RUN: stablehlo-opt --strip-debuginfo %s > %t.1
+// RUN: diff %t.0 %t.1
+// RUN: stablehlo-translate --serialize --target=0.17.0 --strip-debuginfo %s > %t.2
+// RUN: diff %s.bc %t.2
+// RUN: stablehlo-opt --stablehlo-legalize-to-vhlo -emit-bytecode -debug-only=vhlo-bytecode %s 2>&1 | FileCheck --check-prefix=CHECK-WARN %s
+// RUN: stablehlo-opt --stablehlo-legalize-to-vhlo -emit-bytecode %s | stablehlo-opt -debug-only=vhlo-bytecode 2>&1 | FileCheck --check-prefix=CHECK-WARN %s
 
 // CHECK-WARN-NOT: Not Implemented
 

diff --git a/stablehlo/tests/vhlo/stablehlo_legalize_to_vhlo.0_18_0.mlir b/stablehlo/tests/vhlo/stablehlo_legalize_to_vhlo.0_18_0.mlir
@@ -1,7 +1,12 @@
 // RUN: stablehlo-opt --mlir-print-op-generic %s.bc | FileCheck %s
 // RUN: stablehlo-translate --deserialize %s.bc | stablehlo-translate --serialize --target=0.18.0 | stablehlo-opt --mlir-print-op-generic | FileCheck %s
-// RUN: diff <(stablehlo-translate --deserialize %s.bc | stablehlo-opt) <(stablehlo-opt --strip-debuginfo %s)
-// RUN: diff %s.bc <(stablehlo-translate --serialize --target=0.18.0 --strip-debuginfo %s)
+// RUN: stablehlo-translate --deserialize %s.bc | stablehlo-opt > %t.0
+// RUN: stablehlo-opt --strip-debuginfo %s > %t.1
+// RUN: diff %t.0 %t.1
+// RUN: stablehlo-translate --serialize --target=0.18.0 --strip-debuginfo %s > %t.2
+// RUN: diff %s.bc %t.2
+// RUN: stablehlo-opt --stablehlo-legalize-to-vhlo -emit-bytecode -debug-only=vhlo-bytecode %s 2>&1 | FileCheck --check-prefix=CHECK-WARN %s
+// RUN: stablehlo-opt --stablehlo-legalize-to-vhlo -emit-bytecode %s | stablehlo-opt -debug-only=vhlo-bytecode 2>&1 | FileCheck --check-prefix=CHECK-WARN %s
 
 // CHECK-WARN-NOT: Not Implemented
 

diff --git a/stablehlo/tests/vhlo/stablehlo_legalize_to_vhlo.0_19_0.mlir b/stablehlo/tests/vhlo/stablehlo_legalize_to_vhlo.0_19_0.mlir
@@ -1,10 +1,10 @@
-// RUN: stablehlo-opt --stablehlo-legalize-to-vhlo --mlir-print-op-generic --split-input-file %s | FileCheck %s
-// RUN: stablehlo-translate --serialize --target=current %s | stablehlo-translate --deserialize | stablehlo-opt > %t.0
-// RUN: stablehlo-opt %s > %t.1
-// RUN: diff %t.0 %t.1
-// RUN: stablehlo-translate --serialize --target=current %s | stablehlo-opt --pass-pipeline='builtin.module(stablehlo-deserialize)' > %t.0
-// RUN: stablehlo-opt %s > %t.1
+// RUN: stablehlo-opt --mlir-print-op-generic %s.bc | FileCheck %s
+// RUN: stablehlo-translate --deserialize %s.bc | stablehlo-translate --serialize --target=0.19.0 | stablehlo-opt --mlir-print-op-generic | FileCheck %s
+// RUN: stablehlo-translate --deserialize %s.bc | stablehlo-opt > %t.0
+// RUN: stablehlo-opt --strip-debuginfo %s > %t.1
 // RUN: diff %t.0 %t.1
+// RUN: stablehlo-translate --serialize --target=0.19.0 --strip-debuginfo %s > %t.2
+// RUN: diff %s.bc %t.2
 // RUN: stablehlo-opt --stablehlo-legalize-to-vhlo -emit-bytecode -debug-only=vhlo-bytecode %s 2>&1 | FileCheck --check-prefix=CHECK-WARN %s
 // RUN: stablehlo-opt --stablehlo-legalize-to-vhlo -emit-bytecode %s | stablehlo-opt -debug-only=vhlo-bytecode 2>&1 | FileCheck --check-prefix=CHECK-WARN %s
 

diff --git a/stablehlo/tests/vhlo/stablehlo_legalize_to_vhlo.0_19_0.mlir.bc b/stablehlo/tests/vhlo/stablehlo_legalize_to_vhlo.0_19_0.mlir.bc
diff --git a/stablehlo/tests/vhlo/stablehlo_legalize_to_vhlo.0_9_0.mlir b/stablehlo/tests/vhlo/stablehlo_legalize_to_vhlo.0_9_0.mlir
@@ -5,6 +5,10 @@
 // RUN: diff %t.0 %t.1
 // RUN: stablehlo-translate --serialize --target=0.9.0 --strip-debuginfo %s > %t.2
 // RUN: diff %s.bc %t.2
+// RUN: stablehlo-opt --stablehlo-legalize-to-vhlo -emit-bytecode -debug-only=vhlo-bytecode %s 2>&1 | FileCheck --check-prefix=CHECK-WARN %s
+// RUN: stablehlo-opt --stablehlo-legalize-to-vhlo -emit-bytecode %s | stablehlo-opt -debug-only=vhlo-bytecode 2>&1 | FileCheck --check-prefix=CHECK-WARN %s
+
+// CHECK-WARN-NOT: Not Implemented
 
 // ============ ATTRIBUTES ============