Skip to content

[mlir][ArmSME][nfc] Add custom CMake targets to group tests #78448

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 2 commits into from
Closed

[mlir][ArmSME][nfc] Add custom CMake targets to group tests #78448

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b9aa7b0
[mlir][ArmSME][nfc] Add custom CMake targets to group tests
c-rhodes Jan 17, 2024
3790454
document 'ninja -t targets'
c-rhodes Jan 23, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 24 additions & 0 deletions mlir/test/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -214,3 +214,27 @@ set_target_properties(check-mlir PROPERTIES FOLDER "Tests")
add_lit_testsuites(MLIR ${CMAKE_CURRENT_SOURCE_DIR}
DEPENDS ${MLIR_TEST_DEPENDS}
)

# Convenience targets to group related tests. To find targets use
# 'ninja -t targets'.

# Run all ArmSME unit tests.
add_custom_target(check-mlir-unit-armsme DEPENDS
check-mlir-dialect-armsme
check-mlir-dialect-llvmir
check-mlir-conversion-armsmetollvm
check-mlir-conversion-armsmetoscf
check-mlir-conversion-vectortoarmsme
Comment on lines +223 to +227
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where are these targets defined?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where are these targets defined?

they're auto-generated

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(You get them from using add_mlir_conversion_library() etc in CMake, I think the actual check- targets are added deep in the CMake, maybe in llvm_add_library())

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How can I find them? I assume that I can query that with e.g. ninja? It would be good to document that - otherwise this feels like adding some random targets.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How can I find them? I assume that I can query that with e.g. ninja? It would be good to document that - otherwise this feels like adding some random targets.

ninja -t targets

I think they're pretty self-documenting to be honest, they follow the directory structure, i.e.:

  check-mlir-dialect-armsme - mlir/test/Dialect/ArmSME
  check-mlir-dialect-llvmir - mlir/test/Dialect/LLVMIR
  check-mlir-conversion-armsmetollvm - mlir/test/Conversion/ArmSMEToSCF
...

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a link about how are these generated? I don't remember seeing this before and I can't find it in the code right now.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think they're generated by add_lit_testsuites CMake function in LLVM:

llvm-project/llvm/cmake/modules/AddLLVM.cmake

Lines 2036 to 2074 in ccf1e32

function(add_lit_testsuites project directory)
if (NOT LLVM_ENABLE_IDE)
cmake_parse_arguments(ARG "EXCLUDE_FROM_CHECK_ALL" "FOLDER" "PARAMS;DEPENDS;ARGS" ${ARGN})
if (NOT ARG_FOLDER)
set(ARG_FOLDER "Test Subdirectories")
endif()
# Search recursively for test directories by assuming anything not
# in a directory called Inputs contains tests.
file(GLOB_RECURSE to_process LIST_DIRECTORIES true ${directory}/*)
foreach(lit_suite ${to_process})
if(NOT IS_DIRECTORY ${lit_suite})
continue()
endif()
string(FIND ${lit_suite} Inputs is_inputs)
string(FIND ${lit_suite} Output is_output)
if (NOT (is_inputs EQUAL -1 AND is_output EQUAL -1))
continue()
endif()
# Create a check- target for the directory.
string(REPLACE ${directory} "" name_slash ${lit_suite})
if (name_slash)
string(REPLACE "/" "-" name_slash ${name_slash})
string(REPLACE "\\" "-" name_dashes ${name_slash})
string(TOLOWER "${project}${name_dashes}" name_var)
add_lit_target("check-${name_var}" "Running lit suite ${lit_suite}"
${lit_suite}
${EXCLUDE_FROM_CHECK_ALL}
PARAMS ${ARG_PARAMS}
DEPENDS ${ARG_DEPENDS}
ARGS ${ARG_ARGS}
)
set_target_properties(check-${name_var} PROPERTIES FOLDER ${ARG_FOLDER})
endif()
endforeach()
endif()
endfunction()

which is used in MLIR test configuration:

llvm-project/mlir/test/CMakeLists.txt

Lines 212 to 220 in ccf1e32

add_lit_testsuite(check-mlir "Running the MLIR regression tests"
${CMAKE_CURRENT_BINARY_DIR}
DEPENDS ${MLIR_TEST_DEPENDS}
)
set_target_properties(check-mlir PROPERTIES FOLDER "Tests")
add_lit_testsuites(MLIR ${CMAKE_CURRENT_SOURCE_DIR}
DEPENDS ${MLIR_TEST_DEPENDS}
)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

For this patch I'm mostly unsure about the maintainability and scalability of the approach. ARM-SME is just the first one, so how are we gonna do the same thing for all of the other components in MLIR? What would this look like here?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For this patch I'm mostly unsure about the maintainability and scalability of the approach. ARM-SME is just the first one, so how are we gonna do the same thing for all of the other components in MLIR? What would this look like here?

Understand your concern, seeing no other examples of this in the codebase did make me question it. The problem I'm trying to solve is having a fast and convenient way to run all the tests related to the ArmSME dialect, particularly when working on large features (such as #78975) with various dialect / conversion / integration tests.

ninja check-mlir is very slow, takes ~6.5 minutes on my c7g.8xlarge dev machine (Graviton3, Neoverse V1, 32 cores). That's with:

  • MLIR_INCLUDE_INTEGRATION_TESTS
  • MLIR_INCLUDE_TESTS
  • MLIR_RUN_ARM_SVE_TESTS
  • MLIR_RUN_ARM_SME_TESTS

enabled and emulation (ARM_EMULATOR_EXECUTABLE=qemu-aarch64). I've not properly benchmarked this but I suspect a decent chunk of the time is spent running the SparseTensor integration tests (there's many) with SVE enabled under emulation. This particular machine does actually have SVE support so running SVE tests under emulation isn't necessary, but currently this level of granularity can't be expressed in the configuration, if an emulator is specified the SVE and SME tests will use it. We should probably address that, but these custom targets just seemed like a simple fix for what I want, here's some rough timings:

time ninja check-mlir-dialect-armsme  7.67s user 1.82s system 370% cpu 2.562 total
time ninja check-mlir-integration-armsme  255.53s user 7.44s system 419% cpu 1:02.75 total
time ninja check-mlir-armsme  282.31s user 15.02s system 425% cpu 1:09.95 total
time ninja check-mlir  9558.85s user 292.57s system 2508% cpu 6:32.73 total

I'm not sure this is particularly useful for all components, I think this approach is more relevant for target-specific dialects without mature hardware support like ArmSME / AMX / RISCV that require emulation for integration tests.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here's a bash one-liner based on this patch that runs all targets that match the regex ^check-mlir.*armsme:

ninja $(ninja -t targets | grep -e "^check-mlir.*armsme" | sed 's/:.*//')

)

# Run all ArmSME integration tests.
add_custom_target(check-mlir-integration-armsme DEPENDS
check-mlir-integration-dialect-linalg-cpu-armsme
check-mlir-integration-dialect-vector-cpu-armsme
)

# Run all ArmSME tests.
add_custom_target(check-mlir-armsme DEPENDS
check-mlir-unit-armsme
check-mlir-integration-armsme
)