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.

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

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

Merged
merged 20 commits into from
Nov 21, 2023
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
566bd2f
embed.cmake: add support for Windows resource file
apwojcik Oct 13, 2023
57149a0
Merge branch 'develop' into embed_cmake_windows
apwojcik Oct 27, 2023
76ebf5a
incorporate feedback after review
apwojcik Oct 27, 2023
ea64199
Merge branch 'develop' into embed_cmake_windows
apwojcik Oct 28, 2023
7d51c82
fix compilation on SLES
apwojcik Oct 28, 2023
b0dc9d5
convert tabs into spaces
apwojcik Oct 28, 2023
694044e
incorporate review feedback
apwojcik Oct 30, 2023
e4b71ad
Merge branch 'develop' into embed_cmake_windows
apwojcik Oct 31, 2023
d98a0fc
incorporate review feedback
apwojcik Nov 1, 2023
50c7e11
Merge branch 'develop' into embed_cmake_windows
apwojcik Nov 1, 2023
c49352e
fix compile on Windows
apwojcik Nov 1, 2023
6666984
fix Windows paths in resource script file
apwojcik Nov 2, 2023
1c29d9a
Merge branch 'develop' into embed_cmake_windows
apwojcik Nov 6, 2023
d4d80c7
Merge branch 'develop' into embed_cmake_windows
apwojcik Nov 6, 2023
fb01a1f
Merge branch 'develop' into embed_cmake_windows
tperry-amd Nov 9, 2023
a255850
Merge branch 'develop' into embed_cmake_windows
apwojcik Nov 9, 2023
bff17e2
fix missing function migraphx_kernels()
apwojcik Nov 10, 2023
4f44fd0
Merge branch 'develop' into embed_cmake_windows
apwojcik Nov 10, 2023
0f4dd40
Merge branch 'develop' into embed_cmake_windows
causten Nov 21, 2023
cd22434
Merge branch 'develop' into embed_cmake_windows
causten Nov 21, 2023
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
213 changes: 115 additions & 98 deletions cmake/Embed.cmake
100755 → 100644
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,17 +21,25 @@
# 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)
set(EMBED_USE RC CACHE STRING "Use RC or CArrays to embed data files")
set_property(CACHE EMBED_USE PROPERTY STRINGS "RC;CArrays")
else()
set(EMBED_USE CArrays CACHE STRING "Use LD or CArrays to embed data files")
set_property(CACHE EMBED_USE PROPERTY STRINGS "LD;CArrays")
endif()

if(EMBED_USE STREQUAL "LD")
find_program(EMBED_LD ld REQUIRED)
find_program(EMBED_OBJCOPY objcopy REQUIRED)
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})

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

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

