Bug 230437 – Add LLVM/FLang's int128_t implementation to WTF

• Component: New Bugs
• Status: RESOLVED FIXED

[Philip Chimento]

Add LLVM/FLang's int128_t implementation to WTF

[Philip Chimento]

This will be necessary for implementing Temporal.Instant.

[Philip Chimento]

Created attachment 438537 [details]
Patch

[Philip Chimento]

(The style checker is going to fail because the source files contain no copyright notices; I'm not sure what to do about that, because there are none in the original code.)

[Yusuke Suzuki]

Comment on attachment 438537 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=438537&action=review

r=me with comments

> Source/WTF/wtf/CMakeLists.txt:106
> +    Int128.h

Add this header to xcodeproj.

> Source/WTF/wtf/CMakeLists.txt:117
> +    LeadingZeroBitCount.h

Add this header to xcodeproj.

> Source/WTF/wtf/LeadingZeroBitCount.h:41
> +static constexpr std::uint64_t deBruijn {0x07edd5e59a4e28c2};
> +static constexpr std::uint8_t mapping[64] {63, 0, 58, 1, 59, 47, 53, 2, 60, 39,
> +    48, 27, 54, 33, 42, 3, 61, 51, 37, 40, 49, 18, 28, 20, 55, 30, 34, 11, 43,
> +    14, 22, 4, 62, 57, 46, 52, 38, 26, 32, 41, 50, 36, 17, 19, 29, 10, 13, 21,
> +    56, 45, 25, 31, 35, 16, 9, 12, 44, 24, 15, 8, 23, 7, 6, 5};
> +} // namespace

You should create cpp file, and putting these constant arrays in C++ side. Otherwise, each translation unit will materialize this duplicate array, and enlarging binary size.

> Source/WTF/wtf/LeadingZeroBitCount.h:43
> +inline constexpr int LeadingZeroBitCount(std::uint64_t x)

Rename it to leadingZeroBitCount

> Source/WTF/wtf/LeadingZeroBitCount.h:64
> +inline constexpr int LeadingZeroBitCount(std::uint32_t x)

Ditto.

> Source/WTF/wtf/LeadingZeroBitCount.h:69
> +inline constexpr int LeadingZeroBitCount(std::uint16_t x)

Ditto

> Source/WTF/wtf/LeadingZeroBitCount.h:86
> +namespace detail {
> +static constexpr std::uint8_t eightBitLeadingZeroBitCount[256] {8, 7, 6, 6, 5, 5,
> +    5, 5, 4, 4, 4, 4, 4, 4, 4, 4, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3,
> +    3, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2,
> +    2, 2, 2, 2, 2, 2, 2, 2, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1,
> +    1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1,
> +    1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 0, 0, 0,
> +    0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
> +    0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
> +    0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
> +    0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
> +    0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0};
> +}

Ditto.

> Source/WTF/wtf/LeadingZeroBitCount.h:88
> +inline constexpr int LeadingZeroBitCount(std::uint8_t x)

Ditto.

> Source/WTF/wtf/LeadingZeroBitCount.h:93
> +template <typename A> inline constexpr int BitsNeededFor(A x)

Rename it to bitsNeededFor.

> Tools/TestWebKitAPI/CMakeLists.txt:57
> +    Tests/WTF/Int128.cpp
>      Tests/WTF/IntegerToStringConversion.cpp
>      Tests/WTF/IteratorRange.cpp
>      Tests/WTF/JSONValue.cpp
>      Tests/WTF/LEBDecoder.cpp
> +    Tests/WTF/LeadingZeroBitCount.cpp

Add them to xcodeproj to run these tests on macOS / iOS too.

[Alexey Proskuryakov]

If the license is not known, then we cannot accept this code in WebKit. I suspect that these projects do have licenses though.

[Philip Chimento]

The license is known (and listed in the first lines of each source file), but the copyright is not.

[Yusuke Suzuki]

(In reply to Alexey Proskuryakov from comment #5)
> If the license is not known, then we cannot accept this code in WebKit. I
> suspect that these projects do have licenses though.

It is LLVM, from where, we already have some files (B3ComputeDivisionMagic.h, old wtf/StdFilesystem.{cpp,h}, wtf/Span.h).
Let's just include https://llvm.org/LICENSE.txt file as WTF/LICENSE-LLVM.txt as https://trac.webkit.org/changeset/268020/webkit did.

[Philip Chimento]

Created attachment 438710 [details]
Patch

[Philip Chimento]

Created attachment 438712 [details]
Patch

[Radar WebKit Bug Importer]

<rdar://problem/83357495>

[EWS]

Committed r282828 (241959@main): <https://commits.webkit.org/241959@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 438712 [details].