[libcxx-commits] [PATCH] D114805: [NFC][libc++] Recognize int128 as an ABI affecting property

Fri Mar 4 10:58:30 PST 2022

ldionne added a comment.

I trust that you're right when you say it affects the ABI, but do you have an example (I'm sure there's many, I just can't think of one right now).

Also, can you please rebase and re-upload to poke the CI? I have a few comments but I think this makes sense.

================
Comment at: libcxx/CMakeLists.txt:315
+check_cxx_symbol_exists(__SIZEOF_INT128__ "" LIBCXX_HAVE_INT128)
+cmake_dependent_option(LIBCXX_USE_INT128   "Use __int128 in the library" ON
+"LIBCXX_HAVE_INT128" OFF)
----------------
I don't think we should allow this to be set explicitly during CMake configuration. If it's an ABI affecting property of the compiler, I believe we should just detect it and set/unset it unconditionally.

I can't imagine building libc++ with one compiler and using it with a compiler so different that it would differ in whether it supports `int128`. For example, compiling libc++ with a version of Clang and using the headers with a different version of Clang clearly works, however I wouldn't dare ship a library compiled with e.g. GCC to my users. Not saying it won't work, but it seems like living on the edge -- enough that I don't think it's worth supporting. WDYT?

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D114805/new/

https://reviews.llvm.org/D114805