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

c-rhodes · 2024-01-17T13:50:34Z

These targets conveniently group ArmSME related tests and can be used as follows:

ninja check-mlir-unit-armsme # unit only
ninja check-mlir-integration-armsme # integration only
ninja check-mlir-armsme # all

These targets conveniently group ArmSME related tests and can be used as follows: ninja check-mlir-unit-armsme # unit only ninja check-mlir-integration-armsme # integration only ninja check-mlir-armsme # all

llvmbot · 2024-01-17T13:51:03Z

@llvm/pr-subscribers-mlir

Author: Cullen Rhodes (c-rhodes)

Changes

These targets conveniently group ArmSME related tests and can be used as follows:

ninja check-mlir-unit-armsme # unit only
ninja check-mlir-integration-armsme # integration only
ninja check-mlir-armsme # all

Full diff: https://github.com/llvm/llvm-project/pull/78448.diff

1 Files Affected:

(modified) mlir/test/CMakeLists.txt (+23)

diff --git a/mlir/test/CMakeLists.txt b/mlir/test/CMakeLists.txt
index 8ce030feeded92..58f44039a3cda0 100644
--- a/mlir/test/CMakeLists.txt
+++ b/mlir/test/CMakeLists.txt
@@ -214,3 +214,26 @@ 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.
+
+# 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
+)
+
+# 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
+)

banach-space · 2024-01-18T17:14:56Z

mlir/test/CMakeLists.txt

+  check-mlir-dialect-armsme
+  check-mlir-dialect-llvmir
+  check-mlir-conversion-armsmetollvm
+  check-mlir-conversion-armsmetoscf
+  check-mlir-conversion-vectortoarmsme


Where are these targets defined?

Where are these targets defined?

they're auto-generated

(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())

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.

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 ...

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.

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}

)

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?

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.

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/:.*//')

banach-space

Thanks, this will really improve our workflow, LGTM!

I might be a bit biased so please wait for "+1" from somebody not working on SME before landing :)

jpienaar

LGTM - if it ends up that we have many different such classes we could see about some additional structure. But not too likely.

joker-eph

(thread to resolve on maintainability and scalability)

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

b9aa7b0

These targets conveniently group ArmSME related tests and can be used as follows: ninja check-mlir-unit-armsme # unit only ninja check-mlir-integration-armsme # integration only ninja check-mlir-armsme # all

c-rhodes requested review from banach-space and MacDue January 17, 2024 13:50

llvmbot added the mlir label Jan 17, 2024

banach-space reviewed Jan 18, 2024

View reviewed changes

banach-space requested review from joker-eph, ftynse and jpienaar January 18, 2024 19:27

banach-space approved these changes Jan 18, 2024

View reviewed changes

jpienaar approved these changes Jan 22, 2024

View reviewed changes

document 'ninja -t targets'

3790454

joker-eph requested changes Jan 23, 2024

View reviewed changes

c-rhodes closed this Jan 30, 2024

c-rhodes deleted the mlir-arm-sme-cmake-test-targets branch January 30, 2024 08:44

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

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

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

Uh oh!

c-rhodes commented Jan 17, 2024

Uh oh!

llvmbot commented Jan 17, 2024

Uh oh!

banach-space Jan 18, 2024

Uh oh!

c-rhodes Jan 18, 2024

Uh oh!

MacDue Jan 18, 2024

Uh oh!

banach-space Jan 18, 2024

Uh oh!

c-rhodes Jan 18, 2024

Uh oh!

joker-eph Jan 23, 2024

Uh oh!

c-rhodes Jan 23, 2024

Uh oh!

joker-eph Jan 23, 2024

Uh oh!

c-rhodes Jan 24, 2024

Uh oh!

MacDue Jan 24, 2024

Uh oh!

banach-space left a comment

Uh oh!

jpienaar left a comment

Uh oh!

joker-eph left a comment

Uh oh!

Uh oh!

	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()

	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}
	)

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

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

Uh oh!

Conversation

c-rhodes commented Jan 17, 2024

Uh oh!

llvmbot commented Jan 17, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

banach-space left a comment

Choose a reason for hiding this comment

Uh oh!

jpienaar left a comment

Choose a reason for hiding this comment

Uh oh!

joker-eph left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!