CMake: Refactor DDR support

Introduced a number of changes

- Headers can now be directly associated with a ddr set
- A DDR set no longer needs to be specified when enabling DDR for a target.
- Added a new command `ddr_set_add_targets` which adds a target to a DDR
  set. Combined with the above, this allows for a delayed definition of a
  DDR set. In addition targets can be in multiple DDR sets.
- DDR sets can now be composed via `ddr_set_add_targets`. This means we can
  get rid of the awkward `OMR_DDR_SET` mechanics. Instead we just define
  our own DDR set, and consumers simply add our DDR set to theirs
- omr_ prefix removed from ddr related target names.
- Added GLOB_HEADERS and GLOB_HEADERS_RECURSIVE options to
  target_enable_ddr, which glob the target source directory for headers

Signed-off-by: Devin Nakamura <[email protected]>

Author: Devin Nakamura

CMakeLists.txt

@@ -114,11 +114,7 @@ configure_file(./omrversionstrings.CMakeTemplate.h omrversionstrings.h)
 ###
 include(OmrDDRSupport)
 
-if(NOT DEFINED OMR_DDR_SET)
-	set(OMR_DDR_SET "omr")
-	make_ddr_set(${OMR_DDR_SET})
-endif()
-
+make_ddr_set(omrddr)
 
 ###
 ### Native Tooling

cmake/modules/OmrDDRSupport.cmake

@@ -29,16 +29,17 @@ include(OmrUtility)
 include(ExternalProject)
 
 set(OMR_MODULES_DIR ${CMAKE_CURRENT_LIST_DIR})
+set(DDR_INFO_DIR "${CMAKE_BINARY_DIR}/ddr_info")
+
 
 function(make_ddr_set set_name)
 	# if DDR is not enabled, just skip
 	# Also skip if we are on windows since it is unsupported at the moment
 	if((OMR_HOST_OS STREQUAL "win") OR (NOT OMR_DDR))
 		return()
 	endif()
-	set(DDR_TARGET_NAME "${set_name}_ddr")
+	set(DDR_TARGET_NAME "${set_name}")
 	set(DDR_BIN_DIR "${CMAKE_CURRENT_BINARY_DIR}/${DDR_TARGET_NAME}")
-	set(DDR_TARGETS_LIST "${DDR_BIN_DIR}/targets.list")
 	set(DDR_MACRO_INPUTS_FILE "${DDR_BIN_DIR}/macros.list")
 	set(DDR_TOOLS_EXPORT "${omr_BINARY_DIR}/ddr/tools/DDRTools.cmake")
 	set(DDR_CONFIG_STAMP "${DDR_BIN_DIR}/config.stamp")
@@ -54,35 +55,67 @@ function(make_ddr_set set_name)
 		COMMAND ${CMAKE_COMMAND} --build "${DDR_BIN_DIR}"
 	)
 	set_property(TARGET "${DDR_TARGET_NAME}" PROPERTY DDR_BIN_DIR "${DDR_BIN_DIR}")
+	set_property(TARGET "${DDR_TARGET_NAME}" PROPERTY DDR_SET TRUE)
 
 	file(READ ${OMR_MODULES_DIR}/ddr/DDRSetStub.cmake.in cmakelist_template)
 	string(CONFIGURE "${cmakelist_template}" cmakelist_template @ONLY)
 	file(GENERATE OUTPUT ${DDR_BIN_DIR}/CMakeLists.txt CONTENT "${cmakelist_template}")
+
+	# Note: DDR sets have themselves as targets to process
+	# This is so that you can process misc headers which don't logically belong to any other target
+	target_enable_ddr(${DDR_TARGET_NAME})
+	set_property(TARGET "${DDR_TARGET_NAME}" APPEND PROPERTY DDR_TARGETS "${DDR_TARGET_NAME}")
 endfunction(make_ddr_set)
 
+function(ddr_set_add_targets ddr_set)
+	if((OMR_HOST_OS STREQUAL "win") OR (NOT OMR_DDR))
+		return()
+	endif()
 
