3 files changed, 70 insertions, 33 deletions

diff --git a/absl/base/internal/bits.h b/absl/base/internal/bits.h
index b0780f2d121c..450b923f4a4c 100644
--- a/absl/base/internal/bits.h
+++ b/absl/base/internal/bits.h
@@ -50,10 +50,22 @@ namespace base_internal {
 
 ABSL_BASE_INTERNAL_FORCEINLINE int CountLeadingZeros64Slow(uint64_t n) {
   int zeroes = 60;
-  if (n >> 32) zeroes -= 32, n >>= 32;
-  if (n >> 16) zeroes -= 16, n >>= 16;
-  if (n >> 8) zeroes -= 8, n >>= 8;
-  if (n >> 4) zeroes -= 4, n >>= 4;
+  if (n >> 32) {
+    zeroes -= 32;
+    n >>= 32;
+  }
+  if (n >> 16) {
+    zeroes -= 16;
+    n >>= 16;
+  }
+  if (n >> 8) {
+    zeroes -= 8;
+    n >>= 8;
+  }
+  if (n >> 4) {
+    zeroes -= 4;
+    n >>= 4;
+  }
   return "\4\3\2\2\1\1\1\1\0\0\0\0\0\0\0"[n] + zeroes;
 }
 
@@ -95,9 +107,18 @@ ABSL_BASE_INTERNAL_FORCEINLINE int CountLeadingZeros64(uint64_t n) {
 
 ABSL_BASE_INTERNAL_FORCEINLINE int CountLeadingZeros32Slow(uint64_t n) {
   int zeroes = 28;
-  if (n >> 16) zeroes -= 16, n >>= 16;
-  if (n >> 8) zeroes -= 8, n >>= 8;
-  if (n >> 4) zeroes -= 4, n >>= 4;
+  if (n >> 16) {
+    zeroes -= 16;
+    n >>= 16;
+  }
+  if (n >> 8) {
+    zeroes -= 8;
+    n >>= 8;
+  }
+  if (n >> 4) {
+    zeroes -= 4;
+    n >>= 4;
+  }
   return "\4\3\2\2\1\1\1\1\0\0\0\0\0\0\0"[n] + zeroes;
 }
 
diff --git a/absl/base/internal/periodic_sampler.cc b/absl/base/internal/periodic_sampler.cc
index 439745d0c514..69656c8a0fbe 100644
--- a/absl/base/internal/periodic_sampler.cc
+++ b/absl/base/internal/periodic_sampler.cc
@@ -36,13 +36,13 @@ bool PeriodicSamplerBase::SubtleConfirmSample() noexcept {
 
   // Check if this is the first call to Sample()
   if (ABSL_PREDICT_FALSE(stride_ == 1)) {
-    stride_ = -1 - GetExponentialBiased(current_period);
-    if (stride_ < -1) {
+    stride_ = static_cast<uint64_t>(-1 - GetExponentialBiased(current_period));
+    if (static_cast<int64_t>(stride_) < -1) {
       ++stride_;
       return false;
     }
   }
-  stride_ = -1 - GetExponentialBiased(current_period);
+  stride_ = static_cast<uint64_t>(-1 - GetExponentialBiased(current_period));
   return true;
 }
 
diff --git a/absl/base/internal/periodic_sampler.h b/absl/base/internal/periodic_sampler.h
index 2c0600f05ea4..238797424fc0 100644
--- a/absl/base/internal/periodic_sampler.h
+++ b/absl/base/internal/periodic_sampler.h
@@ -111,33 +111,49 @@ class PeriodicSamplerBase {
   // Returns the current period of this sampler. Thread-safe.
   virtual int period() const noexcept = 0;
 
-  int64_t stride_ = 0;
+  // Keep and decrement stride_ as an unsigned integer, but compare the value
+  // to zero casted as a signed int. clang and msvc do not create optimum code
+  // if we use signed for the combined decrement and sign comparison.
+  //
+  // Below 3 alternative options, all compiles generate the best code
+  // using the unsigned increment <---> signed int comparison option.
+  //
+  // Option 1:
+  //   int64_t stride_;
+  //   if (ABSL_PREDICT_TRUE(++stride_ < 0)) { ... }
+  //
+  //   GCC   x64 (OK) : https://gcc.godbolt.org/z/R5MzzA
+  //   GCC   ppc (OK) : https://gcc.godbolt.org/z/z7NZAt
+  //   Clang x64 (BAD): https://gcc.godbolt.org/z/t4gPsd
+  //   ICC   x64 (OK) : https://gcc.godbolt.org/z/rE6s8W
+  //   MSVC  x64 (OK) : https://gcc.godbolt.org/z/ARMXqS
+  //
+  // Option 2:
+  //   int64_t stride_ = 0;
+  //   if (ABSL_PREDICT_TRUE(--stride_ >= 0)) { ... }
+  //
+  //   GCC   x64 (OK) : https://gcc.godbolt.org/z/jSQxYK
+  //   GCC   ppc (OK) : https://gcc.godbolt.org/z/VJdYaA
+  //   Clang x64 (BAD): https://gcc.godbolt.org/z/Xm4NjX
+  //   ICC   x64 (OK) : https://gcc.godbolt.org/z/4snaFd
+  //   MSVC  x64 (BAD): https://gcc.godbolt.org/z/BgnEKE
+  //
+  // Option 3:
+  //   uint64_t stride_;
+  //   if (ABSL_PREDICT_TRUE(static_cast<int64_t>(++stride_) < 0)) { ... }
+  //
+  //   GCC   x64 (OK) : https://gcc.godbolt.org/z/bFbfPy
+  //   GCC   ppc (OK) : https://gcc.godbolt.org/z/S9KkUE
+  //   Clang x64 (OK) : https://gcc.godbolt.org/z/UYzRb4
+  //   ICC   x64 (OK) : https://gcc.godbolt.org/z/ptTNfD
+  //   MSVC  x64 (OK) : https://gcc.godbolt.org/z/76j4-5
+  uint64_t stride_ = 0;
   ExponentialBiased rng_;
 };
 
 inline bool PeriodicSamplerBase::SubtleMaybeSample() noexcept {
-  // We explicitly count up and not down, as the compiler does not generate
-  // ideal code for counting down. See also https://gcc.godbolt.org/z/FTPDSM
-  //
-  // With `if (ABSL_PREDICT_FALSE(++stride_ < 0))`
-  //    add     QWORD PTR fs:sampler@tpoff+8, 1
-  //    jns     .L15
-  //    ret
-  //
-  // With `if (ABSL_PREDICT_FALSE(--stride_ > 0))`
-  //    mov     rax, QWORD PTR fs:sampler@tpoff+8
-  //    sub     rax, 1
-  //    mov     QWORD PTR fs:sampler@tpoff+8, rax
-  //    test    rax, rax
-  //    jle     .L15
-  //    ret
-  //    add     QWORD PTR fs:sampler@tpoff+8, 1
-  //    jns     .L15
-  //    ret
-  //
-  // --stride >= 0 does work, but makes our logic slightly harder as in that
-  // case we have less convenient zero-init and over-run values.
-  if (ABSL_PREDICT_FALSE(++stride_ < 0)) {
+  // See comments on `stride_` for the unsigned increment / signed compare.
+  if (ABSL_PREDICT_TRUE(static_cast<int64_t>(++stride_) < 0)) {
     return false;
   }
   return true;