[PATCH] Use __builtin_is_constant_evaluated in std::less etc. (PR tree-optimization/88775)

[PATCH] Use __builtin_is_constant_evaluated in std::less etc. (PR tree-optimization/88775)

[Jakub Jelinek]

Hi!

In Marc's testcase, we generate terrible code for std::string assignment,
because the __builtin_constant_p is kept in the IL for way too long and the
optimizers (jump threading?) create way too many copies of the
memcpy/memmove calls that it is then hard to bring it back in sanitity.
On the testcase in the PR, GCC 7 emits on x86_64 with -O2 99 bytes long
function, GCC 9 unpatched 259 bytes long, with this patch it emits
139 bytes long, better but still not as good as before.  I guess we'll need
to improve GIMPLE optimizers too, but having twice as small IL for these
heavily used operators where e.g. _M_disjunct uses two of them and we wind
up with twice as many branches because of that is IMHO very useful.

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

1) I'm not really sure about proper formatting in libstdc++, I thought you
   don't use space before ( in function calls, but then why is there a space
   in __builtin_constant_p?
2) not really sure about that #if __cplusplus >= 201402L either, I think we
   don't really want to use __builtin_is_constant_evaluated at least in
   C++98 code, but even in C++11, if the operator isn't constexpr, is there
   any point trying to help it do the right thing in constexpr contexts?

2019-01-09  Jakub Jelinek  <jakub@redhat.com>

	PR tree-optimization/88775
	* include/bits/stl_function.h (greater<_Tp*>::operator(),
	less<_Tp*>::operator(), greater_equal<_Tp*>::operator(),
	less_equal<_Tp*>::operator()): Use __builtin_is_constant_evaluated
	instead of __builtin_constant_p if available.  Don't bother with
	the pointer comparison in C++11 and earlier.

--- libstdc++-v3/include/bits/stl_function.h.jj	2019-01-01 12:45:51.182541077 +0100
+++ libstdc++-v3/include/bits/stl_function.h	2019-01-09 23:15:34.824800676 +0100
@@ -413,8 +413,14 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
       _GLIBCXX14_CONSTEXPR bool
       operator()(_Tp* __x, _Tp* __y) const _GLIBCXX_NOTHROW
       {
+#if __cplusplus >= 201402L
+#ifdef _GLIBCXX_HAVE_BUILTIN_IS_CONSTANT_EVALUATED
+	if (__builtin_is_constant_evaluated())
+#else
 	if (__builtin_constant_p (__x > __y))
+#endif
 	  return __x > __y;
+#endif
 	return (__UINTPTR_TYPE__)__x > (__UINTPTR_TYPE__)__y;
       }
     };
@@ -426,8 +432,14 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
       _GLIBCXX14_CONSTEXPR bool
       operator()(_Tp* __x, _Tp* __y) const _GLIBCXX_NOTHROW
       {
+#if __cplusplus >= 201402L
+#ifdef _GLIBCXX_HAVE_BUILTIN_IS_CONSTANT_EVALUATED
+	if (__builtin_is_constant_evaluated())
+#else
 	if (__builtin_constant_p (__x < __y))
+#endif
 	  return __x < __y;
+#endif
 	return (__UINTPTR_TYPE__)__x < (__UINTPTR_TYPE__)__y;
       }
     };
@@ -439,8 +451,14 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
       _GLIBCXX14_CONSTEXPR bool
       operator()(_Tp* __x, _Tp* __y) const _GLIBCXX_NOTHROW
       {
+#if __cplusplus >= 201402L
+#ifdef _GLIBCXX_HAVE_BUILTIN_IS_CONSTANT_EVALUATED
+	if (__builtin_is_constant_evaluated())
+#else
 	if (__builtin_constant_p (__x >= __y))
+#endif
 	  return __x >= __y;
+#endif
 	return (__UINTPTR_TYPE__)__x >= (__UINTPTR_TYPE__)__y;
       }
     };
