Decompose torch.slice_scatter

[tanyokwok]

No description provided.

[silvasean]

You can delete this code as well:

torch-mlir/lib/Conversion/TorchToLinalg/DataMovement.cpp

Line 1292 in 22307a1

class ConvertAtenSliceScatterOp

Also, do you need to update XFAIL set for MHLO?

[tanyokwok]

You can delete this code as well:

The linalg_on_tensors backend will core dump during the run of the executable of related e2e tests if I delete it from torch-mlir/lib/Conversion/TorchToLinalg/DataMovement.cpp.

Also, do you need to update XFAIL set for MHLO?

No XFAIL will be updated because some extra torch ops still can't be lowered correctly.

[ramiro050]

The linalg_on_tensors backend will core dump during the run of the executable of related e2e tests if I delete it from torch-mlir/lib/Conversion/TorchToLinalg/DataMovement.cpp.

That's very strange. Can you open an issue with the backtrace generated? I can take a look

[tanyokwok]

Can you open an issue with the backtrace generated

@ramiro050 I can't get a concrete stack, but you can recover the exception with branch tanyo/slice_scatter_stage.

The crash stack:

Running SliceScatterModule_basic...

Thread 1 "python" hit Catchpoint 1 (signal SIGABRT), __pthread_kill_implementation (no_tid=0, signo=6, threadid=139922885132736) at ./nptl/pthread_kill.c:44
44      ./nptl/pthread_kill.c: No such file or directory.
(gdb) bt
#0  __pthread_kill_implementation (no_tid=0, signo=6, threadid=139922885132736) at ./nptl/pthread_kill.c:44
#1  __pthread_kill_internal (signo=6, threadid=139922885132736) at ./nptl/pthread_kill.c:78
#2  __GI___pthread_kill (threadid=139922885132736, signo=signo@entry=6) at ./nptl/pthread_kill.c:89
#3  0x00007f4255e17476 in __GI_raise (sig=sig@entry=6) at ../sysdeps/posix/raise.c:26
#4  0x00007f4255dfd7f3 in __GI_abort () at ./stdlib/abort.c:79
#5  0x00007f42558a642a in forward ()
#6  0x00007f425589fe2e in ?? () from /lib/x86_64-linux-gnu/libffi.so.8
#7  0x00007f425589c493 in ?? () from /lib/x86_64-linux-gnu/libffi.so.8
#8  0x00007f4255afb451 in ?? () from /usr/lib/python3.10/lib-dynload/_ctypes.cpython-310-x86_64-linux-gnu.so
#9  0x00007f4255b04ce2 in ?? () from /usr/lib/python3.10/lib-dynload/_ctypes.cpython-310-x86_64-linux-gnu.so
#10 0x0000555efe90730b in _PyObject_MakeTpCall ()
#11 0x0000555efe8ff610 in _PyEval_EvalFrameDefault ()
#12 0x0000555efe91e80e in ?? ()
#13 0x0000555efe8fb8c4 in _PyEval_EvalFrameDefault ()
#14 0x0000555efe910fbc in _PyFunction_Vectorcall ()
#15 0x0000555efe8fb8c4 in _PyEval_EvalFrameDefault ()
#16 0x0000555efe910fbc in _PyFunction_Vectorcall ()
#17 0x0000555efe8f95c9 in _PyEval_EvalFrameDefault ()
#18 0x0000555efe910fbc in _PyFunction_Vectorcall ()
#19 0x0000555efe8f9483 in _PyEval_EvalFrameDefault ()
#20 0x0000555efe910fbc in _PyFunction_Vectorcall ()
#21 0x0000555efe8f9483 in _PyEval_EvalFrameDefault ()
#22 0x0000555efe910fbc in _PyFunction_Vectorcall ()
#23 0x0000555efe8f9483 in _PyEval_EvalFrameDefault ()
#24 0x0000555efe910fbc in _PyFunction_Vectorcall ()
#25 0x0000555efe8f9483 in _PyEval_EvalFrameDefault ()
#26 0x0000555efe8f5cc6 in ?? ()
#27 0x0000555efe9eaeb6 in PyEval_EvalCode ()
#28 0x0000555efe9f04bd in ?? ()
#29 0x0000555efe911219 in ?? ()
#30 0x0000555efe8f9483 in _PyEval_EvalFrameDefault ()
#31 0x0000555efe910fbc in _PyFunction_Vectorcall ()
#32 0x0000555efe8f9483 in _PyEval_EvalFrameDefault ()
#33 0x0000555efe910fbc in _PyFunction_Vectorcall ()
#34 0x0000555efea08f59 in ?? ()
#35 0x0000555efea079d8 in Py_RunMain ()
#36 0x0000555efe9dde6d in Py_BytesMain ()
#37 0x00007f4255dfed90 in __libc_start_call_main (main=main@entry=0x555efe9dde30, argc=argc@entry=8, argv=argv@entry=0x7fff9c321f58) at ../sysdeps/nptl/libc_start_call_main.h:58
#38 0x00007f4255dfee40 in __libc_start_main_impl (main=0x555efe9dde30, argc=8, argv=0x7fff9c321f58, init=<optimized out>, fini=<optimized out>, rtld_fini=<optimized out>, stack_end=0x7fff9c321f48)
    at ../csu/libc-start.c:392
#39 0x0000555efe9ddd65 in _start ()

[ramiro050]

No XFAIL will be updated because some extra torch ops still can't be lowered correctly.

@tanyokwok, there is currently no e2e test that checks that this decomposition is correct. Every time support is added for an op (especially as a decomposition, since it affects every backend), the PR should contain passing e2e tests. Can you revert this commit and add it back once it is passing on at least one of the three backends?

[tanyokwok]

@ramiro050 The decomposition had passed all the e2e tests of SliceScatter in eager_mode.

I think it's sufficient to say the decomposition is correct in those cases. The EagerModeTestConfig will map the input torch.Tensor into TorchMLIRTensor, which is defined in:
https://github.com/llvm/torch-mlir/blob/main/python/torch_mlir/eager_mode/torch_mlir_tensor.py#L51.

The TorchMLIRTensor use __torch_dispatch__ to run the TorchMLIR compiled module, which will run by the torch_mlir_e2e_test.eager_backends.refbackend(where the decompositions happened).

[ramiro050]

@ramiro050 The decomposition had passed all the e2e tests of SliceScatter in eager_mode.

I think it's sufficient to say the decomposition is correct in those cases. The EagerModeTestConfig will map the input torch.Tensor into TorchMLIRTensor, which is defined in: https://github.com/llvm/torch-mlir/blob/main/python/torch_mlir/eager_mode/torch_mlir_tensor.py#L51.

The TorchMLIRTensor use __torch_dispatch__ to run the TorchMLIR compiled module, which will run by the torch_mlir_e2e_test.eager_backends.refbackend(where the decompositions happened).

The eager mode tests have two limitations that make it difficult to rely on them for correctness. The first is that it only uses static shapes in the IR, so dynamic support in implementations is not tested. The second limitation is that if the torch-mlir compilation fails, eager mode will fallback on executing things on conventional PyTorch, so your decomposition could be failing and the tests will still pass. You can try this out by having your decomposition return failure() at the start and running the tests.

Proper testing of a decomposition should involve having passing tests with dynamic shapes on at least one of the three backends: linalg, tosa, mhlo.

[tanyokwok]

@ramiro050 Thanks for reminding me. The revert was created #1659