swift
Android cross-compile on macOS: Fix for compile error addressed Float80 data type.
#25502
Merged

Android cross-compile on macOS: Fix for compile error addressed Float80 data type. #25502

vgorloff
vgorloff6 years ago (edited 6 years ago)

Swift forum discussion: https://forums.swift.org/t/android-crosscompilation-on-macos-is-swift-float80-support-really-needed-for-android-intel-x86-and-x86-64-targets/25835

This PR disables Float80 support for Android. This PR fixes compile error shown below:

cd /swift-everywhere-toolchain/ToolChain/Build/darwin/swift && ninja -j3 swiftGlibc-android-i686 swiftCore-android-i686 swiftSwiftOnoneSupport-android-i686
[33/75] Building CXX object stdlib/public/runtime/CMakeFiles/swiftRuntime-android-i686.dir/SwiftDtoa.cpp.o
FAILED: stdlib/public/runtime/CMakeFiles/swiftRuntime-android-i686.dir/SwiftDtoa.cpp.o
/swift-everywhere-toolchain/ToolChain/Build/darwin/llvm/./bin/clang++  \
-DCMARK_STATIC_DEFINE -DGTEST_HAS_RTTI=0 -D_DEBUG -D__STDC_CONSTANT_MACROS \
-D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -Istdlib/public/runtime \
-I/swift-everywhere-toolchain/ToolChain/Sources/swift/stdlib/public/runtime \
-Iinclude -I/swift-everywhere-toolchain/ToolChain/Sources/swift/include \
-I/swift-everywhere-toolchain/ToolChain/Sources/llvm/include \
-I/swift-everywhere-toolchain/ToolChain/Build/darwin/llvm/include \
-I/swift-everywhere-toolchain/ToolChain/Sources/llvm/tools/clang/include \
-I/swift-everywhere-toolchain/ToolChain/Build/darwin/llvm/tools/clang/include \
-I/swift-everywhere-toolchain/ToolChain/Sources/cmark/src \
-I/swift-everywhere-toolchain/ToolChain/Build/darwin/cmark/src \
-Wno-unknown-warning-option -Werror=unguarded-availability-new -fno-stack-protector \
-fPIC -fvisibility-inlines-hidden -Werror=date-time -Werror=unguarded-availability-new -std=c++11 -Wall -Wextra -Wno-unused-parameter \
-Wwrite-strings -Wcast-qual -Wmissing-field-initializers -Wimplicit-fallthrough -Wcovered-switch-default -Wno-class-memaccess \
-Wno-noexcept-type -Wnon-virtual-dtor -Wdelete-non-virtual-dtor -Wstring-conversion -fdiagnostics-color -Werror=switch -Wdocumentation \
-Wimplicit-fallthrough -Wunreachable-code -Woverloaded-virtual -DOBJC_OLD_DISPATCH_PROTOTYPES=0 \
-fno-sanitize=all -DLLVM_DISABLE_ABI_BREAKING_CHECKS_ENFORCING=1 -O3  \
-isysroot /Volumes/Data/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk   \
-UNDEBUG  -fno-exceptions -fno-rtti -Wall -Wglobal-constructors -Wexit-time-destructors -fvisibility=hidden -DswiftCore_EXPORTS \
-I/swift-everywhere-toolchain/ToolChain/Sources/swift/include -DSWIFT_TARGET_LIBRARY_NAME=swiftRuntime \
-target i686-unknown-linux-android \
--sysroot=/Users/vova/Library/Android/sdk/ndk-bundle/platforms/android-24/arch-x86 \
-B /Users/vova/Library/Android/sdk/ndk-bundle/toolchains/x86-4.9/prebuilt/darwin-x86_64/i686-linux-android/bin \
-O2 -momit-leaf-frame-pointer -g0 -DNDEBUG "-I/ndk-bundle/sources/cxx-stl/llvm-libc++/include" \
/ndk-bundle/sysroot/usr/include/i686-linux-android" -D__ANDROID_API__=24 \
-isystem /swift-everywhere-toolchain/ToolChain/Sources/icu/icu4c/source/common \
-isystem /swift-everywhere-toolchain/ToolChain/Sources/icu/icu4c/source/i18n \
-MD -MT stdlib/public/runtime/CMakeFiles/swiftRuntime-android-i686.dir/SwiftDtoa.cpp.o \
-MF stdlib/public/runtime/CMakeFiles/swiftRuntime-android-i686.dir/SwiftDtoa.cpp.o.d \
-o stdlib/public/runtime/CMakeFiles/swiftRuntime-android-i686.dir/SwiftDtoa.cpp.o \
-c /swift-everywhere-toolchain/ToolChain/Sources/swift/stdlib/public/runtime/SwiftDtoa.cpp
/swift-everywhere-toolchain/ToolChain/Sources/swift/stdlib/public/runtime/SwiftDtoa.cpp:148:2: error: "Are you certain `long double` on this platform is really Intel 80-bit extended precision?"