@@ -452,8 +470,14 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
       _GLIBCXX14_CONSTEXPR bool
       operator()(_Tp* __x, _Tp* __y) const _GLIBCXX_NOTHROW
       {
+#if __cplusplus >= 201402L
+#ifdef _GLIBCXX_HAVE_BUILTIN_IS_CONSTANT_EVALUATED
+	if (__builtin_is_constant_evaluated())
+#else
 	if (__builtin_constant_p (__x <= __y))
+#endif
 	  return __x <= __y;
+#endif
 	return (__UINTPTR_TYPE__)__x <= (__UINTPTR_TYPE__)__y;
       }
     };

	Jakub

Re: [PATCH] Use __builtin_is_constant_evaluated in std::less etc. (PR tree-optimization/88775)

[Jonathan Wakely]

On 10/01/19 10:02 +0100, Jakub Jelinek wrote:
>Hi!
>
>In Marc's testcase, we generate terrible code for std::string assignment,
>because the __builtin_constant_p is kept in the IL for way too long and the
>optimizers (jump threading?) create way too many copies of the
>memcpy/memmove calls that it is then hard to bring it back in sanitity.
>On the testcase in the PR, GCC 7 emits on x86_64 with -O2 99 bytes long
>function, GCC 9 unpatched 259 bytes long, with this patch it emits
>139 bytes long, better but still not as good as before.  I guess we'll need
>to improve GIMPLE optimizers too, but having twice as small IL for these
>heavily used operators where e.g. _M_disjunct uses two of them and we wind
>up with twice as many branches because of that is IMHO very useful.
>
>Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
>
>1) I'm not really sure about proper formatting in libstdc++, I thought you
>   don't use space before ( in function calls, but then why is there a space
>   in __builtin_constant_p?

I guess I probably copy&pasted it from something you gave me :-)

The space shouldn't be there.

>2) not really sure about that #if __cplusplus >= 201402L either, I think we
>   don't really want to use __builtin_is_constant_evaluated at least in
>   C++98 code, but even in C++11, if the operator isn't constexpr, is there
>   any point trying to help it do the right thing in constexpr contexts?

I think there's no point, so only doing it for C++14 and later looks
OK to me.

OK for trunk, thanks.

Re: [PATCH] Use __builtin_is_constant_evaluated in std::less etc. (PR tree-optimization/88775)

[Richard Biener]

