Submitted as an upstream fix
MathExtras has many useful tools. However, they aren't available to use in constexpr equations or functions. Originally this was a patch for swift llvm but it works upstream and as such I have put this PR upstream.
Paths
| Differential D107240
[LLVM][Support][NFC] Make multiple bitwise MathExtras functions constexpr AbandonedPublic Authored by gAlfonso-bit on Aug 1 2021, 11:35 AM.
Details
Summary Submitted as an upstream fix MathExtras has many useful tools. However, they aren't available to use in constexpr equations or functions. Originally this was a patch for swift llvm but it works upstream and as such I have put this PR upstream.
Diff Detail Event TimelinegAlfonso-bit retitled this revision from Make multiple mathextras functions constexpr to [LLVM]ake multiple mathextras functions constexpr. gAlfonso-bit retitled this revision from [LLVM]ake multiple mathextras functions constexpr to [LLVM] Make multiple mathextras functions constexpr.Aug 2 2021, 9:48 AM gAlfonso-bit retitled this revision from [LLVM] Make multiple mathextras functions constexpr to [LLVM][NFC] Make multiple mathextras functions constexpr.Aug 2 2021, 9:55 AM gAlfonso-bit retitled this revision from [LLVM][NFC] Make multiple mathextras functions constexpr to [LLVM][NFC] Make multiple bitwise MathExtras functions constexpr.Aug 2 2021, 11:30 AM gAlfonso-bit retitled this revision from [LLVM][NFC] Make multiple bitwise MathExtras functions constexpr to [LLVM][Support][NFC] Make multiple bitwise MathExtras functions constexpr.Aug 4 2021, 8:17 AM gAlfonso-bit retitled this revision from [LLVM][Support][NFC] Make multiple bitwise MathExtras functions constexpr to [LLVM][NFC] Make multiple bitwise MathExtras functions constexpr.Aug 4 2021, 8:21 AM Comment Actions Please upload patches with full context using -U999999 per the instructions here https://llvm.org/docs/Phabricator.html#requesting-a-review-via-the-web-interface Comment Actions
Alright, I did.
gAlfonso-bit added inline comments.
gAlfonso-bit added inline comments.
gAlfonso-bit marked an inline comment as done. Comment Actions clang-tidy is complaining about function names, but changing every function instance would modify too many files
Comment Actions I copied a chunk of this patch to godbolt and added two different constexpr usages of countLeadingZeros that both fail with MSVC https://godbolt.org/z/Maen4bac4 I'm guessing that because everything is a template, MSVC doesn't diagnose anything until you try to use it in constexpr context.
gAlfonso-bit retitled this revision from [LLVM][NFC] Make multiple bitwise MathExtras functions constexpr to [LLVM][Support][NFC] Make multiple bitwise MathExtras functions constexpr.Aug 5 2021, 11:30 AM This comment was removed by gAlfonso-bit.
Comment Actions Is it possible to add unittests showing the count functions being used in constexpr contexts to prove that this will work with MSVC? Comment Actions Unfortunately, as long as we are using c++14, there is actually no easy solution to this Comment Actions @craig.topper I could use the libcpp_clz, which is constexpr, but that seems to be c++ 20 only.
Revision Contents
Diff 364498 llvm/include/llvm/Support/MathExtras.h
llvm/lib/Support/MathExtras.cpp
|
Doesn't this mean that users of this that want to use it in constexpr can't compile that code with MSVC and would need a different solution just for MSVC? Is that useful for the project?