set(oneValueArgs)
set(multiValueArgs 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 "
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(EMBED_USE STREQUAL "RC")
string(TOUPPER "${SYMBOL}" SYMBOL)
string(APPEND FILE_IDS "#define IDR_${SYMBOL} ${RESOURCE_ID}\n")
cmake_path(NATIVE_PATH FILE NORMALIZE NATIVE_FILE)
string(REPLACE "\\" "\\\\" NATIVE_FILE "${NATIVE_FILE}")
string(APPEND RC_FILE_MAPPING "IDR_${SYMBOL} TEXTFILE \"${NATIVE_FILE}\"\n")
string(APPEND INIT_KERNELS "\n {\"${BASE_NAME}\", resource::read(IDR_${SYMBOL})},")
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 STREQUAL "LD")
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(EMBED_USE STREQUAL "RC")
file(WRITE "${EMBED_DIR}/include/resource.h" "
#define TEXTFILE 256

if(PARSE_RELATIVE)
file(RELATIVE_PATH BASE_NAME ${PARSE_RELATIVE} "${FILE}")
else()
get_filename_component(BASE_NAME "${FILE}" NAME)
endif()
${FILE_IDS}
")
file(WRITE "${EMBED_DIR}/resource.rc" "
#include \"resource.h\"

string(APPEND INIT_KERNELS "
{ \"${BASE_NAME}\", { ${START_SYMBOL}, ${LENGTH_SYMBOL}} },")
endforeach()
${RC_FILE_MAPPING}
")
set(EXTERNS "
#include <Windows.h>
#include \"resource.h\"

file(WRITE "${PARSE_HEADER}" "
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)};
}
}
")
set(EMBED_FILES ${EMBED_DIR}/include/resource.h ${EMBED_DIR}/resource.rc)
endif()
file(WRITE "${EMBED_DIR}/include/${EMBED_NAME}.hpp" "
#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 "${EMBED_DIR}/${EMBED_NAME}.cpp" "
#include <${EMBED_NAME}.hpp>
${EXTERNS}
std::unordered_map<std::string_view, std::string_view> ${EMBED_NAME}()
{
static std::unordered_map<std::string_view, std::string_view> result = {${INIT_KERNELS}};
static std::unordered_map<std::string_view, std::string_view> result = {${INIT_KERNELS}
};
return result;
}
")
list(APPEND EMBED_FILES ${EMBED_DIR}/${EMBED_NAME}.cpp ${EMBED_DIR}/include/${EMBED_NAME}.hpp)
set(EMBED_FILES ${EMBED_FILES} PARENT_SCOPE)
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)
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)
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}
DEPENDS ${FILE}
VERBATIM
)
else()
set_property(DIRECTORY APPEND PROPERTY CMAKE_CONFIGURE_DEPENDS ${FILE})
# reads source file contents as hex string
file(READ ${FILE} HEX_STRING HEX)
# wraps the hex string into multiple lines
wrap_string(VARIABLE HEX_STRING AT_COLUMN 80)
# adds '0x' prefix and comma suffix before and after every byte respectively
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}" "
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)
get_filename_component(OUTPUT_FILE_DIR "${REL_FILE}" DIRECTORY)
file(MAKE_DIRECTORY "${CMAKE_CURRENT_BINARY_DIR}/${OUTPUT_FILE_DIR}")
if(EMBED_USE STREQUAL "LD")
set(OUTPUT_FILE "${CMAKE_CURRENT_BINARY_DIR}/${REL_FILE}.o")
add_custom_command(
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)
set(OUTPUT_FILE ${OUTPUT_FILE} PARENT_SCOPE)
elseif(EMBED_USE STREQUAL "CArrays")
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
wrap_string(VARIABLE HEX_STRING AT_COLUMN 80)
# adds '0x' prefix and comma suffix before and after every byte respectively
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 "${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)
Expand All @@ -168,35 +188,32 @@ function(add_embed_library EMBED_NAME)
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})

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})
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")
file(MAKE_DIRECTORY ${EMBED_DIR})
message(STATUS "Embedding kernel files:")
foreach(FILE ${PARSE_UNPARSED_ARGUMENTS})
embed_file(OUTPUT_FILE OUTPUT_SYMBOL ${FILE})
embed_file(${FILE} ${PARSE_RELATIVE})
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})

message(STATUS "Generating embedding library '${EMBED_NAME}'")
generate_embed_source(${EMBED_NAME} ${EMBED_DIR} "${PARSE_RELATIVE}" SYMBOLS ${SYMBOLS} FILES ${PARSE_UNPARSED_ARGUMENTS})
set(INTERNAL_EMBED_LIB embed_lib_${EMBED_NAME})
add_library(${INTERNAL_EMBED_LIB} OBJECT "${SRC_FILE}")
add_library(${INTERNAL_EMBED_LIB} OBJECT ${EMBED_FILES})
if(EMBED_USE STREQUAL "CArrays")
target_sources(${INTERNAL_EMBED_LIB} PRIVATE ${OUTPUT_FILES})
endif()
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)
if(EMBED_USE STREQUAL "LD")
target_sources(${EMBED_NAME} INTERFACE ${OUTPUT_FILES})
else()
target_sources(${INTERNAL_EMBED_LIB} PRIVATE ${OUTPUT_FILES})
endif()
if(EMBED_USE STREQUAL "RC")
target_link_libraries(${EMBED_NAME} INTERFACE $<TARGET_OBJECTS:${INTERNAL_EMBED_LIB}>)
endif()
target_sources(${EMBED_NAME} INTERFACE $<TARGET_OBJECTS:${INTERNAL_EMBED_LIB}>)
target_include_directories(${EMBED_NAME} INTERFACE "${EMBED_DIR}/include")
endfunction()

1 change: 0 additions & 1 deletion src/targets/gpu/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,6 @@ endif()

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

if(NOT MIGRAPHX_USE_COMPOSABLEKERNEL)
list(REMOVE_ITEM KERNEL_FILES
Expand Down
Loading