-function(target_enable_ddr tgt ddr_set)
+	omr_assert(FATAL_ERROR TEST TARGET "${ddr_set}" MESSAGE "ddrset_add_targets called on non-existent ddr-set ${ddr_set}")
+	get_target_property(is_ddrset "${ddr_set}" DDR_SET)
+	omr_assert(FATAL_ERROR TEST is_ddrset MESSAGE "ddrset_add_targets called on ddr-set ${ddr_set}, which is not a ddr set")
+	foreach(tgt IN LISTS ARGN)
+		# Check if the target we are adding is itself a DDR set
+		get_target_property(tgt_is_ddr_set ${tgt} DDR_SET)
+		if(tgt_is_ddr_set)
+			set_property(TARGET ${ddr_set} APPEND PROPERTY DDR_SUBSETS ${tgt})
+		else()
+			set_property(TARGET ${ddr_set} APPEND PROPERTY DDR_TARGETS ${tgt})
+		endif()
+		add_dependencies(${ddr_set} ${tgt})
+	endforeach()
+endfunction(ddr_set_add_targets)
+
+function(target_enable_ddr tgt)
 	if((OMR_HOST_OS STREQUAL "win") OR (NOT OMR_DDR))
 		return()
 	endif()
 
-	set(opt_EARLY_SOURCE_EVAL )
-	set(opt_UNPARSED_ARGUMENTS )
-	cmake_parse_arguments(opt "EARLY_SOURCE_EVAL" "" "" ${ARGN})
-	omr_assert(FATAL_ERROR TEST NOT opt_UNPARSED_ARGUMENTS MESSAGE "target_enable_ddr: unrecognized options ${opt_UNPARSED_ARGUMENTS}")
+	omr_assert(FATAL_ERROR TEST TARGET ${tgt} MESSAGE "target_enable_ddr called on non-existent target ${tgt}")
+
+	set(options
+		EARLY_SOURCE_EVAL
+		GLOB_HEADERS
+		GLOB_HEADERS_RECURSIVE
+	)
+	set(oneValueArgs "")
+	set(multiValueArgs "")
 
+	# Clear variables
+	foreach(opt_name IN LISTS options oneValueArgs multiValueArgs)
+		set(opt_${opt_name} )
+	endforeach()
 
-	set(DDR_SET_TARGET "${ddr_set}_ddr")
-	omr_assert(FATAL_ERROR TEST TARGET ${tgt} MESSAGE "target_enable_ddr called on non-existant target ${tgt}")
-	omr_assert(FATAL_ERROR TEST TARGET "${DDR_SET_TARGET}" MESSAGE "target_enable_ddr called on non-existant ddr_set ${ddr_set}")
+
+	cmake_parse_arguments(opt "${options}" "${oneValueArgs}" "${multiValueArgs}"  ${ARGN})
+	omr_assert(FATAL_ERROR TEST NOT opt_UNPARSED_ARGUMENTS MESSAGE "target_enable_ddr: unrecognized options ${opt_UNPARSED_ARGUMENTS}")
 
 	get_target_property(target_type "${tgt}" TYPE)
 	if(target_type MATCHES "INTERFACE_LIBRARY")
 		message(FATAL_ERROR "Cannot call enable_ddr on interface libraries")
 	endif()
 
-	get_property(DDR_BIN_DIR TARGET "${DDR_SET_TARGET}" PROPERTY DDR_BIN_DIR)
-
 	if(opt_EARLY_SOURCE_EVAL)
 		set(source_property "DDR_EVAL_SOURCE")
 		get_target_property(sources "${tgt}" "SOURCES")
@@ -109,6 +142,28 @@ function(target_enable_ddr tgt ddr_set)
 		set(MAGIC_TEMPLATE "OUTPUT_FILE\n$<TARGET_FILE:${tgt}>\n${MAGIC_TEMPLATE}")
 	endif()
 
-	file(GENERATE OUTPUT "${DDR_BIN_DIR}/${tgt}.txt" CONTENT "${MAGIC_TEMPLATE}\n")
-	set_property(TARGET ${DDR_SET_TARGET} APPEND PROPERTY INPUT_TARGETS ${tgt})
+	file(GENERATE OUTPUT "${DDR_INFO_DIR}/targets/${tgt}.txt" CONTENT "${MAGIC_TEMPLATE}\n")
+
+	if(opt_GLOB_HEADERS OR opt_GLOB_HEADERS_RECURSIVE)
+		get_target_property(source_dir ${tgt} SOURCE_DIR)
+		if(opt_GLOB_HEADERS_RECURSIVE)
+			set(glob  GLOB_RECURSE)
+		else()
+			set(glob GLOB)
+		endif()
+		file(${glob} c_headers "${source_dir}/*.h")
+		file(${glob} cpp_headers "${source_dir}/*.hpp")
+		ddr_add_headers(${tgt}
+			${c_headers}
+			${cpp_headers}
+		)
+
+	endif()
 endfunction(target_enable_ddr)
