MLIR tests failing under Release

[rofirrim]

Hi all,

when building LLVM in release mode, enabling mlir, like so

cmake -G Ninja <llvm-source-path> -DCMAKE_BUILD_TYPE=Release \
    '-DLLVM_ENABLE_PROJECTS=clang;mlir' \
   -DLLVM_USE_LINKER=lld \
   -DLLVM_ENABLE_LIBCXX=ON

Then I see the following failures on check-mlir.

Failed Tests (3):
  MLIR :: Dialect/LLVMIR/sroa-statistics.mlir
  MLIR :: Dialect/MemRef/mem2reg-statistics.mlir
  MLIR :: Dialect/Transform/test-repro-dump.mlir

(libc++ and LLD may not be necessary when building but I'm consistently able to trigger the errors under that build configuration)

System is an x86-64 Ubuntu 22.04 LTS building with Clang 15.0.6 from llvm.org

I've investigated a bit, the tests does not crash but does not print what it is expected from it.

From what I saw in the debugger, the switch in https://github.com/llvm/llvm-project/blob/main/mlir/lib/Pass/PassStatistics.cpp#L152 even if it covers all the cases of the enum class PassDisplayMode (https://github.com/llvm/llvm-project/blob/main/mlir/include/mlir/Pass/PassManager.h#L195) is never executed.

At firstg it seems a miscompilation triggered by building in Release mode (a lot of things get inlined) but I have not found anything obvious.

[llvmbot]

@llvm/issue-subscribers-mlir

[EugeneZelenko]

Could you please try 16 or main branch?

[rofirrim]

Could you please try 16 or main branch?

Sure.

I used main's 7ffeb8e

I first built stage 1 for it

cmake -G Ninja <path-to-llvm>/llvm \
  -DCMAKE_BUILD_TYPE=Release \
  '-DLLVM_ENABLE_PROJECTS=clang;lld' \
  '-DLLVM_ENABLE_RUNTIMES=libcxx;libcxxabi' \
  -DCMAKE_INSTALL_PREFIX=<stage1-install>
ninja
ninja install

and then in another directory I built

# libcxx at runtime
export LD_LIBRARY_PATH=<stage1-install>/lib/x86_64-unknown-linux-gnu

CC=<stage1-install>/bin/clang \
CXX=<stage1-install>/bin/clang++ \
   cmake -G Ninja <path-to-llvm>/llvm \
   -DCMAKE_BUILD_TYPE=Release \
   '-DLLVM_ENABLE_PROJECTS=clang;mlir' \
   -DLLVM_USE_LINKER=lld \
   -DLLVM_ENABLE_LIBCXX=ON
ninja
ninja check-mlir

Now only two of the three tests above fail:

********************
********************
Failed Tests (2):
  MLIR :: Dialect/LLVMIR/sroa-statistics.mlir
  MLIR :: Dialect/MemRef/mem2reg-statistics.mlir

[dtzSiFive]

The statistics tests need to be gated on whether statistics are enabled in the build (LLVM_ENABLE_STATS), see pipeline-stats.mlir and pipeline-stats-nested.mlir tests. This is needed since 18839be (and we can probably drop the regex matching to handle stats disabled scenario that previously printed zeros but not does nothing other than print the header).

Since there's no lit feature for whether statistics are enabled, the solution is to make these tests depend on ASSERTS which implies statistics are enabled (but missed when opted into w/o asserts), via the REQUIRES: asserts lines in those tests.

Probably the repro dump test also needs to be gated on "asserts" configuration (for the -debug-only functionality).

I can't submit the patch for this today, but should be easy for someone to add the missing REQUIRES lines to these tests.

[dtzSiFive]

(Looks like this was handled on the test-repro-dump.mlir test here: ccb9115 which is why it no longer fails )

[rofirrim]

The statistics tests need to be gated on whether statistics are enabled in the build (LLVM_ENABLE_STATS)

Thanks for the explanation @dtzSiFive.

Since there's no lit feature for whether statistics are enabled, the solution is to make these tests depend on ASSERTS which implies statistics are enabled (but missed when opted into w/o asserts), via the REQUIRES: asserts lines in those tests.

I can try to add the REQUIRES lines.

Would it make sense that we don't even attempt to execute the switch by protecting it with LLVM_ENABLE_STATS as well? From your explanation I understand that the corresponding variables are not even initialised? This could be another patch, of course.

[joker-eph]

From what I saw in the debugger, the switch in https://github.com/llvm/llvm-project/blob/main/mlir/lib/Pass/PassStatistics.cpp#L152 even if it covers all the cases of the enum class PassDisplayMode (https://github.com/llvm/llvm-project/blob/main/mlir/include/mlir/Pass/PassManager.h#L195) is never executed.

I'm a bit confused about the story for the switch: I didn't quite get where is the problem with it right now?
Note that printResultsAsPipeline for example is already guarded by LLVM_ENABLE_STATS, it is possible that the optimizer eliminates the switch entirely already in this case.

[rofirrim]

From what I saw in the debugger, the switch in https://github.com/llvm/llvm-project/blob/main/mlir/lib/Pass/PassStatistics.cpp#L152 even if it covers all the cases of the enum class PassDisplayMode (https://github.com/llvm/llvm-project/blob/main/mlir/include/mlir/Pass/PassManager.h#L195) is never executed.

I'm a bit confused about the story for the switch: I didn't quite get where is the problem with it right now? Note that printResultsAsPipeline for example is already guarded by LLVM_ENABLE_STATS, it is possible that the optimizer eliminates the switch entirely already in this case.

Hi @joker-eph, yep you're right I double checked and I failed to realise that the body of the function is compiled out. Sorry for the fuss.

[rofirrim]

Fixed in https://reviews.llvm.org/rG666a56f1f1683535967d1a3b7587bf85044ea005