Bug 179861 – [WTF] Use system endianess information instead of relying CPU information

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Yusuke Suzuki]

[WTF] Use system endianess information instead of relying CPU information

[Yusuke Suzuki]

Created attachment 327313 [details]
Patch

[Yusuke Suzuki]

My goal is removing bunch of minor CPUs from Platform.h.

[Yusuke Suzuki]

Created attachment 327314 [details]
Patch

[JF Bastien]

Comment on attachment 327314 [details]
Patch

Do we want something that’ll work with C++17?  http://en.cppreference.com/w/cpp/types/endian

[Yusuke Suzuki]

(In reply to JF Bastien from comment #4)
> Comment on attachment 327314 [details]
> Patch
> 
> Do we want something that’ll work with C++17? 
> http://en.cppreference.com/w/cpp/types/endian

In our current use cases, we need preprocessor defines to switch implementation by ifdefs. But C++ enum would work well if we implement things and switch by it.

template<std::endian>
void implementation();

template<>
void implementation<std::endian::little>()
{
    ...;
}

template<>
void implementation<std::endian::big>()
{
    ...;
}

And layout change is like,

template<std::endian>
struct Layout;

template<>
struct Layout<std::endian::big> {
    uint8_t f0;
    uint8_t f1;
};

template<>
struct Layout<std::endian::little> {
    uint8_t f1;
    uint8_t f0;
};

I'm not sure how to handle PDP endian (a.k.a. middle endian) in C++17.

[JF Bastien]

(In reply to Yusuke Suzuki from comment #5)
> (In reply to JF Bastien from comment #4)
> > Comment on attachment 327314 [details]
> > Patch
> > 
> > Do we want something that’ll work with C++17? 
> > http://en.cppreference.com/w/cpp/types/endian
> 
> In our current use cases, we need preprocessor defines to switch
> implementation by ifdefs. But C++ enum would work well if we implement
> things and switch by it.

Sounds like you'd rather do that separately? Sounds fine, I just figured I would ask! :)

Shame that the proposal didn't have a feature test macro so we could conditionally use it when available :(

> I'm not sure how to handle PDP endian (a.k.a. middle endian) in C++17.

Can we just static_assert that we never get such endian? I don't think WebKit supports this type of platform?

Otherwise r=me

[Yusuke Suzuki]

(In reply to JF Bastien from comment #6)
> (In reply to Yusuke Suzuki from comment #5)
> > (In reply to JF Bastien from comment #4)
> > > Comment on attachment 327314 [details]
> > > Patch
> > > 
> > > Do we want something that’ll work with C++17? 
> > > http://en.cppreference.com/w/cpp/types/endian
> > 
> > In our current use cases, we need preprocessor defines to switch
> > implementation by ifdefs. But C++ enum would work well if we implement
> > things and switch by it.
> 
> Sounds like you'd rather do that separately? Sounds fine, I just figured I
> would ask! :)
> 
> Shame that the proposal didn't have a feature test macro so we could
> conditionally use it when available :(

Yeah, I think currently using CPU(LITTLE_ENDIAN) / CPU(BIG_ENDIAN) is enough for WebKit. But once C++17 is everywhere, we can consider using std::endian instead in our code base.

> 
> > I'm not sure how to handle PDP endian (a.k.a. middle endian) in C++17.
> 
> Can we just static_assert that we never get such endian? I don't think
> WebKit supports this type of platform?
> 
> Otherwise r=me

Right, I think the current WebKit does not support middle endian well.
And I don't think there are any stake holders using middle endian CPUs.
I'll add `static_assert(!CPU(MIDDLE_ENDIAN), "");` to ensure this.

[Yusuke Suzuki]

(In reply to Yusuke Suzuki from comment #7)
> (In reply to JF Bastien from comment #6)
> > (In reply to Yusuke Suzuki from comment #5)
> > > (In reply to JF Bastien from comment #4)
> > > > Comment on attachment 327314 [details]
> > > > Patch
> > > > 
> > > > Do we want something that’ll work with C++17? 
> > > > http://en.cppreference.com/w/cpp/types/endian
> > > 
> > > In our current use cases, we need preprocessor defines to switch
> > > implementation by ifdefs. But C++ enum would work well if we implement
> > > things and switch by it.
> > 
> > Sounds like you'd rather do that separately? Sounds fine, I just figured I
> > would ask! :)
> > 
> > Shame that the proposal didn't have a feature test macro so we could
> > conditionally use it when available :(
> 
> Yeah, I think currently using CPU(LITTLE_ENDIAN) / CPU(BIG_ENDIAN) is enough
> for WebKit. But once C++17 is everywhere, we can consider using std::endian
> instead in our code base.
> 
> > 
> > > I'm not sure how to handle PDP endian (a.k.a. middle endian) in C++17.
> > 
> > Can we just static_assert that we never get such endian? I don't think
> > WebKit supports this type of platform?
> > 
> > Otherwise r=me
> 
> Right, I think the current WebKit does not support middle endian well.
> And I don't think there are any stake holders using middle endian CPUs.
> I'll add `static_assert(!CPU(MIDDLE_ENDIAN), "");` to ensure this.

Since it's macro, and we do not want to add any C++ things to Platform.h, I'll insert

#if !CPU(LITTLE_ENDIAN) && !CPU(BIG_ENDIAN)
#error Unsupported endian
#endif

[Yusuke Suzuki]

Committed r225032: <https://trac.webkit.org/changeset/225032>

[Radar WebKit Bug Importer]

<rdar://problem/35635902>