+
+function(ddr_add_headers tgt)
+	if(NOT OMR_DDR)
+		return()
+	endif()
+	set_property(TARGET ${tgt} APPEND PROPERTY DDR_HEADERS ${ARGN})
+endfunction(ddr_add_headers)

cmake/modules/ddr/DDRSetStub.cmake.in

@@ -21,29 +21,42 @@
 
 cmake_minimum_required(VERSION 3.3 FATAL_ERROR)
 
-project(dummy LANGUAGES NONE)
-
-set(OMR_MODULES_DIR @OMR_MODULES_DIR@)
+set(OMR_MODULES_DIR "@OMR_MODULES_DIR@")
 set(DDR_SUPPORT_DIR "@OMR_MODULES_DIR@/ddr")
-set(DDR_INPUTS
-	$<JOIN:$<TARGET_PROPERTY:@DDR_TARGET_NAME@,INPUT_TARGETS>,
-	>
-)
+set(DDR_INFO_DIR "@DDR_INFO_DIR@")
 set(DDR_BLACKLIST "$<TARGET_PROPERTY:@DDR_TARGET_NAME@,BLACKLIST>")
 set(DDR_OVERRIDES_FILE "$<TARGET_PROPERTY:@DDR_TARGET_NAME@,OVERRIDES_FILE>")
 set(DDR_BLOB "$<TARGET_PROPERTY:@DDR_TARGET_NAME@,BLOB>")
 set(DDR_SUPERSET "$<TARGET_PROPERTY:@DDR_TARGET_NAME@,SUPERSET>")
+set(DDR_MACRO_LIST "${DDR_INFO_DIR}/sets/@[email protected]")
+set(CMAKE_MODULE_PATH ${CMAKE_MODULE_PATH} ${OMR_MODULES_DIR})
+
+set(DDR_TARGETS
+	$<JOIN:$<TARGET_PROPERTY:@DDR_TARGET_NAME@,DDR_TARGETS>,
+	>
+)
+set(DDR_SUBSETS
+	$<JOIN:$<TARGET_PROPERTY:@DDR_TARGET_NAME@,DDR_SUBSETS>,
+	>
+)
+
+project(@DDR_TARGET_NAME@ LANGUAGES NONE)
 
 include("@DDR_TOOLS_EXPORT@")
+include(OmrUtility)
 
-# If not provided, set default values for superset and blob outputs
-if(NOT DDR_BLOB)
-	set(DDR_BLOB "${CMAKE_CURRENT_BINARY_DIR}/blob.dat")
-endif()
+# given a var, make the path specified absolute,
+# assuming paths are relative to the ddr set source directory
+macro(make_absolute var_name)
+	if(${var_name})
+		get_filename_component(${var_name} "${${var_name}}" ABSOLUTE  BASE_DIR "$<TARGET_PROPERTY:@DDR_TARGET_NAME@,SOURCE_DIR>")
+	endif()
+endmacro()
 
-if(NOT DDR_SUPERSET)
-	set(DDR_SUPERSET "${CMAKE_CURRENT_BINARY_DIR}/superset.out")
-endif()
+make_absolute(DDR_BLACKLIST)
+make_absolute(DDR_OVERRIDES_FILE)
+make_absolute(DDR_BLOB)
+make_absolute(DDR_SUPERSET)
 
 function(get_relative_path output filename base)
 	get_filename_component(temp "${filename}" ABSOLUTE BASE_DIR "${base}")
@@ -127,7 +140,6 @@ function(process_source_files src_files)
 	endif()
 
 	if(OPT_OUTPUT_ANNOTATED)
-		message(STATUS "setting annotated file for ${target} ${annotated_files}")
 		set("${OPT_OUTPUT_ANNOTATED}" "${annotated_files}" PARENT_SCOPE)
 	endif()
 endfunction(process_source_files)