On Thu, Jan 10, 2019 at 10:02 AM Jakub Jelinek <jakub@redhat.com> wrote:
>
> Hi!
>
> In Marc's testcase, we generate terrible code for std::string assignment,
> because the __builtin_constant_p is kept in the IL for way too long and the
> optimizers (jump threading?) create way too many copies of the
> memcpy/memmove calls that it is then hard to bring it back in sanitity.
> On the testcase in the PR, GCC 7 emits on x86_64 with -O2 99 bytes long
> function, GCC 9 unpatched 259 bytes long, with this patch it emits
> 139 bytes long, better but still not as good as before.  I guess we'll need
> to improve GIMPLE optimizers too, but having twice as small IL for these
> heavily used operators where e.g. _M_disjunct uses two of them and we wind
> up with twice as many branches because of that is IMHO very useful.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
>
> 1) I'm not really sure about proper formatting in libstdc++, I thought you
>    don't use space before ( in function calls, but then why is there a space
>    in __builtin_constant_p?
> 2) not really sure about that #if __cplusplus >= 201402L either, I think we
>    don't really want to use __builtin_is_constant_evaluated at least in
>    C++98 code, but even in C++11, if the operator isn't constexpr, is there
>    any point trying to help it do the right thing in constexpr contexts?
>
> 2019-01-09  Jakub Jelinek  <jakub@redhat.com>
>
>         PR tree-optimization/88775
>         * include/bits/stl_function.h (greater<_Tp*>::operator(),
>         less<_Tp*>::operator(), greater_equal<_Tp*>::operator(),
>         less_equal<_Tp*>::operator()): Use __builtin_is_constant_evaluated
>         instead of __builtin_constant_p if available.  Don't bother with
>         the pointer comparison in C++11 and earlier.
>
> --- libstdc++-v3/include/bits/stl_function.h.jj 2019-01-01 12:45:51.182541077 +0100
> +++ libstdc++-v3/include/bits/stl_function.h    2019-01-09 23:15:34.824800676 +0100
> @@ -413,8 +413,14 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>        _GLIBCXX14_CONSTEXPR bool
>        operator()(_Tp* __x, _Tp* __y) const _GLIBCXX_NOTHROW
>        {
> +#if __cplusplus >= 201402L
> +#ifdef _GLIBCXX_HAVE_BUILTIN_IS_CONSTANT_EVALUATED
> +       if (__builtin_is_constant_evaluated())
> +#else
>         if (__builtin_constant_p (__x > __y))
> +#endif
>           return __x > __y;
> +#endif
>         return (__UINTPTR_TYPE__)__x > (__UINTPTR_TYPE__)__y;

I wonder what the idea behind this is.  It smells like trying to avoid
undefined behavior (relational compare of pointers to different objects?)
but then executing that nevertheless when "constant"?

I think this just doesn't work since the compiler, when evaluating
__x > __y [for constant folding] is exploiting the fact that doing
non-equality compares on pointers into different objects invoke
undefined behavior.

So why is this not just

  return (__UINTPTR_TYPE__)__x > (__UINTPTR_TYPE__)__y;

or with the casts elided?  Does the C++ standard say pointers are
to be compared unsigned here?  Or do all targets GCC support
lay out the address space in a way that this is correct for pointers
into distinct objects?

Richard.


>        }
>      };
> @@ -426,8 +432,14 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>        _GLIBCXX14_CONSTEXPR bool
>        operator()(_Tp* __x, _Tp* __y) const _GLIBCXX_NOTHROW
>        {
> +#if __cplusplus >= 201402L
> +#ifdef _GLIBCXX_HAVE_BUILTIN_IS_CONSTANT_EVALUATED
> +       if (__builtin_is_constant_evaluated())
> +#else
>         if (__builtin_constant_p (__x < __y))
> +#endif
>           return __x < __y;
> +#endif
>         return (__UINTPTR_TYPE__)__x < (__UINTPTR_TYPE__)__y;
>        }
>      };
> @@ -439,8 +451,14 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>        _GLIBCXX14_CONSTEXPR bool
>        operator()(_Tp* __x, _Tp* __y) const _GLIBCXX_NOTHROW
>        {
> +#if __cplusplus >= 201402L
> +#ifdef _GLIBCXX_HAVE_BUILTIN_IS_CONSTANT_EVALUATED
> +       if (__builtin_is_constant_evaluated())
> +#else
>         if (__builtin_constant_p (__x >= __y))
> +#endif
>           return __x >= __y;
> +#endif
>         return (__UINTPTR_TYPE__)__x >= (__UINTPTR_TYPE__)__y;
>        }
>      };
> @@ -452,8 +470,14 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>        _GLIBCXX14_CONSTEXPR bool
>        operator()(_Tp* __x, _Tp* __y) const _GLIBCXX_NOTHROW
>        {
> +#if __cplusplus >= 201402L
> +#ifdef _GLIBCXX_HAVE_BUILTIN_IS_CONSTANT_EVALUATED
> +       if (__builtin_is_constant_evaluated())
> +#else
>         if (__builtin_constant_p (__x <= __y))
> +#endif
>           return __x <= __y;
> +#endif
>         return (__UINTPTR_TYPE__)__x <= (__UINTPTR_TYPE__)__y;
>        }
>      };
>
>         Jakub

Re: [PATCH] Use __builtin_is_constant_evaluated in std::less etc. (PR tree-optimization/88775)

[Ville Voutilainen]

