It seems that the run time for Windows has changed and supports more math functions than it used to, especially on AArch64, ARM, and AMD64, since at least VS2015, if not earlier.
Fixes PR40541.
Differential D57625
[TargetLibraryInfo] Update run time support for Windows evandro on Feb 1 2019, 3:08 PM. Authored by
Details
It seems that the run time for Windows has changed and supports more math functions than it used to, especially on AArch64, ARM, and AMD64, since at least VS2015, if not earlier. Fixes PR40541.
Diff Detail
Event TimelineComment Actions Even if a specific version of MSVC is required to compile LLVM, code generated by LLVM can run on a wider range of msvcrt versions. (And it also works with mingw which normally uses an ancient msvcrt.) I didn't look closely at the change itself yet. Comment Actions Indeed, this change only affects Windows proper, as both Cygwin and MinGW were already excluded from this selection of supported math functions. Comment Actions Oh, right - I didn't check the patch that far. Ok, so that's not a concern then. But the other point still stands; even if compiling LLVM requires VS 2015 or newer, the generated code can work with older MSVC runtimes as well. @rnk - is there any declared minimum for this? I'm using Clang with MSVC 2013 SDK/runtimes just fine in one setup at least (although this setup currently uses Clang/LLVM 4.0, I haven't recently retested it with latest). AFAIK, the exact version of MSVC that is targeted is kept available via Triplet::getEnvironmentVersion, so unless it's intended to drop support for all the older runtimes as well, this would have to check the version of it. Other than that, the patch is pretty hard to follow - it would be easier split up in individual changes - from my reading through it so far, I can see 3-4 different kinds of changes. E.g. moving the x86 specific single precision function block, making some math functions available, marking some other functions as unavailable (copysignf etc).
Comment Actions It's not clear to me that the version of the MSVCRT is handled by the triplet. However, when I worked at AMD, we worked with MS to get them more math functions into it for x86-64, so it's probably been the case since then. However, after 15 years or so, my recollection of the details is fuzzy. Again, I don't work on Windows, so I'd appreciate any help sorting out the legality of these changes.
Comment Actions Do we know when most of these were added? Did the all arrive in more or less the same version? It would be nice to make this conditional on a new enough environment version. That gives users a way to indicate that they are targetting an old CRT if they need to. Comment Actions Thanks, this looks much more readable!
Hmm, indeed. The triplet can be used to set the MSVC compatibility version though, e.g. like -target <arch>-windows-msvc19 for indicating MSVC 19 (aka 2015), which is the version that got UCRT which has got all these new functions. I expected the reverse to be true as well, that the internal triplet gets populated with the version number, but it seems I might have misread the source. In Clang it's handled via code like LangOpts.isCompatibleWithMSVC(LangOptions::MSVC2015) and similar. @rnk - do you know if it's possible to check the MSVC(RT) target version within LLVM somehow?
Comment Actions They're certainly available since UCRT, but msvcr120.dll (MSVC 2013, aka 18.00) also seems to have at least rint, round, cbrt, exp2. (I'm checking https://sourceforge.net/p/mingw-w64/mingw-w64/ci/master/tree/mingw-w64-crt/lib64/msvcr120.def.in.) Comment Actions I don't think we need to go the extra mile to provide rint to users targeting 2013. We can reduce it to just pre-2015 and post, as is done in this patch.
Comment Actions Goodness, I wrote this document (https://is.gd/vQYYpY) in 2004 about VS6! Just don't ask me any question about it, as it was in another life.
Comment Actions Assume VC19 as the default. However, I wonder if this should be the default earlier, in the clang driver.
Comment Actions I looked at the version checks and I'm happy with this. I haven't read all the discussion between you and @mstorsjo, but feel free to land if his concerns are addressed. Thanks for circling around and addressing this!
|