@@ -137,15 +149,17 @@ set(target_files )
 
 function(process_target target)
 	file(MAKE_DIRECTORY "${CMAKE_CURRENT_BINARY_DIR}/${target}")
-	file(STRINGS "${CMAKE_SOURCE_DIR}/${target}.txt" target_config)
+	set(target_info_file "${DDR_INFO_DIR}/targets/${target}.txt")
+	# Make cmake configuration depend on input file
+	set_property(DIRECTORY "${CMAKE_CURRENT_SOURCE_DIR}" APPEND PROPERTY CMAKE_CONFIGURE_DEPENDS "${target_info_file}")
+	file(STRINGS "${target_info_file}" target_config)
 
 	cmake_parse_arguments("OPT" "" "SOURCE_DIR;OUTPUT_FILE" "INCLUDE_PATH;DEFINES;SOURCES;HEADERS;PREINCLUDES" ${target_config})
 
 	if(OPT_OUTPUT_FILE)
 		set(target_files ${target_files} "${OPT_OUTPUT_FILE}" PARENT_SCOPE)
 	endif()
 
-
 	process_source_files(
 		${OPT_SOURCES}
 		TARGET "${target}"
@@ -175,38 +189,66 @@ function(process_target target)
 
 endfunction(process_target)
 
-foreach(target IN LISTS DDR_INPUTS)
+foreach(target IN LISTS DDR_TARGETS)
 	process_target("${target}")
 endforeach()
 
+set(subset_macro_lists "")
+foreach(subset IN LISTS DDR_SUBSETS)
+	list(APPEND subset_macro_lists "${DDR_INFO_DIR}/sets/${subset}.macros")
+endforeach()
 add_custom_command(
-	OUTPUT macroList
-	DEPENDS ${annotated_files}
-	COMMAND cat ${annotated_files} > macroList
+	OUTPUT ${DDR_MACRO_LIST}
+	DEPENDS ${annotated_files} ${subset_macro_lists}
+	COMMAND cat ${annotated_files} ${subset_macro_lists} > ${DDR_MACRO_LIST}
 )
 
 set(EXTRA_DDRGEN_OPTIONS "")
 if(DDR_BLACKLIST)
 	list(APPEND EXTRA_DDRGEN_OPTIONS "--blacklist" "${DDR_BLACKLIST}")
 endif()
+
+if(DDR_BLOB)
+	list(APPEND EXTRA_DDRGEN_OPTIONS "--blob" "${DDR_BLOB}")
+endif()
+
+if(DDR_SUPERSET)
+	list(APPEND EXTRA_DDRGEN_OPTIONS "--superset" "${DDR_SUPERSET}")
+endif()
+
 if(DDR_OVERRIDES_FILE)
 	# Because ddr overrides files specify relative paths, we need to run ddrgen
 	# in the same directory as the overrides files.
-	get_filename_component(override_dir "${DDR_OVERRIDES_FILE}" DIRECTORY)
+	get_filename_component(override_file_dir "${DDR_OVERRIDES_FILE}" DIRECTORY)
 
-	list(APPEND EXTRA_DDRGEN_OPTIONS "--overrides" "${DDR_OVERRIDES_FILE}" WORKING_DIRECTORY "${override_dir}")
+	list(APPEND EXTRA_DDRGEN_OPTIONS "--overrides" "${DDR_OVERRIDES_FILE}" WORKING_DIRECTORY "${override_file_dir}")
 endif()
 
-add_custom_command(
-	OUTPUT ${DDR_BLOB} ${DDR_SUPERSET}
-	DEPENDS macroList
-	COMMAND omr_ddrgen
-		${target_files}
-		-e
-		--macrolist macroList
-		--blob ${DDR_BLOB}
-		--superset ${DDR_SUPERSET}
-		${EXTRA_DDRGEN_OPTIONS}
-)
+set(subset_binaries)
+foreach(subset IN LISTS DDR_SUBSETS)
+	# Name of file which subset generates, listing all of the input binaries
+	set(binaries_list_file "${DDR_INFO_DIR}/sets/${subset}.binaries")
+	set_property(DIRECTORY "${CMAKE_CURRENT_SOURCE_DIR}" APPEND PROPERTY CMAKE_CONFIGURE_DEPENDS "${binaries_list_file}")
+	file(STRINGS "${binaries_list_file}" binaries)
+	list(APPEND subset_binaries ${binaries})
+endforeach()
 
-add_custom_target(ddrgen ALL DEPENDS ${DDR_BLOB})
+# Now we generate our own list of binaries, so that they can be parsed if we are a subset
+omr_join(binaries_list "\n"
+	${target_files}
+	${subset_binaries}
+)
+file(WRITE "${DDR_INFO_DIR}/sets/@[email protected]" "${binaries_list}")
+
+if(DDR_BLOB OR DDR_SUPERSET)
+	add_custom_command(
+		OUTPUT ${DDR_BLOB} ${DDR_SUPERSET}
+		DEPENDS ${DDR_MACRO_LIST} ${target_files} ${subset_binaries}
+		COMMAND omr_ddrgen
+			${target_files} ${subset_binaries}
+			--show-empty
+			--macrolist ${DDR_MACRO_LIST}
+			${EXTRA_DDRGEN_OPTIONS}
+	)
+endif()
+add_custom_target(@DDR_TARGET_NAME@ ALL DEPENDS ${DDR_BLOB} ${DDR_MACRO_LIST})

cmake/modules/ddr/cmake_ddr.awk

@@ -130,7 +130,7 @@ END {
 }
 
 NR == 1 {
-	print "DDRFILE_BEGIN " FILENAME
+	begin_file(FILENAME);
 }
 
 /@ddr_options: *valuesonly/ { add_values = 1; add_flags = 0}

ddr/test/CMakeLists.txt

@@ -30,8 +30,6 @@ add_executable(ddrgentest
     test.cpp
 )
 
-make_ddr_set(testset)
-target_enable_ddr(ddrgentest testset)
-
-file(GLOB_RECURSE headerfiles "${CMAKE_CURRENT_SOURCE_DIR}/*.h(pp)?")
-set_property(TARGET ddrgentest PROPERTY DDR_HEADERS ${headerfiles})
+make_ddr_set(ddr_testset)
+target_enable_ddr(ddrgentest GLOB_HEADERS_RECURSIVE)
+ddr_set_add_targets(ddr_testset ddrgentest)

gc/CMakeLists.txt

@@ -372,7 +372,8 @@ target_link_libraries(omrgc
 		${OMR_PORT_LIB}
 		${OMR_HOOK_LIB}
 )
-target_enable_ddr(omrgc "${OMR_DDR_SET}" EARLY_SOURCE_EVAL)
+target_enable_ddr(omrgc EARLY_SOURCE_EVAL)
+ddr_set_add_targets(omrddr omrgc)
 
 set_target_properties(omrgc omrgc_hookgen omrgc_tracegen PROPERTIES FOLDER gc)
 

omr/CMakeLists.txt

@@ -84,4 +84,5 @@ if(OMR_GC)
 	)
 endif(OMR_GC)
 
