llvm / llvm-project

The LLVM Project is a collection of modular and reusable compiler and toolchain technologies.
http://llvm.org
Other
28.69k stars 11.87k forks source link

Remove RUN line duplication in mlir/test/Integration/Dialect/SparseTensor/CPU/ #60628

Open banach-space opened 1 year ago

banach-space commented 1 year ago

When extending SparseCompiler integrations tests to run via SVE codegen (see https://reviews.llvm.org/D143514 and https://reviews.llvm.org/D121304), we effectively duplicated many of the RUN lines.

More specifically, if the MLIR_RUN_ARM_SVE_TESTS CMake flag is disabled, the following RUN lines are effectively identical (they verify similar code-path):

  1. https://github.com/llvm/llvm-project/blob/6a4e9ccb2c6f3340eb01bd374988a55daed2a68d/mlir/test/Integration/Dialect/SparseTensor/CPU/sparse_unary.mlir#L15-L16
  2. https://github.com/llvm/llvm-project/blob/4c72266830ffa332ebb7cf1d3bbd6c56d001fa0f/mlir/test/Integration/Dialect/SparseTensor/CPU/sparse_unary.mlir#L20-L27

We should avoid this. My suggestion would be to replace both RUN lines with RUN line 2. above. Additionally, we should make sure that there are public buildbots that

This way we will be making sure that all configurations are tested, but there is not code duplication in tests. WDYT?

CC @aartbik

-Andrzej

llvmbot commented 1 year ago

@llvm/issue-subscribers-mlir-sparse

banach-space commented 1 year ago

Apologies, I forgot to update this with the progress so far.

The most natural way to remove duplication would be to replace lli with mlir-cpu-runner and to fold the two RUN lines listed above into 1.

  1. Patch to add -mattr and -march to -mlir-cpu-runner so that there's no need to use lli instead of mlir-cpu-runner: https://reviews.llvm.org/D146917
  2. Patch to "fold" the duplicated RUN lines into one: https://reviews.llvm.org/D148005.

The 2nd patch is still under review.