[builtins] Enable CRT_HAS_128BIT for MSVC

[rongjiecomputer]

Summary

This allow clang/MSVC to use __int128 and get runtime functions from clang_rt.builtins-<arch>.lib.

The following tests are temporarily disabled for MSVC as both pure MSVC and clang/MSVC do not have __float128 and 80-bit precision long double. Fix them when clang/MSVC exposes __float128 and 80-bit precision long double.

• fixunsxfti_test.c
• fixxfti_test.c
• floattixf_test.c
• floatuntixf_test.c

[joerg]

This works in 32bit mode as well? I'm suprised.

[rongjiecomputer]

@joerg You are right. I have checked that __int128 is not available when compile with -target i386-windows-msvc.

Added extra guard with _WIN64.

[rongjiecomputer]

Gentle ping after 7 days.

[rnk]

Thanks, looks good. I will apply this.

[rnk]

I get these test failures in check-builtins now, so I can't merge this: https://reviews.llvm.org/P8060

[rongjiecomputer]

There is a compile warning for char assumption_3[sizeof(long double)*CHAR_BIT == 128] = {0} in these codes because long double == double in Clang + MSVC:

• fixunsxfti_test.c
• fixxfti_test.c
• floattixf_test.c
• floatuntixf_test.c

mulvti3_test.c hit a compilerrt_abort:

..\lib\builtins\mulvti3.c:50: abort in __mulvti3

Summary of other test failures:

fixunsdfti_test.c
error in __fixunsdfti(0X1.FFFFFFFFFFFFFP+63) = 0x0000000000000000FFFFFFFFFFFFF800, expected 0xFFFFFFFFFFFFFFFFFFFFFFFFFFFFF800

fixunssfti_test.c
error in __fixunssfti(0X1.FFFFFE0000000P+63) = 0x0000000000000000FFFFFF0000000000, expected 0xFFFFFFFFFFFFFFFFFFFFFF0000000000

fixunsxfti_test.c
error in __fixunsxfti(0X1.0000000000000P+0) = 0x00000000000000000000000000000000, expected 0x00000000000000000000000000000001

fixxfti_test.c
error in __fixxfti(0X1.0000000000000P+0) = 0x00000000000000000000000000000000, expected 0x00000000000000000000000000000001

floattixf_test.c
error in __floattixf(0x00000000000000000000000000000001) = 0X1.0000000000000P-1022, expected 0X1.0000000000000P+0

floatuntisf_test.c
error in __floatuntisf(0xFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFE) = inf, expected 0x1.0000000000000p+64

floatuntixf_test.c
error in __floatuntixf(0x00000000000000000000000000000001) = 0X1.0000000000000P-1022, expected 0X1.0000000000000P+0

modti3_test.c
error in __modti3: 0xFFFFFFFFFFFFFFFF8000000000000000 % 0x00000000000000000000000000000003 = 0xFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFE, expected 0x00000000000000000000000000000002

muloti4_test.c
error in __muloti4: overflow=1 expected=0
error in __muloti4: 0x000000000000000000000000B504F333 * 0x000000000000000000000000B504F333 = 0x00000000000000007FFFFFFE9EA1DC29, expected 0x00000000000000007FFFFFFE9EA1DC29

Since the tests work on other platforms, I am guessing the tests fail for clang-cl due to Clang trying to imitate MSVC's bugs. I am not sure how to make them work, so I might have to abandon this patch.

Thanks for your time to review this.

[rongjiecomputer]

Fix several bugs that only appear on clang/MSVC.

Disable the following tests for MSVC due to lack of __float128 and 80-bit precision long double

• fixunsxfti_test.c
• fixxfti_test.c
• floattixf_test.c
• floatuntixf_test.c

