Skip to content
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.

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

embed.cmake: add support for Windows resource file #2330

Merged
merged 20 commits into from
Nov 21, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
216 changes: 117 additions & 99 deletions cmake/Embed.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -21,17 +21,21 @@
# OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
# THE SOFTWARE.
#####################################################################################
find_program(EMBED_LD ld)
find_program(EMBED_OBJCOPY objcopy)

option(EMBED_USE_LD "Use ld to embed data files" OFF)
Copy link
Collaborator

Choose a reason for hiding this comment

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

We shouldn't remove this option.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The technique with LD on Linux is the same as RC on Windows. I changed EMBED_USE_LD to EMBED_USE_RESOURCES because, in both cases, the literature refers to both techniques as embedding resource files in program binary. Here is the example: https://atwillys.de/content/cc/embedding-resource-files-in-a-c-plus-plus-program-binary-on-linux-unix/?lang=en. I can easily find tonnes more. We only need one option to control resources or *.cpp files. It makes no sense to have both and expose EMBED_USE_LD on Windows and EMBOED_USE_RC on Linux.

Copy link
Collaborator

Choose a reason for hiding this comment

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

The technique with LD on Linux is the same as RC on Windows. I changed EMBED_USE_LD to EMBED_USE_RESOURCES because, in both cases, the literature refers to both techniques as embedding resource files in program binary.

The name is the technique used for embedding the resource. We are embedding the resource when we use the C arrays as well, so the EMBED_USE_RESOURCES flag would apply there as well, so it doesnt make sense.

It makes no sense to have both and expose EMBED_USE_LD on Windows and EMBOED_USE_RC on Linux.

