[PATCH] D41813: [builtins] Enable CRT_HAS_128BIT for MSVC

[Loo Rong Jie via Phabricator via llvm-commits]

rongjiecomputer added inline comments.


================
Comment at: lib/builtins/muloti4.c:56
     {
-        if (abs_a > MIN / -abs_b)
+        if (abs_a > __divti3(MIN, -abs_b))
             *overflow = 1;
----------------
joerg wrote:
> 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.
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


https://reviews.llvm.org/D41813