#error "Are you certain `long double` on this platform is really Intel 80-bit extended precision?"
 ^
1 error generated.
vgorloff Fixes issue addressed Float80 data type. Float80 is disabled for Inte…
484bae67
vgorloff vgorloff changed the title Fixes issue addressed Float80 data type. Android cross-compile on macOS: Fix for compile error addressed Float80 data type. 6 years ago
compnerd
compnerd6 years ago
compnerd
compnerd6 years ago

@swift-ci please smoke test

finagolfin
finagolfin6 years ago

Note that @stephentyrone is wrong in that thread when he says "it appears to me that long double on Android is just an alias of double, even on x86." It may seem that way on 32-bit Android platforms, where double and long double are both 64-bit, even on x86, but on 64-bit Android, long double is 128-bit, even on x86_64.

drodriguez
drodriguez approved these changes on 2019-06-16
drodriguez6 years ago

Thanks!

I have started support for Float128 for Android AArch64, but I hit a couple road blocks, and I never finished. I think the only code I uploaded was https://github.com/drodriguez/swift/tree/integers-128 which is needed for some return value of Float128. I have some commit or stash with work for the actual Float128 locally. I will see if I can find it, and upload it, so maybe someone can work on it.

Conversation is marked as resolved
Show resolved
include/swift/Runtime/SwiftDtoa.h
40#if defined(__ANDROID__) && (defined(__i386) || defined(__x86_64__))
41 #undef SWIFT_DTOA_FLOAT80_SUPPORT
42#endif
43
drodriguez6 years ago

Nit: I would create a block above, since SWIFT_DTOA_FLOAT80_SUPPORT is determined there for other platforms. Android is probably falling through || defined(__linux__), so maybe adding #elif defined(__ANDROID__) before that one is enough. I will also remove the checks for the architecture in that case, since I don't think Android has Float80 in any architecture.

vgorloff6 years ago

Let me check that "if/else" checks to avoid #undef...

vgorloff6 years ago

True. Float80 needs to be disabled for any Android architecture.

vgorloff More precise condition check.
402358fb
drodriguez
drodriguez6 years ago

@swift-ci please smoke test

vgorloff
vgorloff6 years ago

Looks like we can merge this PR.
Thank you!

stephentyrone
stephentyrone6 years ago (edited 6 years ago)

Note that @stephentyrone is wrong in that thread when he says "it appears to me that long double on Android is just an alias of double, even on x86." It may seem that way on 32-bit Android platforms, where double and long double are both 64-bit, even on x86, but on 64-bit Android, long double is 128-bit, even on x86_64.

That is a spectacularly bad choice, but I suppose they'll get to learn why in the fullness of time, and the reasons why it's a bad idea fortunately don't really impact Swift. In any event, it doesn't change the basic correctness of this patch; if/when the stdlib vends a Float128 type, we can import long double as that on Android and arm64 Linux.

stephentyrone stephentyrone merged 63706816 into master 6 years ago
vgorloff vgorloff deleted the macos-to-android-crosscompile branch 6 years ago
swiftlang swiftlang deleted a comment from swift-ci on 2019-06-17
swift-ci
swift-ci6 years ago

Build failed
Swift Test Linux Platform
Git Sha - 484bae6

vgorloff
vgorloff6 years ago

Seems like CI trying to build non-existing branch and this cause build failure :0

Login to write a write a comment.

Login via GitHub

Reviewers
Assignees
No one assigned
Labels
Milestone