On Mon, 14 Jan 2019 at 10:29, Richard Biener <richard.guenther@gmail.com> wrote:
> >        _GLIBCXX14_CONSTEXPR bool
> >        operator()(_Tp* __x, _Tp* __y) const _GLIBCXX_NOTHROW
> >        {
> > +#if __cplusplus >= 201402L
> > +#ifdef _GLIBCXX_HAVE_BUILTIN_IS_CONSTANT_EVALUATED
> > +       if (__builtin_is_constant_evaluated())
> > +#else
> >         if (__builtin_constant_p (__x > __y))
> > +#endif
> >           return __x > __y;
> > +#endif
> >         return (__UINTPTR_TYPE__)__x > (__UINTPTR_TYPE__)__y;
>
> I wonder what the idea behind this is.  It smells like trying to avoid
> undefined behavior (relational compare of pointers to different objects?)
> but then executing that nevertheless when "constant"?
>
> I think this just doesn't work since the compiler, when evaluating
> __x > __y [for constant folding] is exploiting the fact that doing
> non-equality compares on pointers into different objects invoke
> undefined behavior.

When that happens, the function is ill-formed when constant-evaluated,
which is fine.
When the comparison is not UB, it should constant-evaluate without problems.

> So why is this not just
>   return (__UINTPTR_TYPE__)__x > (__UINTPTR_TYPE__)__y;
> or with the casts elided?

Those casts are reinterpret_casts, so the function could never be
constant-evaluated.
The casts need to be there to avoid UB for the run-time cases.

Re: [PATCH] Use __builtin_is_constant_evaluated in std::less etc. (PR tree-optimization/88775)

[Jakub Jelinek]

On Mon, Jan 14, 2019 at 09:29:03AM +0100, Richard Biener wrote:
> So why is this not just
> 
>   return (__UINTPTR_TYPE__)__x > (__UINTPTR_TYPE__)__y;
> 
> or with the casts elided?  Does the C++ standard say pointers are
> to be compared unsigned here?  Or do all targets GCC support
> lay out the address space in a way that this is correct for pointers
> into distinct objects?

See PR78420 for details on why it is done that way.

	Jakub

Re: [PATCH] Use __builtin_is_constant_evaluated in std::less etc. (PR tree-optimization/88775)

[Richard Biener]

On Mon, Jan 14, 2019 at 9:42 AM Jakub Jelinek <jakub@redhat.com> wrote:
>
> On Mon, Jan 14, 2019 at 09:29:03AM +0100, Richard Biener wrote:
> > So why is this not just
> >
> >   return (__UINTPTR_TYPE__)__x > (__UINTPTR_TYPE__)__y;
> >
> > or with the casts elided?  Does the C++ standard say pointers are
> > to be compared unsigned here?  Or do all targets GCC support
> > lay out the address space in a way that this is correct for pointers
> > into distinct objects?
>
> See PR78420 for details on why it is done that way.

I see.  So the __builtin_is_constant_evaluated thing makes it
"correct" (but then eventually exposing the non-total order issue again).

And if I read the PR correctly we'd really like to be able to write

 if (__builtin_constant_p (<expr>, &result))
   return result;

to make sure whatever undesired-in-the-IL things of <expr>
do not leak there.

Btw, wouldn't sth like

  if (__builtin_is_constant_evaluated())
    {
       union U { __UINTPTR_TYPE__ u; _Tp *p } __ux, __uy;
       __ux.p = __x;
       __uy.p = __y;
       return __ux.u < __uy.u;
    }

be more correct and consistent?  Well, or any other way of
evading that reinterpret-cast "issue"?

Richard.


>
>         Jakub

Re: [PATCH] Use __builtin_is_constant_evaluated in std::less etc. (PR tree-optimization/88775)

[Jonathan Wakely]

On Mon, 14 Jan 2019 at 09:17, Richard Biener <richard.guenther@gmail.com> wrote:
>
> On Mon, Jan 14, 2019 at 9:42 AM Jakub Jelinek <jakub@redhat.com> wrote:
> >
> > On Mon, Jan 14, 2019 at 09:29:03AM +0100, Richard Biener wrote:
> > > So why is this not just
> > >
> > >   return (__UINTPTR_TYPE__)__x > (__UINTPTR_TYPE__)__y;
> > >
> > > or with the casts elided?  Does the C++ standard say pointers are
> > > to be compared unsigned here?  Or do all targets GCC support
> > > lay out the address space in a way that this is correct for pointers
> > > into distinct objects?
> >
> > See PR78420 for details on why it is done that way.
>
> I see.  So the __builtin_is_constant_evaluated thing makes it
> "correct" (but then eventually exposing the non-total order issue again).

No, because comparing unrelated pointers isn't allowed in constexpr
contexts, so it just gets rejected at compile time.


> And if I read the PR correctly we'd really like to be able to write
>
>  if (__builtin_constant_p (<expr>, &result))
>    return result;
>
> to make sure whatever undesired-in-the-IL things of <expr>
> do not leak there.
>
> Btw, wouldn't sth like
>
>   if (__builtin_is_constant_evaluated())
>     {
>        union U { __UINTPTR_TYPE__ u; _Tp *p } __ux, __uy;
>        __ux.p = __x;
>        __uy.p = __y;
>        return __ux.u < __uy.u;
>     }
>
> be more correct and consistent?  Well, or any other way of
> evading that reinterpret-cast "issue"?
>
> Richard.
>
>
> >
> >         Jakub

Re: [PATCH] Use __builtin_is_constant_evaluated in std::less etc. (PR tree-optimization/88775)

[Richard Biener]

On Mon, Jan 14, 2019 at 10:21 AM Jonathan Wakely <jwakely.gcc@gmail.com> wrote:
>
> On Mon, 14 Jan 2019 at 09:17, Richard Biener <richard.guenther@gmail.com> wrote:
> >
> > On Mon, Jan 14, 2019 at 9:42 AM Jakub Jelinek <jakub@redhat.com> wrote:
> > >
> > > On Mon, Jan 14, 2019 at 09:29:03AM +0100, Richard Biener wrote:
> > > > So why is this not just
> > > >
> > > >   return (__UINTPTR_TYPE__)__x > (__UINTPTR_TYPE__)__y;
> > > >
> > > > or with the casts elided?  Does the C++ standard say pointers are
> > > > to be compared unsigned here?  Or do all targets GCC support
> > > > lay out the address space in a way that this is correct for pointers
> > > > into distinct objects?
> > >
> > > See PR78420 for details on why it is done that way.
> >
> > I see.  So the __builtin_is_constant_evaluated thing makes it
> > "correct" (but then eventually exposing the non-total order issue again).
>
> No, because comparing unrelated pointers isn't allowed in constexpr
> contexts, so it just gets rejected at compile time.

I see.

>
> > And if I read the PR correctly we'd really like to be able to write
> >
> >  if (__builtin_constant_p (<expr>, &result))
> >    return result;
> >
> > to make sure whatever undesired-in-the-IL things of <expr>
> > do not leak there.
> >
> > Btw, wouldn't sth like
> >
> >   if (__builtin_is_constant_evaluated())
> >     {
> >        union U { __UINTPTR_TYPE__ u; _Tp *p } __ux, __uy;
> >        __ux.p = __x;
> >        __uy.p = __y;
> >        return __ux.u < __uy.u;
> >     }
> >
> > be more correct and consistent?  Well, or any other way of
> > evading that reinterpret-cast "issue"?
> >
> > Richard.
> >
> >
> > >
> > >         Jakub

Re: [PATCH] Use __builtin_is_constant_evaluated in std::less etc. (PR tree-optimization/88775)

[Jonathan Wakely]

On Mon, 14 Jan 2019 at 09:17, Richard Biener <richard.guenther@gmail.com> wrote:
>
> On Mon, Jan 14, 2019 at 9:42 AM Jakub Jelinek <jakub@redhat.com> wrote:
> >
> > On Mon, Jan 14, 2019 at 09:29:03AM +0100, Richard Biener wrote:
> > > So why is this not just
> > >
> > >   return (__UINTPTR_TYPE__)__x > (__UINTPTR_TYPE__)__y;
> > >
> > > or with the casts elided?  Does the C++ standard say pointers are
> > > to be compared unsigned here?  Or do all targets GCC support
> > > lay out the address space in a way that this is correct for pointers
> > > into distinct objects?
> >
> > See PR78420 for details on why it is done that way.
>
> I see.  So the __builtin_is_constant_evaluated thing makes it
> "correct" (but then eventually exposing the non-total order issue again).
>
> And if I read the PR correctly we'd really like to be able to write
>
>  if (__builtin_constant_p (<expr>, &result))
>    return result;
>
> to make sure whatever undesired-in-the-IL things of <expr>
> do not leak there.
>
> Btw, wouldn't sth like
>
>   if (__builtin_is_constant_evaluated())
>     {
>        union U { __UINTPTR_TYPE__ u; _Tp *p } __ux, __uy;
>        __ux.p = __x;
>        __uy.p = __y;
>        return __ux.u < __uy.u;
>     }
>
> be more correct and consistent?  Well, or any other way of
> evading that reinterpret-cast "issue"?

Also no, because you can't change the active member of a union in
constexpr (and can't type pun using unions either).

Re: [PATCH] Use __builtin_is_constant_evaluated in std::less etc. (PR tree-optimization/88775)

[Jonathan Wakely]

On Mon, 14 Jan 2019 at 08:29, Richard Biener <richard.guenther@gmail.com> wrote:
>
> On Thu, Jan 10, 2019 at 10:02 AM Jakub Jelinek <jakub@redhat.com> wrote:
> >
> > Hi!
> >
> > In Marc's testcase, we generate terrible code for std::string assignment,
> > because the __builtin_constant_p is kept in the IL for way too long and the
> > optimizers (jump threading?) create way too many copies of the
> > memcpy/memmove calls that it is then hard to bring it back in sanitity.
> > On the testcase in the PR, GCC 7 emits on x86_64 with -O2 99 bytes long
> > function, GCC 9 unpatched 259 bytes long, with this patch it emits
> > 139 bytes long, better but still not as good as before.  I guess we'll need
> > to improve GIMPLE optimizers too, but having twice as small IL for these
> > heavily used operators where e.g. _M_disjunct uses two of them and we wind
> > up with twice as many branches because of that is IMHO very useful.
> >
> > Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
> >
> > 1) I'm not really sure about proper formatting in libstdc++, I thought you
> >    don't use space before ( in function calls, but then why is there a space
> >    in __builtin_constant_p?
> > 2) not really sure about that #if __cplusplus >= 201402L either, I think we
> >    don't really want to use __builtin_is_constant_evaluated at least in
> >    C++98 code, but even in C++11, if the operator isn't constexpr, is there
> >    any point trying to help it do the right thing in constexpr contexts?
> >
> > 2019-01-09  Jakub Jelinek  <jakub@redhat.com>
> >
> >         PR tree-optimization/88775
> >         * include/bits/stl_function.h (greater<_Tp*>::operator(),
> >         less<_Tp*>::operator(), greater_equal<_Tp*>::operator(),
> >         less_equal<_Tp*>::operator()): Use __builtin_is_constant_evaluated
> >         instead of __builtin_constant_p if available.  Don't bother with
> >         the pointer comparison in C++11 and earlier.
> >
> > --- libstdc++-v3/include/bits/stl_function.h.jj 2019-01-01 12:45:51.182541077 +0100
> > +++ libstdc++-v3/include/bits/stl_function.h    2019-01-09 23:15:34.824800676 +0100
> > @@ -413,8 +413,14 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
> >        _GLIBCXX14_CONSTEXPR bool
> >        operator()(_Tp* __x, _Tp* __y) const _GLIBCXX_NOTHROW
> >        {
> > +#if __cplusplus >= 201402L
> > +#ifdef _GLIBCXX_HAVE_BUILTIN_IS_CONSTANT_EVALUATED
> > +       if (__builtin_is_constant_evaluated())
> > +#else
> >         if (__builtin_constant_p (__x > __y))
> > +#endif
> >           return __x > __y;
> > +#endif
> >         return (__UINTPTR_TYPE__)__x > (__UINTPTR_TYPE__)__y;
>
> I wonder what the idea behind this is.  It smells like trying to avoid
> undefined behavior (relational compare of pointers to different objects?)

That's not undefined in C++, it just gives an unspecified result (so
it's not specified whether x < y or y < x, or possibly even x == y,
e.g. for segmented memory).

The std::greater, std::less etc. function objects are required to give
a total order across all pointers, different objects or not, i.e.
while < might give any unspecified result, std::less has tighter
restrictions.

For that to work in general, we need the casts, or GCC's optimizers
give the wrong result. But within constexpr those functions only need
to be valid for related objects (as in C) and so we just use < there,
and rely on the compiler to reject comparisons to different objects
(because that's not allowed in constexpr).

So we can't just use < everywhere, because the optimizers don't like
it, and we can't use the cast everywhere, because that's not allowed
in constexpr.