This is an archive of the discontinued LLVM Phabricator instance.

[CMake] Disable 64bit file position on old 32 bit Androids
ClosedPublic

Authored by eugene on Aug 30 2017, 2:53 PM.

Details

Summary

This is needed for building LLVM on Android with new NDK (newer than r15c) and API level < 24. Android C library (Bionic) didn't have support for 64 bit file position until Android N.

Diff Detail

Repository
rL LLVM

Event Timeline

eugene created this revision.Aug 30 2017, 2:53 PM

I would add in the commit description that this is Linux/Android specific. For the reference, 32bit systems like BSD ship with 64bit off_t always.

pirama edited edge metadata.Aug 30 2017, 3:06 PM
pirama added subscribers: srhines, enh, danalbert, aoli.

https://reviews.llvm.org/D35648 does something similar. If this is just for building compiler-rt runtimes, we can defer to that patch. This way we can just remove the exception for Android once the NDK defaults to something past API level 24, rather than support a build configuration for ever.

eugene updated this revision to Diff 113324.Aug 30 2017, 3:10 PM

Added comment

https://reviews.llvm.org/D35648 does something similar. If this is just for building compiler-rt runtimes, we can defer to that patch. This way we can just remove the exception for Android once the NDK defaults to something past API level 24, rather than support a build configuration for ever.

I need this for LLDB support on Android. It's going to be a long while till LLDB can default to API level 24.

https://reviews.llvm.org/D35648 does something similar. If this is just for building compiler-rt runtimes, we can defer to that patch. This way we can just remove the exception for Android once the NDK defaults to something past API level 24, rather than support a build configuration for ever.

I need this for LLDB support on Android. It's going to be a long while till LLDB can default to API level 24.

Aah, ok. So https://reviews.llvm.org/D35648 doesn't cover your use case. Can we just say "AND (NOT ANDROID)" instead of a new CMake option?

Aah, ok. So https://reviews.llvm.org/D35648 doesn't cover your use case. Can we just say "AND (NOT ANDROID)" instead of a new CMake option?

Sure we can. I just wanted it to be a bit more flexible.

eugene updated this revision to Diff 113336.Aug 30 2017, 4:27 PM
eugene retitled this revision from CMake option to compile LLVM on 32bit systems with 32bit fpos_t to Disable 64bit file position on old 32 bit Androids.
eugene edited the summary of this revision. (Show Details)
aoli added a comment.Aug 30 2017, 9:18 PM

Pirama: shall I remove remove_definitions(-D_FILE_OFFSET_BITS=64) in https://reviews.llvm.org/D35648? Or we don't want FILE_OFFSET_BITS=64 for runtimes no matter what ANDROID_NATIVE_API_LEVEL is.

LGTM for Android, but I'll let other reviewers formally accept it, in case they have build-system centric comments.

In D37314#857392, @aoli wrote:

Pirama: shall I remove remove_definitions(-D_FILE_OFFSET_BITS=64) in https://reviews.llvm.org/D35648? Or we don't want FILE_OFFSET_BITS=64 for runtimes no matter what ANDROID_NATIVE_API_LEVEL is.

We should remove the line you mention above once this change goes in.

eugene retitled this revision from Disable 64bit file position on old 32 bit Androids to [CMake] Disable 64bit file position on old 32 bit Androids.Aug 31 2017, 1:43 PM

Could somebody please take another look at this change? It would help us fix an internal build with the latest NDK.

srhines accepted this revision.Sep 1 2017, 12:41 PM
This revision is now accepted and ready to land.Sep 1 2017, 12:41 PM
This revision was automatically updated to reflect the committed changes.