omjavaid commented on a change in pull request #11383:
URL: https://github.com/apache/arrow/pull/11383#discussion_r733154445



##########
File path: cpp/cmake_modules/ThirdpartyToolchain.cmake
##########
@@ -903,7 +903,13 @@ if(ARROW_USE_UBSAN)
   set(ARROW_USE_NATIVE_INT128 FALSE)
 else()
   include(CheckCXXSymbolExists)
-  check_cxx_symbol_exists("__SIZEOF_INT128__" "" ARROW_USE_NATIVE_INT128)
+  check_cxx_symbol_exists("_M_ARM64" "" WIN32_ARM64_TARGET)
+  if (WIN32_ARM64_TARGET AND CMAKE_CXX_COMPILER_ID MATCHES "Clang")
+    # NOTE: For clang/win-arm64, native int128_t produce linker errors
+    set(ARROW_USE_NATIVE_INT128 FALSE)
+  else ()
+    check_cxx_symbol_exists("__SIZEOF_INT128__" "" ARROW_USE_NATIVE_INT128)
+  endif()

Review comment:
       > Apparently, clang for windows on arm64 has some bugs around int128_t 
type. The issue is very similar to the linker issue highlighted in line 900.
   > 
   > ```
   > if(ARROW_USE_UBSAN)
   >   # NOTE: Avoid native int128_t on clang with UBSan as it produces linker 
errors
   >   # (such as "undefined reference to '__muloti4'")
   >   set(ARROW_USE_NATIVE_INT128 FALSE)
   > else()
   > ```
   > 
   > For example even simple programs like following would throw an error
   > 
   > ```
   > int main(){
   >     __int128_t x, y;
   >     // int128_t div
   >     auto r = x / y;
   >     // cast int128_t to float
   >     float mean = static_cast<float>(x);
   >     return 0;
   > }
   > ```
   > 
   > Linker error raised
   > 
   > ```
   > lld-link: error: undefined symbol: __divti3
   > >>> referenced by C:\Users\niysai01\AppData\Local\Temp\test-ed00b3.o:(main)
   > 
   > lld-link: error: undefined symbol: __floattisf
   > >>> referenced by C:\Users\niysai01\AppData\Local\Temp\test-ed00b3.o:(main)
   > ```
   > 
   > I am waiting for an LLVM account to file a bug report.
   > 
   > I had this change initially but then reverted it thinking that it wasn't 
necessary and since I wasn't doing a clean build this didn't show up as an 
issue after reverting. I have tried on a clean checkout yesterday and can 
confirm that this change is required.
   
   This looks more like a feature request where we want Clang to enable VC++ 
missing intrinsic like __int128 from compiler-rt when building for MSVC




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to