LLVM Bugzilla is read-only and represents the historical archive of all LLVM issues filled before November 26, 2021. Use github to submit LLVM bugs

Bug 37983 - Clang/LLVM optimizes division and modulo worse than MSVC
Summary: Clang/LLVM optimizes division and modulo worse than MSVC
Status: NEW
Alias: None
Product: libraries
Classification: Unclassified
Component: Scalar Optimizations (show other bugs)
Version: trunk
Hardware: PC All
: P normal
Assignee: Unassigned LLVM Bugs
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2018-06-28 17:38 PDT by Stephan T. Lavavej
Modified: 2021-07-07 09:26 PDT (History)
8 users (show)

See Also:
Fixed By Commit(s):


Attachments
Test case (505 bytes, text/plain)
2018-06-28 17:38 PDT, Stephan T. Lavavej
Details
Clang codegen for workaround (1.10 KB, text/plain)
2018-06-28 17:39 PDT, Stephan T. Lavavej
Details
Clang codegen for modulo (this is the bug) (1.41 KB, text/plain)
2018-06-28 17:40 PDT, Stephan T. Lavavej
Details
MSVC codegen for workaround (2.59 KB, text/plain)
2018-06-28 17:41 PDT, Stephan T. Lavavej
Details
MSVC codegen for modulo (this is fine) (2.59 KB, text/plain)
2018-06-28 17:41 PDT, Stephan T. Lavavej
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Stephan T. Lavavej 2018-06-28 17:38:53 PDT
Created attachment 20488 [details]
Test case

This affects the Ryu algorithm for printing floating-point numbers (https://github.com/ulfjack/ryu ) and therefore affects C++17 floating-point std::to_chars(). This is possibly the same bug as https://bugs.llvm.org/show_bug.cgi?id=23106 "Division followed by modulo generates longer machine code than vice versa".

I observe that MSVC's codegen is unaffected by USE_MODULO, while Clang/LLVM generates more assembly code (which is slower when profiled in the real algorithm) for USE_MODULO.

C:\Temp\TESTING_X64>cl
Microsoft (R) C/C++ Optimizing Compiler Version 19.15.26504 for x64
Copyright (C) Microsoft Corporation.  All rights reserved.

usage: cl [ option... ] filename... [ /link linkoption... ]

C:\Temp\TESTING_X64>clang-cl -m64 -v
clang version 6.0.0 (tags/RELEASE_600/final)
Target: x86_64-pc-windows-msvc
Thread model: posix
InstalledDir: S:\msvc\src\vctools\NonShip\ClangLLVM\bin

C:\Temp\TESTING_X64>type meow.cpp
unsigned long long ryu(unsigned long long vp, unsigned long long vm) {
    bool vmIsTrailingZeros = true;

    while (vp / 10 > vm / 10) {
#ifdef USE_MODULO
        vmIsTrailingZeros &= vm % 10 == 0;
#else
        // The compiler does not realize that vm % 10 can be computed from vm / 10
        // as vm - (vm / 10) * 10.
        vmIsTrailingZeros &= vm - (vm / 10) * 10 == 0; // vm % 10 == 0;
#endif
        vp /= 10;
        vm /= 10;
    }

    return vmIsTrailingZeros ? vp : vm;
}

C:\Temp\TESTING_X64>cl /EHsc /nologo /W4 /MT /O2 /c /FAsc /Famsvc_workaround.cod meow.cpp
meow.cpp

C:\Temp\TESTING_X64>cl /EHsc /nologo /W4 /MT /O2 /c /FAsc /Famsvc_modulo.cod /DUSE_MODULO meow.cpp
meow.cpp

C:\Temp\TESTING_X64>clang-cl -m64 /EHsc /nologo /W4 /MT /O2 /c /FA /Faclang_workaround.asm meow.cpp

C:\Temp\TESTING_X64>clang-cl -m64 /EHsc /nologo /W4 /MT /O2 /c /FA /Faclang_modulo.asm /DUSE_MODULO meow.cpp

C:\Temp\TESTING_X64>
Comment 1 Stephan T. Lavavej 2018-06-28 17:39:54 PDT
Created attachment 20489 [details]
Clang codegen for workaround
Comment 2 Stephan T. Lavavej 2018-06-28 17:40:28 PDT
Created attachment 20490 [details]
Clang codegen for modulo (this is the bug)
Comment 3 Stephan T. Lavavej 2018-06-28 17:41:00 PDT
Created attachment 20491 [details]
MSVC codegen for workaround
Comment 4 Stephan T. Lavavej 2018-06-28 17:41:23 PDT
Created attachment 20492 [details]
MSVC codegen for modulo (this is fine)
Comment 5 Stephan T. Lavavej 2018-06-28 17:54:31 PDT
Here's a Godbolt link demonstrating the codegen difference (this isn't Windows-specific): https://godbolt.org/g/RsUT4a
Comment 6 Trass3r 2018-11-24 05:19:55 PST
Relevant: https://bugs.llvm.org/show_bug.cgi?id=35479
Comment 7 Nico Weber 2019-07-26 19:57:04 PDT
https://reviews.llvm.org/rL364600 seems to fix this. Can you confirm?
Comment 8 Nico Weber 2019-07-30 18:24:17 PDT
Looks like that landed in https://reviews.llvm.org/rL367374 . Is this done now?
Comment 9 Nico Weber 2019-07-30 18:25:01 PDT
Sorry, wrong bug, please ignore comment 8 (but not comment 7)
Comment 10 Stephan T. Lavavej 2019-07-30 18:43:20 PDT
Looks like this got into the 9.x release branch; I'll try to verify this soon. Thanks!