ninja -C build check-builtins
ninja: Entering directory `build'
[0/1] Running the Builtins tests
-- Testing: 194 tests, 4 threads --
Testing: 0 .. 10.. 20.. 30.. 40.. 50.. 60.. 70.. 80.. 90..
Testing Time: 357.44s
  Expected Passes    : 167
  Expected Failures  : 2
  Unsupported Tests  : 25

[rongjiecomputer]

@rnk I decided to continue to work on this patch, can you take a look?

Explanation:

1.) Change the suffix of some hex literals from LL to ULL to work around the following clang/MSVC's "bug":

http://coliru.stacked-crooked.com/a/e090d6db12ac32aa

On GCC/Mingw and GCC/Linux, the output is:

Hello
0000000000000000FFFFFFFFFFFFF800
0000000000000000FFFFFFFFFFFFF800

But on Clang/MSVC, it is:

World
FFFFFFFFFFFFFFFFFFFFFFFFFFFFF800
0000000000000000FFFFFFFFFFFFF800

2.) Change __int128's / operations to explicit __divti3 function calls, also to work around clang/MSVC's bug.

3.) Disable the following failing tests (all related to __float128/long double which clang/MSVC does not have):

• fixunsxfti_test.c
• fixxfti_test.c
• floattixf_test.c
• floatuntixf_test.c

All *xf* builtin functions won't work as they require long double to be 128 bits, but I don't bother to
disable them right now as clang/MSVC won't emit calls to these functions anyway (until __float128 is exposed).

I plan to start a discussion in cfe-dev about exposing __float128 and long double for clang/MSVC.
clang/MSVC already exposes __int128, I think exposing __float128 should be low-risk.

Do we really need to follow all the bugs in pure MSVC?

[joerg]

lib/builtins/muloti4.c

56 ↗

(On Diff #134894)

I find this and related changes to be not acceptable. It hard-codes the assumption that the division is not lowered natively by the compiler, which is IMO not valid.

[rongjiecomputer]

lib/builtins/muloti4.c

56 ↗

(On Diff #134894)

By "related change", I assume it means changes in muloti4.c, mulvti3.c and int_lib.h only.

clang/MSVC has a very weird bug. clang/MSVC does lower ti_int '/' to __divti3. For large ti_int values, if the values are known in compile time (forced by constexpr), / will produce correct answer, but if the values are only known in runtime, / will sometime return 0. / still works for small runtime values. __divti3 always works.

Also, [1], [2] etc directly calls __divsi3 instead of letting compiler to lower int division. I do not see why explicitly calling __divti3 is so harmful here.

[1]: https://github.com/llvm-mirror/compiler-rt/blob/master/lib/builtins/divmodsi4.c#L22
[2]: https://github.com/llvm-mirror/compiler-rt/blob/master/lib/builtins/modsi3.c#L22

[rnk]

I'm not a compiler-rt person and I can't find time for this.

[joerg]

The difference is that modsi3 etc are all paired instructions. A backend should not be lowering to one of them if a real division instruction exists and it should be consistent in the lowering.

[gpakosz]

Hello,

I landed here after having discussed https://bugs.llvm.org/show_bug.cgi?id=25305

Can someone please provide feedback on the status of this PR please?

Thank you

[rongjiecomputer]

I am not working on this CL anymore. Changes need to be made in LLVM first to make passing of __int128 and long double
works with Microsoft ABI and I don't have such knowledge to make it happen.

After LLVM and Clang are fixed, then this CL should work.

Some links that might help future contributors who want to try this:

• https://bugs.chromium.org/p/chromium/issues/detail?id=787617
• https://github.com/JuliaLang/julia/blob/3a26bbc32334253bb582eb058596683ff6fe2fb1/deps/patches/win64-int128.llvm-3.3.patch (Julia used to have a patch to fix this in LLVM, but this patch is gone in master branch for some reason. Not sure if the patch is correct or not).
• https://github.com/ziglang/zig/pull/930 (Zig wraps compiler-rt with inline assembly to conform with Microsoft ABI instead of touching LLVM).
• https://software.intel.com/en-us/articles/80-bit-long-double-math-functions-from-intel-may-conflict-with-microsoft-visual-c-64-bit

lib/builtins/int_types.h

     }s;
 } udwords;
 
-#if (defined(__LP64__) || defined(__wasm__) || defined(__mips64))
+#if (defined(__LP64__) || defined(__wasm__) || defined(__mips64) || \
+     defined(_MSC_VER))
 #define CRT_HAS_128BIT
 #endif