-target_enable_ddr(omrcore "${OMR_DDR_SET}")
+target_enable_ddr(omrcore)
+ddr_set_add_targets(omrddr omrcore)

omr/startup/CMakeLists.txt

@@ -51,4 +51,5 @@ if(OMR_GC)
 	)
 endif()
 
-target_enable_ddr(omrvmstartup "${OMR_DDR_SET}")
+target_enable_ddr(omrvmstartup)
+ddr_set_add_targets(omrddr omrvmstartup)

omrsigcompat/CMakeLists.txt

@@ -104,4 +104,5 @@ install(
 	LIBRARY DESTINATION ${OMR_INSTALL_LIB_DIR}/
 )
 
-target_enable_ddr(omrsig "${OMR_DDR_SET}")
+target_enable_ddr(omrsig)
+ddr_set_add_targets(omrddr omrsig)

omrtrace/CMakeLists.txt

@@ -47,7 +47,8 @@ target_link_libraries(omrtrace
 		omrutil
 )
 
-target_enable_ddr(omrtrace "${OMR_DDR_SET}")
+target_enable_ddr(omrtrace)
+ddr_set_add_targets(omrddr omrtrace)
 
 #TODO: check if following makefile fragment still required:
 #ifeq (gcc,$(OMR_TOOLCHAIN))