Skip to content

Support 64bit MP_DIGIT on Windows under clang-cl#592

Open
R-Goc wants to merge 1 commit intolibtom:developfrom
R-Goc:clang-cl
Open

Support 64bit MP_DIGIT on Windows under clang-cl#592
R-Goc wants to merge 1 commit intolibtom:developfrom
R-Goc:clang-cl

Conversation

@R-Goc
Copy link
Copy Markdown

@R-Goc R-Goc commented Aug 28, 2025

This commit make MP_DIGIT 64 bit when using clang-cl. It also explicitly sets compile flags for clang-cl.

@R-Goc
Copy link
Copy Markdown
Author

R-Goc commented Aug 28, 2025

This also allows for building a debug version with clang-cl. I couldn't get it to happen with msvc. As in if I did the things required for it it is no longer a debug build as it would require at least turning off real time checks (/RTC1 passed by cmake) which defeats the purpose of a debug build. Maybe I am missing something here.
Other than that on msvc there are still some warnings:

E:\lib\libtommath\mp_get_i64.c(6): warning C4146: unary minus operator applied to unsigned type, result still unsigned
[38/166] Building C object CMakeFiles\libtommath.dir\mp_get_i32.c.obj
E:\lib\libtommath\mp_get_i32.c(6): warning C4146: unary minus operator applied to unsigned type, result still unsigned
[46/166] Building C object CMakeFiles\libtommath.dir\mp_get_l.c.obj
E:\lib\libtommath\mp_get_l.c(6): warning C4146: unary minus operator applied to unsigned type, result still unsigned
[102/166] Building C object CMakeFiles\libtommath.dir\mp_set_i64.c.obj
E:\lib\libtommath\mp_set_i64.c(6): warning C4146: unary minus operator applied to unsigned type, result still unsigned
[103/166] Building C object CMakeFiles\libtommath.dir\mp_set_l.c.obj
E:\lib\libtommath\mp_set_l.c(6): warning C4146: unary minus operator applied to unsigned type, result still unsigned
[104/166] Building C object CMakeFiles\libtommath.dir\mp_set_i32.c.obj
E:\lib\libtommath\mp_set_i32.c(6): warning C4146: unary minus operator applied to unsigned type, result still unsigned
[162/166] Building C object demo\CMakeFiles\test-ltm.dir\test.c.obj
E:\lib\libtommath\demo\test.c(54): warning C4146: unary minus operator applied to unsigned type, result still unsigned
E:\lib\libtommath\demo\test.c(59): warning C4146: unary minus operator applied to unsigned type, result still unsigned

@R-Goc R-Goc force-pushed the clang-cl branch 2 times, most recently from c7bd2fd to 1a39ed7 Compare August 29, 2025 00:10
@R-Goc
Copy link
Copy Markdown
Author

R-Goc commented Aug 29, 2025

My only concern is what happens if tommath is compiled with msvc but the consumer uses clang-cl. At that point everything will break I am afraid. Is MP_WORD anywhere in the ABI of the library? If so then that could also cause issues as __int128 is not stable in the msvc ABI and afaik not even defined.

This commit make MP_DIGIT 64 bit when using clang-cl. It also explicitly
sets compile flags for clang-cl.
Copy link
Copy Markdown
Member

@sjaeckel sjaeckel left a comment

Choose a reason for hiding this comment

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

Thanks for the PR!

Basically LGTM besides the comments.

Comment thread CMakeLists.txt
target_include_directories(${PROJECT_NAME} PUBLIC
$<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}>
$<INSTALL_INTERFACE:${CMAKE_INSTALL_INCLUDEDIR}/${PROJECT_NAME}>
$<INSTALL_INTERFACE:${CMAKE_INSTALL_INCLUDEDIR}>
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This change must be reverted. We decided that our header files will reside in a separate folder.

Comment thread CMakeLists.txt
# We need to link the runtime for 128 bit int support, which clang-cl has
link_libraries(clang_rt.builtins-x86_64.lib)
else()
set(LTM_C_FLAGS /W3 /D_CRT_SECURE_NO_WARNINGS)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Not sure if we want _CRT_SECURE_NO_WARNINGS defined or not. Someone else who cares about Windows as a platform has to decide on that.

Comment thread CMakeLists.txt
if (CMAKE_C_COMPILER_ID STREQUAL "Clang")
# We are clang-cl
# Some flags for linux similarity and warnings
set(LTM_C_FLAGS -fstrict-aliasing /W3 /D_CRT_SECURE_NO_WARNINGS)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Same here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants