Re: [PATCH] libstdc++: More efficient std::chrono::year::leap.

[Cassio Neri <cassio.neri@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 1806 bytes --]

I've checked the generated code and the compiler doesn't figure out
the logic. I added a comment to explain.

(Revised patch below and attached.)

Best wishes,
Cassio.

---

Simple change to std::chrono::year::is_leap. If a year is multiple of 100,
then it's divisible by 400 if and only if it's divisible by 16. The latter
allows for better code generation.

Tested on x86_64-pc-linux-gnu.

libstdc++-v3/ChangeLog:
libstdc++-v3/ChangeLog:

    * include/std/chrono:

diff --git a/libstdc++-v3/include/std/chrono b/libstdc++-v3/include/std/chrono
index 4631a727d73..85aa0379432 100644
--- a/libstdc++-v3/include/std/chrono
+++ b/libstdc++-v3/include/std/chrono
@@ -1612,7 +1612,9 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
     constexpr uint32_t __offset       = __max_dividend / 2 / 100 * 100;
     const bool __is_multiple_of_100
       = __multiplier * (_M_y + __offset) < __bound;
-    return (!__is_multiple_of_100 || _M_y % 400 == 0) && _M_y % 4 == 0;
+    // Usually we test _M_y % 400 == 0 but, when it's already known that
+    // _M_y%100 == 0, then _M_y % 400==0 is equivalent to _M_y % 16 == 0.
+    return (!__is_multiple_of_100 || _M_y % 16 == 0) && _M_y % 4 == 0;
       }

       explicit constexpr

On Fri, May 21, 2021 at 7:05 PM Koning, Paul <Paul.Koning@dell.com> wrote:
>
>
>
> > On May 21, 2021, at 1:46 PM, Cassio Neri via Gcc-patches <gcc-patches@gcc.gnu.org> wrote:
> >
> > Simple change to std::chrono::year::is_leap. If a year is multiple of 100,
> > then it's divisible by 400 if and only if it's divisible by 16. The latter
> > allows for better code generation.
>
> I wonder if the optimizer could be taught to do that.
>
> The change seems correct but it is very confusing; at the very least the reasoning you gave should be stated in a comment on that check.
>
>         paul
>
>

[-- Attachment #2: leap.patch --]
[-- Type: text/x-patch, Size: 1011 bytes --]

Simple change to std::chrono::year::is_leap. If a year is multiple of 100,
then it's divisible by 400 if and only if it's divisible by 16. The latter
allows for better code generation.

Tested on x86_64-pc-linux-gnu.

libstdc++-v3/ChangeLog:
libstdc++-v3/ChangeLog:

	* include/std/chrono:

diff --git a/libstdc++-v3/include/std/chrono b/libstdc++-v3/include/std/chrono
index 4631a727d73..85aa0379432 100644
--- a/libstdc++-v3/include/std/chrono
+++ b/libstdc++-v3/include/std/chrono
@@ -1612,7 +1612,9 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 	constexpr uint32_t __offset       = __max_dividend / 2 / 100 * 100;
 	const bool __is_multiple_of_100
 	  = __multiplier * (_M_y + __offset) < __bound;
-	return (!__is_multiple_of_100 || _M_y % 400 == 0) && _M_y % 4 == 0;
+	// Usually we test _M_y % 400 == 0 but, when it's already known that
+	// _M_y%100 == 0, then _M_y % 400==0 is equivalent to _M_y % 16 == 0.
+	return (!__is_multiple_of_100 || _M_y % 16 == 0) && _M_y % 4 == 0;
       }

       explicit constexpr