This patch updates the remaining SparseCompiler integration tests to
target SVE when available.
TODO: Triage sparse_matmul.mlir and sparse_tanh.mlir
Details
Details
Diff Detail
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
| mlir/test/Integration/Dialect/SparseTensor/CPU/concatenate.mlir | ||
|---|---|---|
| 18 | We actually always run this right? If so, we should really make this (here and everywhere) // Do the same run, but now with direct IR generation and, if available, VLA | |
Comment Actions
- Updated comments (as per suggestion from @aartbik)
- Fixed options that are passed to mlir-opt (with these changes most tests pass)
There are currently two tests that are XFAIL'ed - I will triage them shortly.
Tested with:
- cmake -DMLIR_INCLUDE_INTEGRATION_TESTS=On -DMLIR_RUN_ARM_SVE_TESTS=On <other cmake flags>
- cmake -DMLIR_INCLUDE_INTEGRATION_TESTS=On -DMLIR_RUN_ARM_SVE_TESTS=On -DARM_EMULATOR_EXECUTABLE="qemu-aarch64" -DARM_EMULATOR_LLI_EXECUTABLE=<build-dir>/bin/lli <other cmake flags>
Comment Actions
Split "sparse_reduce_custom.mlir" into two tests
I've split sparse_reduce_custom.mlir into:
- "sparse_reduce_custom.mlir", and
- "sparse_reduce_custom_prod.mlir".
The latter contains product reducitons that are currently supported on AArch64. We did the same for
"sparse_reductions.mlir" in https://reviews.llvm.org/D121304.
Comment Actions
Note that in the long run it would perhaps be desirable to really skip the run if there is no VLA available, since we now do another similar run in most cases, which wastes testing resources.
But it is okay to proceed for now. Perhaps add one TODO in the lit setup for this, so we remember.
Comment Actions
Thanks for reviewing!
I have also thought about it and created https://github.com/llvm/llvm-project/issues/60628 to track this.
Btw, I created https://github.com/llvm/llvm-project/issues/60626 for the outstanding issues with the generated output.
| mlir/test/Integration/Dialect/SparseTensor/CPU/sparse_out_mult_elt.mlir | ||
|---|---|---|
| 20 | The enable-runtime-library=false option causes the values to be slightly different in our tests. | |
| mlir/test/Integration/Dialect/SparseTensor/CPU/sparse_out_reduction.mlir | ||
|---|---|---|
| 20 | We run into a similar issue here. Here, it is ( 7, 69, -1414812757, -1414812757 ) when ( 7, 69, 0, 0 ) is expected | |
| mlir/test/Integration/Dialect/SparseTensor/CPU/sparse_out_mult_elt.mlir | ||
|---|---|---|
| 20 | Probably because we allocated more than needed (because it is impossible to know the nnz beforehand), and values after "20" are uninitialized random value, this should not cause problem as we would access it. You can try out enable-buffer-initialization=true to force sparse compiler to zero out newly allocated memory and see. | |
| 20 |
Sorry, *would not | |
Comment Actions
@frgossen , I'm sorry that you are hitting these issues :(
The enable-runtime-library=false option causes the values to be slightly different in our tests. Instead of (14, 20, 0, 0) the values are only close to 0 (14, 20, -1.21979e-12, -1.21979e-12). Any idea why?
No, sorry. But perhaps we should be using fpcmp for verifying FP output 🤔 .
clang-aarch64-sve-vla (which runs these tests) is green, so ideally I would avoid reverting this change. Perhaps you could blacklist the failing tests and create a GitHub ticket like https://github.com/llvm/llvm-project/issues/60626? We may need to approach this on a case-by-case basis.
Comment Actions
Fix in https://reviews.llvm.org/D143674
(our new push_back uses size/capacity a bit different, so a vector_transfer+print may see unitialized memory, which is why the enable-buffer-initialization was introduced to make sure memory is consistent)
if there are issues beyond this problem, please let me know!