[mlir][dxsa] Add gather4 variants#203
Conversation
There was a problem hiding this comment.
With the latest PRs, I've introduced a new approach to test files that was approved by Anton.
Can we combine all gather-related tests to a single test file gather_ops.test and run mlir-translate with --split-input-file option?
atomic_ops.test
// RUN: mlir-translate --split-input-file --import-dxsa-hex %s | FileCheck %s
// RUN: mlir-translate --split-input-file --import-dxsa-hex %s | mlir-opt --split-input-file --verify-roundtrip
// CHECK-LABEL: dxsa.module {
// CHECK-NEXT: dxsa.atomic_and u<0>, r<0>, r<1, <x>>
// CHECK-NEXT: }
0x070000a9, 0x0011e000, 0x00000000, 0x00100e46, 0x00000000, 0x0010000a, 0x00000001
// -----
// CHECK-LABEL: dxsa.module {
// CHECK-NEXT: dxsa.atomic_and g<0>, r<2>, r<3, <x>>
// CHECK-NEXT: }
0x070000a9, 0x0011f000, 0x00000000, 0x00100e46, 0x00000002, 0x0010000a, 0x00000003There was a problem hiding this comment.
What is the reason to split instructions into individual modules?
There was a problem hiding this comment.
A couple of reasons, most of them about making failures point at a single instruction rather than at a blob:
-
Per-variant round-trip. The --verify-roundtrip run is the main motivation. In one big dxsa.module the first round-trip mismatch aborts the whole check, so you never learn whether the remaining variants still round-trip. With --split-input-file each variant is verified independently, so a regression lands on exactly one block.
-
Independent import. If one opcode mis-decodes, it can shift the token stream for everything that follows in the same module and produce misleading cascade failures. Isolated modules keep a decode bug contained to its own case.
So the split is not about isolating the ops conceptually, it's about isolating failures per variant while keeping the whole family in a single reviewable file.
There was a problem hiding this comment.
Agree. Let's split the tests if we encounter any issues next time.
0a8f477 to
208f9cc
Compare
0e35e7c to
a84755b
Compare
a84755b to
6e8e51c
Compare
Please ignore the first commit in the stack. It is reviewed separately in #182.