From bccb2f43c9b19684cff4f0a37a49db19acb33df3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marek=20Lema=C5=84czyk?= Date: Mon, 13 Oct 2025 19:56:47 +0200 Subject: [PATCH] Document unreachable guards in GPU by-divisor pipeline --- ...MersenneNumberDivisorByDivisorCpuTester.cs | 97 ++++++++++-------- ...MersenneNumberDivisorByDivisorGpuTester.cs | 98 +++++++++++-------- ...senneNumberDivisorCandidateGpuEvaluator.cs | 26 +++-- ...rsenneNumberDivisorMontgomeryGpuBuilder.cs | 17 +++- ...ersenneNumberDivisorRemainderGpuStepper.cs | 31 +++--- ...ersenneNumberDivisorResidueGpuEvaluator.cs | 19 ++-- .../MersenneNumberDivisorByDivisorTester.cs | 12 ++- 7 files changed, 178 insertions(+), 122 deletions(-) diff --git a/PerfectNumbers.Core/Cpu/MersenneNumberDivisorByDivisorCpuTester.cs b/PerfectNumbers.Core/Cpu/MersenneNumberDivisorByDivisorCpuTester.cs index fed7e76..b74761f 100644 --- a/PerfectNumbers.Core/Cpu/MersenneNumberDivisorByDivisorCpuTester.cs +++ b/PerfectNumbers.Core/Cpu/MersenneNumberDivisorByDivisorCpuTester.cs @@ -146,10 +146,11 @@ public sealed class MersenneNumberDivisorByDivisorCpuTester : IMersenneNumberDiv public void PrepareCandidates(ReadOnlySpan primes, Span allowedMaxValues) { - if (allowedMaxValues.Length < primes.Length) - { - throw new ArgumentException("allowedMaxValues span must be at least as long as primes span.", nameof(allowedMaxValues)); - } + // EvenPerfectBitScanner always provides equally sized spans here, so the historical guard never triggers. + // if (allowedMaxValues.Length < primes.Length) + // { + // throw new ArgumentException("allowedMaxValues span must be at least as long as primes span.", nameof(allowedMaxValues)); + // } ulong divisorLimit; @@ -236,11 +237,12 @@ public sealed class MersenneNumberDivisorByDivisorCpuTester : IMersenneNumberDiv } UInt128 step = (UInt128)prime << 1; - if (step == UInt128.Zero) - { - processedAll = true; - return false; - } + // EvenPerfectBitScanner only supplies primes greater than one here, so the shifted step never becomes zero. + // if (step == UInt128.Zero) + // { + // processedAll = true; + // return false; + // } UInt128 limit = allowedMax; UInt128 divisor = step + UInt128.One; @@ -321,10 +323,11 @@ public sealed class MersenneNumberDivisorByDivisorCpuTester : IMersenneNumberDiv return true; } - if (divisorCycle == 0UL) - { - Console.WriteLine($"Divisor cycle was not calculated for {prime}"); - } + // DivisorCycleCache guarantees a non-zero cycle for every divisor reached on the EvenPerfectBitScanner path. + // if (divisorCycle == 0UL) + // { + // Console.WriteLine($"Divisor cycle was not calculated for {prime}"); + // } } divisor += step; @@ -369,11 +372,13 @@ public sealed class MersenneNumberDivisorByDivisorCpuTester : IMersenneNumberDiv GpuUInt128 step128 = new GpuUInt128(prime); step128.ShiftLeft(1); - if (step128.IsZero) - { - processedAll = true; - return false; - } + // Primes above one yield a non-zero 2p value even after widening to GpuUInt128, so the defensive + // zero guard remains commented out to document the invariant without branching. + // if (step128.IsZero) + // { + // processedAll = true; + // return false; + // } GpuUInt128 limit128 = new GpuUInt128(allowedMax); GpuUInt128 divisor128 = step128; @@ -449,10 +454,12 @@ public sealed class MersenneNumberDivisorByDivisorCpuTester : IMersenneNumberDiv int chunkCount = remainingCandidates > (ulong)batchCapacity ? batchCapacity : (int)remainingCandidates; - if (chunkCount <= 0) - { - break; - } + // batchCapacity and remainingCandidates are strictly positive in this path, so the zero-count + // shortcut never executes on production scans. + // if (chunkCount <= 0) + // { + // break; + // } Span candidateSpan = candidateBuffer.AsSpan(0, chunkCount); Span maskSpan = maskBuffer.AsSpan(0, chunkCount); @@ -604,11 +611,12 @@ public sealed class MersenneNumberDivisorByDivisorCpuTester : IMersenneNumberDiv } ulong interval = PerfectNumberConstants.ConsoleInterval; - if (interval == 0UL) - { - _lastStatusDivisor = 0UL; - return; - } + // PerfectNumberConstants.ConsoleInterval is a positive constant, so this guard never triggers on EvenPerfectBitScanner. + // if (interval == 0UL) + // { + // _lastStatusDivisor = 0UL; + // return; + // } ulong total = _lastStatusDivisor + processedCount; // TODO: Replace this modulo with the ring-buffer style counter (subtract loop) used in the fast CLI @@ -696,11 +704,14 @@ public sealed class MersenneNumberDivisorByDivisorCpuTester : IMersenneNumberDiv } ulong modulus = effectiveDivisorData.Modulus; - if (modulus <= 1UL || (modulus & 1UL) == 0UL) - { - hits.Clear(); - return; - } + // EvenPerfectBitScanner only schedules odd divisors greater than one, and the fallback + // above rebuilds Montgomery data when the modulus mismatches, so this guard never + // triggers for production scans. + // if (modulus <= 1UL || (modulus & 1UL) == 0UL) + // { + // hits.Clear(); + // return; + // } if (divisorCycle == 0UL) { @@ -803,10 +814,12 @@ public sealed class MersenneNumberDivisorByDivisorCpuTester : IMersenneNumberDiv private void EnsureCapacity(int requiredLength) { - if (requiredLength <= 0) - { - return; - } + // Prime batches are never empty on the by-divisor scanner path, so the zero-length guard stays commented + // out to remove the redundant branch. + // if (requiredLength <= 0) + // { + // return; + // } // Sessions release pooled buffers between scans; a single null check covers every array because buffers are rented and returned together. if (requiredLength <= _bufferCapacity && _primeDeltas is not null) @@ -873,11 +886,13 @@ public sealed class MersenneNumberDivisorByDivisorCpuTester : IMersenneNumberDiv private static void ComputeCycleRemainders(ulong cycleLength, ReadOnlySpan primes, ReadOnlySpan deltas, Span remainders) { - if (cycleLength == 0UL || primes.Length == 0) - { - remainders.Clear(); - return; - } + // Divisor cycles are validated before reaching the GPU helper, and prime batches are non-empty, so + // the defensive zero checks remain commented out here. + // if (cycleLength == 0UL || primes.Length == 0) + // { + // remainders.Clear(); + // return; + // } ulong remainder = primes[0] % cycleLength; remainders[0] = remainder; diff --git a/PerfectNumbers.Core/Gpu/MersenneNumberDivisorByDivisorGpuTester.cs b/PerfectNumbers.Core/Gpu/MersenneNumberDivisorByDivisorGpuTester.cs index 9d10387..2974517 100644 --- a/PerfectNumbers.Core/Gpu/MersenneNumberDivisorByDivisorGpuTester.cs +++ b/PerfectNumbers.Core/Gpu/MersenneNumberDivisorByDivisorGpuTester.cs @@ -87,11 +87,13 @@ public sealed class MersenneNumberDivisorByDivisorGpuTester : IMersenneNumberDiv batchCapacity = _gpuBatchSize; } - if (allowedMax < 3UL) - { - divisorsExhausted = true; - return true; - } + // The GPU path always receives an allowed maximum well above the minimal divisor, so the legacy + // guard stays commented out to document the invariant without branching. + // if (allowedMax < 3UL) + // { + // divisorsExhausted = true; + // return true; + // } bool composite; bool coveredRange; @@ -222,7 +224,8 @@ public sealed class MersenneNumberDivisorByDivisorGpuTester : IMersenneNumberDiv ArrayView1D exponentView; ArrayView1D hitsView; - cycleCapacity = Math.Max(1, cycleCapacity); + // Batch resources expose a positive cycle capacity, so skip the redundant clamp here. + // cycleCapacity = Math.Max(1, cycleCapacity); int chunkCapacity = Math.Min(batchCapacity, cycleCapacity); int maxThreadsPerGroup = (int)accelerator.MaxNumThreadsPerGroup; @@ -231,20 +234,22 @@ public sealed class MersenneNumberDivisorByDivisorGpuTester : IMersenneNumberDiv chunkCapacity = Math.Min(chunkCapacity, maxThreadsPerGroup); } - if (chunkCapacity <= 0) - { - chunkCapacity = cycleCapacity; - } + // cycleCapacity already bounds chunkCapacity above zero, so the fallback would never execute. + // if (chunkCapacity <= 0) + // { + // chunkCapacity = cycleCapacity; + // } ulong[] filteredDivisors = ArrayPool.Shared.Rent(chunkCapacity); ulong[] divisorGaps = ArrayPool.Shared.Rent(chunkCapacity); UInt128 twoP128 = (UInt128)prime << 1; - if (twoP128 == UInt128.Zero) - { - coveredRange = true; - return false; - } + // Primes above one yield a non-zero 2p value, so the legacy zero guard remains commented out. + // if (twoP128 == UInt128.Zero) + // { + // coveredRange = true; + // return false; + // } UInt128 allowedMax128 = allowedMax; UInt128 firstDivisor128 = twoP128 + UInt128.One; @@ -255,11 +260,13 @@ public sealed class MersenneNumberDivisorByDivisorGpuTester : IMersenneNumberDiv } UInt128 maxK128 = (allowedMax128 - UInt128.One) / twoP128; - if (maxK128 == UInt128.Zero) - { - coveredRange = true; - return false; - } + // Once the first divisor falls within the allowed range, at least one multiplier k is admissible, + // so keep the defensive zero check commented out. + // if (maxK128 == UInt128.Zero) + // { + // coveredRange = true; + // return false; + // } ulong maxK = maxK128 > ulong.MaxValue ? ulong.MaxValue : (ulong)maxK128; ulong currentK = 1UL; @@ -301,10 +308,12 @@ public sealed class MersenneNumberDivisorByDivisorGpuTester : IMersenneNumberDiv chunkCount = (int)remainingK; } - if (chunkCount <= 0) - { - break; - } + // chunkCapacity and the remaining multiplier range are strictly positive here, so the zero + // chunk fallback never triggers on production scans. + // if (chunkCount <= 0) + // { + // break; + // } Span candidateSpan = cycleCandidates.AsSpan(0, chunkCount); Span gapSpan = divisorGaps.AsSpan(0, chunkCount); @@ -465,10 +474,12 @@ public sealed class MersenneNumberDivisorByDivisorGpuTester : IMersenneNumberDiv Action, ArrayView, byte> deltaKernel, Action, ArrayView, byte, byte> scanKernel) { - if (count <= 0) - { - return; - } + // GPU batches always contain at least one divisor gap, so the empty-count guard remains commented + // to eliminate redundant branching. + // if (count <= 0) + // { + // return; + // } deltaKernel(count, gapView, deltaView, modulus); SharedMemoryConfig sharedMemoryConfig = SharedMemoryConfig.RequestDynamic(count); @@ -491,11 +502,13 @@ public sealed class MersenneNumberDivisorByDivisorGpuTester : IMersenneNumberDiv Span destination, Action, ArrayView, ArrayView, ArrayView, byte, ArrayView> maskKernel) { - if (count <= 0) - { - destination.Clear(); - return; - } + // Candidate batches always provide at least one entry, so the zero-count guard stays commented out + // to avoid clearing spans unnecessarily. + // if (count <= 0) + // { + // destination.Clear(); + // return; + // } maskKernel(count, remainder10View, remainder8View, remainder5View, remainder3View, lastIsSevenFlag, maskView); maskView.CopyToCPU(ref MemoryMarshal.GetReference(destination), count); @@ -503,10 +516,12 @@ public sealed class MersenneNumberDivisorByDivisorGpuTester : IMersenneNumberDiv private static byte FetchLastRemainder(ArrayView1D remainderView, int count) { - if (count <= 0) - { - return 0; - } + // Kernels always process at least one remainder in this path, so the empty-span guard remains + // commented to document the invariant. + // if (count <= 0) + // { + // return 0; + // } byte value = 0; remainderView.SubView(count - 1, 1).CopyToCPU(ref value, 1); @@ -519,10 +534,11 @@ public sealed class MersenneNumberDivisorByDivisorGpuTester : IMersenneNumberDiv if (result >= modulus) { result -= modulus; - if (result >= modulus) - { - result %= modulus; - } + // The step deltas remain below the modulus, so a single subtraction suffices on this path. + // if (result >= modulus) + // { + // result %= modulus; + // } } return (byte)result; diff --git a/PerfectNumbers.Core/Gpu/MersenneNumberDivisorCandidateGpuEvaluator.cs b/PerfectNumbers.Core/Gpu/MersenneNumberDivisorCandidateGpuEvaluator.cs index e4e0fe1..b64aad2 100644 --- a/PerfectNumbers.Core/Gpu/MersenneNumberDivisorCandidateGpuEvaluator.cs +++ b/PerfectNumbers.Core/Gpu/MersenneNumberDivisorCandidateGpuEvaluator.cs @@ -68,10 +68,12 @@ internal sealed class MersenneNumberDivisorCandidateGpuEvaluator : IDisposable } int count = candidates.Length; - if (count == 0) - { - return; - } + // Candidate batches always carry at least one divisor on the by-divisor path, so the empty-count + // guard remains commented out to eliminate the branch. + // if (count == 0) + // { + // return; + // } EnsureCapacity(count); @@ -115,7 +117,12 @@ internal sealed class MersenneNumberDivisorCandidateGpuEvaluator : IDisposable return; } - int newCapacity = _capacity == 0 ? 1 : _capacity; + int newCapacity = _capacity; + // The evaluator always starts with a positive capacity, so the zero-capacity fallback remains commented. + // if (newCapacity == 0) + // { + // newCapacity = 1; + // } while (newCapacity < required) { newCapacity <<= 1; @@ -188,10 +195,11 @@ internal sealed class MersenneNumberDivisorCandidateGpuEvaluator : IDisposable private static int ComputeRemainder(byte remainder, byte step, int index, int modulus) { - if (modulus == 0) - { - return 0; - } + // Modulus lookups only cover {10, 8, 5, 3, 7, 11}, so the zero-modulus guard stays commented out. + // if (modulus == 0) + // { + // return 0; + // } long value = remainder; value += (long)step * index; diff --git a/PerfectNumbers.Core/Gpu/MersenneNumberDivisorMontgomeryGpuBuilder.cs b/PerfectNumbers.Core/Gpu/MersenneNumberDivisorMontgomeryGpuBuilder.cs index 5c290a2..6b8a163 100644 --- a/PerfectNumbers.Core/Gpu/MersenneNumberDivisorMontgomeryGpuBuilder.cs +++ b/PerfectNumbers.Core/Gpu/MersenneNumberDivisorMontgomeryGpuBuilder.cs @@ -54,10 +54,12 @@ internal sealed class MersenneNumberDivisorMontgomeryGpuBuilder : IDisposable } int length = moduli.Length; - if (length == 0) - { - return; - } + // Montgomery batches always include at least one modulus on the by-divisor path, so the empty-length + // guard stays commented out to avoid branching. + // if (length == 0) + // { + // return; + // } EnsureCapacity(length); @@ -83,7 +85,12 @@ internal sealed class MersenneNumberDivisorMontgomeryGpuBuilder : IDisposable return; } - int newCapacity = _capacity == 0 ? 1 : _capacity; + int newCapacity = _capacity; + // The builder always starts with a positive capacity, so the zero-capacity fallback remains commented. + // if (newCapacity == 0) + // { + // newCapacity = 1; + // } while (newCapacity < required) { newCapacity <<= 1; diff --git a/PerfectNumbers.Core/Gpu/MersenneNumberDivisorRemainderGpuStepper.cs b/PerfectNumbers.Core/Gpu/MersenneNumberDivisorRemainderGpuStepper.cs index e293430..9dd132f 100644 --- a/PerfectNumbers.Core/Gpu/MersenneNumberDivisorRemainderGpuStepper.cs +++ b/PerfectNumbers.Core/Gpu/MersenneNumberDivisorRemainderGpuStepper.cs @@ -71,10 +71,12 @@ internal sealed class MersenneNumberDivisorRemainderGpuStepper : IDisposable throw new ObjectDisposedException(nameof(MersenneNumberDivisorRemainderGpuStepper)); } - if (advanceCount <= 0 || remainders.Length == 0) - { - return; - } + // GPU remainder batches always advance by a positive chunk count and operate on fixed-size tables, so + // the defensive early-return remains commented out. + // if (advanceCount <= 0 || remainders.Length == 0) + // { + // return; + // } if (remainders.Length != _length) { @@ -99,17 +101,20 @@ internal sealed class MersenneNumberDivisorRemainderGpuStepper : IDisposable private static void AdvanceRemaindersKernel(Index1D index, int advanceCount, ArrayView remainders, ArrayView steps, ArrayView results) { int idx = index; - if (idx >= remainders.Length) - { - return; - } + // Kernel launches match the remainder table length, so the bounds guard stays commented to avoid redundant branching. + // if (idx >= remainders.Length) + // { + // return; + // } int modulus = GetModulus(idx); - if (modulus == 0) - { - results[idx] = remainders[idx]; - return; - } + // The modulus selector only emits positive bases (10, 8, 5, 3, 7, 11) for the configured indices, so the zero-modulus + // fallback remains commented out. + // if (modulus == 0) + // { + // results[idx] = remainders[idx]; + // return; + // } int remainder = remainders[idx] % modulus; int step = steps[idx] % modulus; diff --git a/PerfectNumbers.Core/Gpu/MersenneNumberDivisorResidueGpuEvaluator.cs b/PerfectNumbers.Core/Gpu/MersenneNumberDivisorResidueGpuEvaluator.cs index f6c7d9f..8970fc2 100644 --- a/PerfectNumbers.Core/Gpu/MersenneNumberDivisorResidueGpuEvaluator.cs +++ b/PerfectNumbers.Core/Gpu/MersenneNumberDivisorResidueGpuEvaluator.cs @@ -51,10 +51,12 @@ internal sealed class MersenneNumberDivisorResidueGpuEvaluator : IDisposable } int length = exponents.Length; - if (length == 0) - { - return; - } + // Residue batches always include at least one exponent when scheduled from the by-divisor scanner, so + // the empty-length guard remains commented out to avoid branching. + // if (length == 0) + // { + // return; + // } EnsureCapacity(length); @@ -79,10 +81,11 @@ internal sealed class MersenneNumberDivisorResidueGpuEvaluator : IDisposable } int newCapacity = _capacity; - if (newCapacity == 0) - { - newCapacity = 1; - } + // The evaluator always starts with a positive capacity, so the zero-capacity fallback stays commented. + // if (newCapacity == 0) + // { + // newCapacity = 1; + // } while (newCapacity < required) { diff --git a/PerfectNumbers.Core/MersenneNumberDivisorByDivisorTester.cs b/PerfectNumbers.Core/MersenneNumberDivisorByDivisorTester.cs index 2511190..508c06d 100644 --- a/PerfectNumbers.Core/MersenneNumberDivisorByDivisorTester.cs +++ b/PerfectNumbers.Core/MersenneNumberDivisorByDivisorTester.cs @@ -180,11 +180,13 @@ public static class MersenneNumberDivisorByDivisorTester return; } - if (maxPrime <= 1UL) - { - Console.WriteLine("The filter specified by --filter-p must contain at least one prime exponent greater than 1 for --mersenne=bydivisor."); - return; - } + // EvenPerfectBitScanner removes candidates <= 1 before reaching this point, + // so this guard never triggers during production runs. + // if (maxPrime <= 1UL) + // { + // Console.WriteLine("The filter specified by --filter-p must contain at least one prime exponent greater than 1 for --mersenne=bydivisor."); + // return; + // } tester.ConfigureFromMaxPrime(maxPrime);