I would prefer the two flags because there are other techniques to embed resources as well(such as using #embed) so I would prefer to have a flag to specify the technique being used to embed the resources.

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 prefer a single option for LD and RC (e.g., EMBED_USE_BINARY or EMBED_USE_NATIVE or EMBED_USE_OS_NATIVE) and separate for #embed when we switch to C++23 (e.g. EMBED_USE_CXX_EMBED or EMBED_USE_CXX or EMBED_USE_CXX_NATIVE) or something like that. Two flags for LD and RC would result in different Linux and Windows building instructions. One cannot use LD on Windows and RC on Linux. I prefer to keep them the same for both OSes.

Copy link
Collaborator

Choose a reason for hiding this comment

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

We should probably have one EMBED_USE variable that can be set to EMBED_USE=RC for windows resource files, EMBED_USE=LD for gnu linker, and EMBED_USE=CArrays for the carrays version.

We can also use set_property(CACHE EMBED_USE PROPERTY STRINGS "LD;CArrays") to set the options for linux, so in cmake gui(or tui) you can select the option you want.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done

if(WIN32)
option(EMBED_USE_RESOURCES "Use data file embedding to binary" ON)
else()
option(EMBED_USE_RESOURCES "Use data file embedding to binary" OFF)
endif()

if(NOT WIN32 AND EMBED_USE_RESOURCES)
find_program(EMBED_LD ld)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why are we searching for ld when EMBED_USE_RESOURCES is enabled on linux? This should be an error.

Copy link
Collaborator Author

@apwojcik apwojcik Oct 16, 2023

Choose a reason for hiding this comment

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

Because one may turn it on Linux, see my comment above.

find_program(EMBED_OBJCOPY objcopy)
endif()

function(wrap_string)
set(options)
set(oneValueArgs VARIABLE AT_COLUMN)
set(multiValueArgs)
cmake_parse_arguments(PARSE "${options}" "${oneValueArgs}" "${multiValueArgs}" ${ARGN})
cmake_parse_arguments(WRAP_STRING "${options}" "${oneValueArgs}" "" ${ARGN})
cmake_parse_arguments(PARSE "" "${oneValueArgs}" "" ${ARGN})

string(LENGTH ${${PARSE_VARIABLE}} string_length)
math(EXPR offset "0")
Expand All @@ -54,61 +58,88 @@ function(wrap_string)
set(${PARSE_VARIABLE} "${lines}" PARENT_SCOPE)
endfunction()

function(generate_embed_source EMBED_NAME)
set(options)
set(oneValueArgs SRC HEADER RELATIVE)
set(multiValueArgs OBJECTS SYMBOLS FILES)

cmake_parse_arguments(PARSE "${options}" "${oneValueArgs}" "${multiValueArgs}" ${ARGN})

set(EXTERNS)
set(INIT_KERNELS)

list(LENGTH PARSE_SYMBOLS SYMBOLS_LEN)
list(LENGTH PARSE_OBJECTS OBJECTS_LEN)
if(NOT ${SYMBOLS_LEN} EQUAL ${OBJECTS_LEN})
message(FATAL_ERROR "Symbols and objects dont match: ${SYMBOLS_LEN} != ${OBJECTS_LEN}")
endif()
math(EXPR LEN "${SYMBOLS_LEN} - 1")

foreach(idx RANGE ${LEN})
list(GET PARSE_SYMBOLS ${idx} SYMBOL)
list(GET PARSE_OBJECTS ${idx} OBJECT)
list(GET PARSE_FILES ${idx} FILE)

set(START_SYMBOL "_binary_${SYMBOL}_start")
set(LENGTH_SYMBOL "_binary_${SYMBOL}_length")
if(EMBED_USE_LD)
string(APPEND EXTERNS "
function(generate_embed_source EMBED_NAME SRC_FILE HEADER_FILE BASE_DIRECTORY)
set(multiValueArgs SYMBOLS FILES)
cmake_parse_arguments(PARSE "" "" "${multiValueArgs}" ${ARGN})

set(RESOURCE_ID 100)
foreach(SYMBOL FILE IN ZIP_LISTS PARSE_SYMBOLS PARSE_FILES)
cmake_path(RELATIVE_PATH FILE BASE_DIRECTORY ${BASE_DIRECTORY} OUTPUT_VARIABLE BASE_NAME)
if(WIN32 AND EMBED_USE_RESOURCES)
string(TOUPPER "${SYMBOL}" SYMBOL)
string(APPEND FILE_IDS "#define IDR_${SYMBOL} ${RESOURCE_ID}\n")
string(APPEND RC_MAPPING "IDR_${SYMBOL} TEXTFILE \"${BASE_NAME}\"\n")
string(APPEND INIT_KERNELS " {\"${BASE_NAME}\", resource::read(IDR_${SYMBOL})},\n")
math(EXPR RESOURCE_ID "${RESOURCE_ID} + 1" OUTPUT_FORMAT DECIMAL)
else()
set(START_SYMBOL "_binary_${SYMBOL}_start")
set(LENGTH_SYMBOL "_binary_${SYMBOL}_length")
if(EMBED_USE_RESOURCES)
string(APPEND EXTERNS "
extern const char ${START_SYMBOL}[];
extern const size_t _binary_${SYMBOL}_size;
const auto ${LENGTH_SYMBOL} = reinterpret_cast<size_t>(&_binary_${SYMBOL}_size);
")
else()
string(APPEND EXTERNS "
")
else()
string(APPEND EXTERNS "
extern const char ${START_SYMBOL}[];
extern const size_t ${LENGTH_SYMBOL};
")
")
endif()
string(APPEND INIT_KERNELS "
{ \"${BASE_NAME}\", { ${START_SYMBOL}, ${LENGTH_SYMBOL}} },")
endif()
endforeach()
if(WIN32 AND EMBED_USE_RESOURCES)
set(EXTERNS "
#include <Windows.h>
#include \"resource.h\"

if(PARSE_RELATIVE)
file(RELATIVE_PATH BASE_NAME ${PARSE_RELATIVE} "${FILE}")
else()
get_filename_component(BASE_NAME "${FILE}" NAME)
endif()
namespace resource {
std::string_view read(int id)
{
HMODULE handle = GetModuleHandle(nullptr);
HRSRC rc = FindResource(handle, MAKEINTRESOURCE(id), MAKEINTRESOURCE(TEXTFILE));
HGLOBAL data = LoadResource(handle, rc);
return {static_cast<const char*>(LockResource(data)), SizeofResource(handle, rc)};
}
}

string(APPEND INIT_KERNELS "
{ \"${BASE_NAME}\", { ${START_SYMBOL}, ${LENGTH_SYMBOL}} },")
endforeach()
")
file(WRITE "${EMBED_DIR}/include/resource.h" "
#define TEXTFILE 256

file(WRITE "${PARSE_HEADER}" "
${FILE_IDS}
")
file(WRITE "${EMBED_DIR}/resource.rc" "
#include \"resource.h\"

${RC_FILE_MAPPING}
")
set(EXTERNS "
#include <Windows.h>
#include \"resource.h\"

namespace resource {
std::string_view read(int id)
{
HMODULE handle = GetModuleHandle(nullptr);
HRSRC rc = FindResource(handle, MAKEINTRESOURCE(id), MAKEINTRESOURCE(TEXTFILE));
HGLOBAL data = LoadResource(handle, rc);
return {static_cast<const char*>(LockResource(data)), SizeofResource(handle, rc)};
}
}

")
endif()
file(WRITE "${HEADER_FILE}" "
#include <string_view>
#include <unordered_map>
#include <utility>
std::unordered_map<std::string_view, std::string_view> ${EMBED_NAME}();
")

file(WRITE "${PARSE_SRC}" "
file(WRITE "${SRC_FILE}" "
#include <${EMBED_NAME}.hpp>
${EXTERNS}
std::unordered_map<std::string_view, std::string_view> ${EMBED_NAME}()
Expand All @@ -119,32 +150,24 @@ std::unordered_map<std::string_view, std::string_view> ${EMBED_NAME}()
")
endfunction()

function(embed_file OUTPUT_FILE OUTPUT_SYMBOL FILE)
set(WORKING_DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR})
# Glob is used to compute the relative path
file(GLOB FILES RELATIVE ${WORKING_DIRECTORY} ${FILE})
foreach(REL_FILE ${FILES})
string(MAKE_C_IDENTIFIER "${REL_FILE}" SYMBOL)
function(embed_file FILE BASE_DIRECTORY)
message(STATUS " ${FILE}")
cmake_path(RELATIVE_PATH FILE BASE_DIRECTORY ${BASE_DIRECTORY} OUTPUT_VARIABLE REL_FILE)
string(MAKE_C_IDENTIFIER "${REL_FILE}" OUTPUT_SYMBOL)
if(NOT WIN32 OR NOT EMBED_USE_RESOURCES)
get_filename_component(OUTPUT_FILE_DIR "${REL_FILE}" DIRECTORY)
file(MAKE_DIRECTORY "${CMAKE_CURRENT_BINARY_DIR}/${OUTPUT_FILE_DIR}")
if(EMBED_USE_LD)
set(OUT_FILE "${CMAKE_CURRENT_BINARY_DIR}/${REL_FILE}.o")
else()
set(OUT_FILE "${CMAKE_CURRENT_BINARY_DIR}/${REL_FILE}.cpp")
endif()
set(${OUTPUT_SYMBOL} ${SYMBOL} PARENT_SCOPE)
set(${OUTPUT_FILE} "${OUT_FILE}" PARENT_SCOPE)
if(EMBED_USE_LD)
if(EMBED_USE_RESOURCES)
set(OUTPUT_FILE "${CMAKE_CURRENT_BINARY_DIR}/${REL_FILE}.o")
add_custom_command(
OUTPUT "${OUT_FILE}"
COMMAND ${EMBED_LD} -r -o "${OUT_FILE}" -z noexecstack --format=binary "${REL_FILE}"
COMMAND ${EMBED_OBJCOPY} --rename-section .data=.rodata,alloc,load,readonly,data,contents "${OUT_FILE}"
WORKING_DIRECTORY ${WORKING_DIRECTORY}
OUTPUT ${OUTPUT_FILE}
COMMAND ${EMBED_LD} -r -o "${OUTPUT_FILE}" -z noexecstack --format=binary "${REL_FILE}"
COMMAND ${EMBED_OBJCOPY} --rename-section .data=.rodata,alloc,load,readonly,data,contents "${OUTPUT_FILE}"
WORKING_DIRECTORY ${BASE_DIRECTORY}
DEPENDS ${FILE}
VERBATIM
)
VERBATIM)
else()
set_property(DIRECTORY APPEND PROPERTY CMAKE_CONFIGURE_DEPENDS ${FILE})
set(OUTPUT_FILE "${CMAKE_CURRENT_BINARY_DIR}/${REL_FILE}.cpp")
# reads source file contents as hex string
file(READ ${FILE} HEX_STRING HEX)
# wraps the hex string into multiple lines
Expand All @@ -153,50 +176,45 @@ function(embed_file OUTPUT_FILE OUTPUT_SYMBOL FILE)
string(REGEX REPLACE "([0-9a-f][0-9a-f])" "0x\\1, " ARRAY_VALUES ${HEX_STRING})
# removes trailing comma
string(REGEX REPLACE ", $" "" ARRAY_VALUES ${ARRAY_VALUES})
file(WRITE "${OUT_FILE}" "
file(WRITE "${OUTPUT_FILE}" "
#include <cstddef>
extern const char _binary_${SYMBOL}_start[] = { ${ARRAY_VALUES} };
extern const size_t _binary_${SYMBOL}_length = sizeof(_binary_${SYMBOL}_start);
extern const char _binary_${OUTPUT_SYMBOL}_start[] = { ${ARRAY_VALUES} };
extern const size_t _binary_${OUTPUT_SYMBOL}_length = sizeof(_binary_${OUTPUT_SYMBOL}_start);
")
endif()
endforeach()
set(OUTPUT_FILE ${OUTPUT_FILE} PARENT_SCOPE)
endif()
set(OUTPUT_SYMBOL ${OUTPUT_SYMBOL} PARENT_SCOPE)
endfunction()

function(add_embed_library EMBED_NAME)
set(options)
set(oneValueArgs RELATIVE)
set(multiValueArgs)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Dont remove these extra variables.

cmake_parse_arguments(PARSE "${options}" "${oneValueArgs}" "${multiValueArgs}" ${ARGN})
set(oneValueArgs BASE_DIRECTORY)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why is RELATIVE removed? We need to specify which directory we want the paths to relative to.

Copy link
Collaborator Author

@apwojcik apwojcik Oct 16, 2023

Choose a reason for hiding this comment

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

I changed RELATIVE to BASE_DIRECTORY to align with the cmake_path() command naming to make it cleaner to read cmake_path(RELATIVE_PATH <path-var> BASE_DIRECTORY <input> ...).

Copy link
Collaborator

Choose a reason for hiding this comment

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

It uses RELATIVE to match file(GLOB) command. I dont think cmake_path makes sense 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.

I changed file(GLOB) to cmake_path() for making relative paths. However, I don't care much about the name. If that is the only obstacle blocking this PR and you want the name to be RELATIVE, I make it RELATIVE again.

cmake_parse_arguments(PARSE "" "${oneValueArgs}" "" ${ARGN})

file(MAKE_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}/embed)
file(MAKE_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}/embed/${EMBED_NAME})
set(EMBED_DIR ${CMAKE_CURRENT_BINARY_DIR}/embed/${EMBED_NAME})
file(MAKE_DIRECTORY ${EMBED_DIR})
set(SRC_FILE "${EMBED_DIR}/${EMBED_NAME}.cpp")
set(HEADER_FILE "${EMBED_DIR}/include/${EMBED_NAME}.hpp")
set(WORKING_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR})
set(OUTPUT_FILES)
set(SYMBOLS)
message(STATUS "Embedding files")
message(STATUS "Embedding kernel files:")
foreach(FILE ${PARSE_UNPARSED_ARGUMENTS})
embed_file(OUTPUT_FILE OUTPUT_SYMBOL ${FILE})
embed_file(${FILE} ${PARSE_BASE_DIRECTORY})
list(APPEND OUTPUT_FILES ${OUTPUT_FILE})
list(APPEND SYMBOLS ${OUTPUT_SYMBOL})
endforeach()
message(STATUS "Generating embedding library ${EMBED_NAME}")
generate_embed_source(${EMBED_NAME} SRC ${SRC_FILE} HEADER ${HEADER_FILE} OBJECTS ${OUTPUT_FILES} SYMBOLS ${SYMBOLS} RELATIVE ${PARSE_RELATIVE} FILES ${PARSE_UNPARSED_ARGUMENTS})

set(INTERNAL_EMBED_LIB embed_lib_${EMBED_NAME})
add_library(${INTERNAL_EMBED_LIB} OBJECT "${SRC_FILE}")
target_include_directories(${INTERNAL_EMBED_LIB} PRIVATE "${EMBED_DIR}/include")
target_compile_options(${INTERNAL_EMBED_LIB} PRIVATE -Wno-reserved-identifier -Wno-extern-initializer -Wno-missing-variable-declarations)
set_target_properties(${INTERNAL_EMBED_LIB} PROPERTIES POSITION_INDEPENDENT_CODE On)

add_library(${EMBED_NAME} INTERFACE)
if(EMBED_USE_LD)
target_sources(${EMBED_NAME} INTERFACE ${OUTPUT_FILES})
else()
target_sources(${INTERNAL_EMBED_LIB} PRIVATE ${OUTPUT_FILES})
message(STATUS "Generating embedding library '${EMBED_NAME}'")
generate_embed_source(${EMBED_NAME} ${SRC_FILE} ${HEADER_FILE} "${PARSE_BASE_DIRECTORY}" SYMBOLS ${SYMBOLS} FILES ${PARSE_UNPARSED_ARGUMENTS})
add_library(embed_lib_${EMBED_NAME} OBJECT ${SRC_FILE})
if(NOT EMBED_USE_RESOURCES)
target_sources(embed_lib_${EMBED_NAME} PRIVATE ${OUTPUT_FILES})
endif()
target_include_directories(embed_lib_${EMBED_NAME} PUBLIC ${EMBED_DIR}/include)
target_compile_options(embed_lib_${EMBED_NAME} PRIVATE
-Wno-reserved-identifier -Wno-extern-initializer -Wno-missing-variable-declarations)
set_target_properties(embed_lib_${EMBED_NAME} PROPERTIES POSITION_INDEPENDENT_CODE ON)
add_library(${EMBED_NAME} INTERFACE $<TARGET_OBJECTS:embed_lib_${EMBED_NAME}> ${OUTPUT_FILES})
target_link_libraries(${EMBED_NAME} INTERFACE $<TARGET_OBJECTS:embed_lib_${EMBED_NAME}>)
if(NOT WIN32 AND EMBED_USE_RESOURCES)
target_link_libraries(${EMBED_NAME} INTERFACE ${OUTPUT_FILES})
endif()
target_sources(${EMBED_NAME} INTERFACE $<TARGET_OBJECTS:${INTERNAL_EMBED_LIB}>)
target_include_directories(${EMBED_NAME} INTERFACE "${EMBED_DIR}/include")
target_include_directories(${EMBED_NAME} INTERFACE ${EMBED_DIR}/include)
endfunction()
3 changes: 1 addition & 2 deletions src/targets/gpu/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,6 @@ endif()

file(GLOB KERNEL_FILES CONFIGURE_DEPENDS
${CMAKE_CURRENT_SOURCE_DIR}/kernels/include/migraphx/kernels/*.hpp)
message(STATUS "KERNEL_FILES: ${KERNEL_FILES}")

if(WIN32)
# TODO: re-enable when CK is ported to Windows
Expand All @@ -60,7 +59,7 @@ if(WIN32)
endif()

include(Embed)
add_embed_library(migraphx_kernels ${KERNEL_FILES} RELATIVE ${CMAKE_CURRENT_SOURCE_DIR}/kernels/include/)
add_embed_library(migraphx_kernels ${KERNEL_FILES} BASE_DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR}/kernels/include/)

configure_file(device/targets.hpp.in include/migraphx/gpu/device/targets.hpp)
file(GLOB DEVICE_GPU_SRCS CONFIGURE_DEPENDS ${CMAKE_CURRENT_SOURCE_DIR}/device/*.cpp)
Expand Down