[libcxx-commits] [PATCH] D91413: [libcxx] Provide __int128_t builtins needed for filesystem on MSVC

[Martin Storsjö via Phabricator via libcxx-commits]

mstorsjo added a comment.

In D91413#2394199 <https://reviews.llvm.org/D91413#2394199>, @thakis wrote:

> I'd try to do some fix on the clang side. For example, don't define __SIZEOF_INT128T__ when targeting windows triples (and don't have int128_t support in general), or make it a compile-time option, for people who want to link against compiler-rt vs those who don't, or make it not depend on compiler-rt but add the bit above as intrinsic when targeting windows.

The distinction between "has compiler-rt/libgcc or not" already exists, as the distinction between msvc and mingw targets.

As for not defining `__SIZEOF_INT128T__` (i.e. disabling support for `__int128_t`) when targeting msvc, it's simple to do, but it breaks half a dozen clang tests at least, including CodeGenCXX/mangle-ms-templates.cpp, where it seems pretty much intentional that the int128 type is supported for x86_64 MSVC targets.

As for making int128 divisions expand inline to code instead of emitting a libcall, for msvc targets, I guess it would be doable with some amount of effort, but the division expansion code in llvm currently only supports 64 bit divisions (but maybe it'd be straightforward to extend it to 128 bit?).

In any case, I can leave out this and D91139 <https://reviews.llvm.org/D91139> for now; my main target for std::filesystem is for mingw targets, but I thought I'd make it work for MSVC targets at the same time, but that'd require a MSVC target code owner to drive a path forward.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91413