Unnecessary memory zeroing commented out. TODOs added for the remainders.

This commit is contained in:
mlemanczyk committed 2025-10-23 19:42:03 +02:00
1 parent 8e3e990499
commit 0d7df2e45b
7 files changed
+53 -15

No files matched your search

@@ -68,7 +68,6 @@ internal static class IncrementalKernels
return;
}
// TODO: Is this expected that the value of kStart is modified?
GpuUInt128 k = kStart + (GpuUInt128)idx;
GpuUInt128 q = twoP;
GpuUInt128.Mul64(ref q, k.High, k.Low);
@@ -125,13 +124,16 @@ internal static class IncrementalKernels
ulong exponent,
GpuUInt128 twoP,
GpuUInt128 kStart,
byte lastIsSeven,
byte _,
ulong divMul,
ResidueAutomatonArgs ra,
ArrayView<int> found,
ArrayView1D<ulong, Stride1D.Dense> smallCycles)
{
// TODO: Modify this kernel to always assign the required found element to the result of the calculations and
// remove any clearing of the output buffer in the caller methods.
ulong idx = (ulong)index.X;
// TODO: Replace % operator in GPU and their outgoing paths with logical operations, where possible.
ulong idxMod3 = idx % 3UL;
ulong idxMod5 = idx % 5UL;
ulong r10 = ra.Q0M10 + (ra.Step10 * idx).Mod10(); r10 -= (r10 >= 10UL) ? 10UL : 0UL;
@@ -6,6 +6,7 @@ namespace PerfectNumbers.Core.Gpu;
internal static class Pow2ModKernels
{
/// This kernel always sets the result of the corresponding element. Callers don't need to clear the output buffers.
public static void Pow2ModKernelScan(
Index1D index,
ulong exponent,
@@ -22,10 +23,12 @@ internal static class Pow2ModKernels
ArrayView1D<ulong, Stride1D.Dense> smallPrimesPow2LastSeven)
{
ulong idx = (ulong)index.X;
// TODO: Replace % operator in GPU and their outgoing paths with logical operations, where possible.
ulong idxMod3 = idx % 3UL;
ulong idxMod5 = idx % 5UL;
ulong r10 = ra.Q0M10 + (ra.Step10 * idx).Mod10(); r10 -= (r10 >= 10UL) ? 10UL : 0UL;
bool shouldCheck = r10 != 5UL;
// TODO: Replace these "if" statements with ?:
if (shouldCheck)
{
ulong r8 = (ra.Q0M8 + ((ra.Step8 * idx) & 7UL)) & 7UL;
@@ -61,7 +64,6 @@ internal static class Pow2ModKernels
return;
}
// TODO: kStart is modified after this. Is this expected?
kStart.Add(idx);
GpuUInt128 q = twoP;
GpuUInt128.Mul64(ref q, kStart.High, kStart.Low);
@@ -104,13 +106,15 @@ internal static class Pow2ModKernels
orders[index] = exponent;
}
/// This kernel doesn't always set the result of the found array. It sets the value only when it needs to. The callers
/// must clean the output buffer before the call to get a deterministic result.
public static void Pow2ModOrderKernelScan(
Index1D index,
ulong exponent,
GpuUInt128 twoP,
GpuUInt128 kStart,
byte lastIsSeven,
ulong _,
byte _,
ulong __,
ResidueAutomatonArgs ra,
ArrayView<int> found,
ArrayView1D<ulong, Stride1D.Dense> smallCycles)
@@ -10,6 +10,8 @@ namespace PerfectNumbers.Core.Gpu;
internal static partial class PrimeOrderGpuHeuristics
{
/// This kernel always sets the result and operates within the allowed / required bounds, never accessing elements outside of them.
/// It sets the value only when it needs to. The callers don't need to clear the output / input buffers.
internal static void PartialFactorKernel(
Index1D index,
ArrayView1D<uint, Stride1D.Dense> primes,
@@ -111,6 +113,8 @@ internal static partial class PrimeOrderGpuHeuristics
}
}
/// This kernel doesn't always set the result of the first element. It sets the value only when it needs to. The callers
/// must clean the output buffer before the call to get a deterministic result.
internal static void CalculateOrderKernel(
Index1D index,
ulong prime,
@@ -126,6 +130,8 @@ internal static partial class PrimeOrderGpuHeuristics
return;
}
// TODO: Modify this kernel to always set the output result so that callers never need to clear any buffers and remove
// any existing cleaning of the buffers after that.
ArrayView1D<ulong, Stride1D.Dense> phiFactors = buffers.PhiFactors;
ArrayView1D<int, Stride1D.Dense> phiExponents = buffers.PhiExponents;
ArrayView1D<ulong, Stride1D.Dense> workFactors = buffers.WorkFactors;
@@ -145,6 +151,9 @@ internal static partial class PrimeOrderGpuHeuristics
statusOut[0] = (byte)PrimeOrderKernelStatus.Fallback;
// TODO: Is this condition ever satisfied on EventPerfectBitScanner's execution paths? Comment it out with explanatory
// comment before it, if not. Otherwise add a comment explaining how it's used. If tests or benchmarks require this,
// modify such tests and benchmark to avoid that.
if (prime <= 3UL)
{
ulong orderValue = prime == 3UL ? 2UL : 1UL;
@@ -1153,6 +1162,8 @@ internal static partial class PrimeOrderGpuHeuristics
return order;
}
/// This kernel always sets the result of the corresponding element. Callers don't need to clear the output buffers.
internal static void Pow2ModKernel(Index1D index, ArrayView1D<ulong, Stride1D.Dense> exponents, MontgomeryDivisorData divisor, ArrayView1D<ulong, Stride1D.Dense> remainders)
{
ulong exponent = exponents[index];
@@ -1169,6 +1180,7 @@ internal static partial class PrimeOrderGpuHeuristics
});
}
/// This kernel always sets the result of the corresponding element. Callers don't need to clear the output buffers.
internal static void Pow2ModKernelWide(Index1D index, ArrayView1D<GpuUInt128, Stride1D.Dense> exponents, GpuUInt128 modulus, ArrayView1D<GpuUInt128, Stride1D.Dense> remainders)
{
GpuUInt128 exponent = exponents[index];
@@ -41,7 +41,8 @@ public sealed class MersenneNumberDivisorGpuTester
var accelerator = gpu.Accelerator;
var kernel = GetKernel(accelerator);
var resultBuffer = _resultBuffers.GetOrAdd(accelerator, acc => acc.Allocate1D<byte>(1));
resultBuffer.MemSetToZero();
// There is no point in clearing this buffer. We always override item [0] and never use it beyond item [0]
// resultBuffer.MemSetToZero();
kernel(1, exponent, divisor, resultBuffer.View);
accelerator.Synchronize();
Span<byte> result = stackalloc byte[1];
@@ -35,6 +35,7 @@ public class MersenneNumberOrderGpuTester(GpuKernelType kernelType, bool useGpuO
{
remaining = maxK - kStart + 1UL;
currentSize = remaining > (UInt128)batchSize ? batchSize : (int)remaining;
// This clear is required because the result may not always be set. Remove it after the kernels are modified to always set the result.
foundBuffer.MemSetToZero();
// Precompute residue automaton bases for this batch
UInt128 q0 = twoP * kStart + 1UL;
@@ -144,7 +144,8 @@ public class MersenneNumberResidueGpuTester(bool useGpuOrder)
{
if (resources.Capacity >= capacity)
{
resources.OrderBuffer.MemSetToZero();
// The caller always overwrites the required elements. We don't need to worry about clearing the output buffer.
// resources.OrderBuffer.MemSetToZero();
return resources;
}
@@ -152,7 +153,8 @@ public class MersenneNumberResidueGpuTester(bool useGpuOrder)
}
var created = new ResidueResources(accelerator, capacity);
created.OrderBuffer.MemSetToZero();
// The caller always overwrites the required elements. We don't need to worry about clearing the output buffer.
// created.OrderBuffer.MemSetToZero();
return created;
}
@@ -201,17 +201,19 @@ internal static partial class PrimeOrderGpuHeuristics
var kernel = GetPartialFactorKernel(accelerator);
SmallPrimeDeviceCache cache = GetSmallPrimeDeviceCache(accelerator);
// TODO: We should create / reallocate these buffer only once or if the new required length is bigger than capacity.
var factorBuffer = accelerator.Allocate1D<ulong>(primeTargets.Length);
var exponentBuffer = accelerator.Allocate1D<int>(exponentTargets.Length);
var countBuffer = accelerator.Allocate1D<int>(1);
var remainingBuffer = accelerator.Allocate1D<ulong>(1);
var fullyFactoredBuffer = accelerator.Allocate1D<byte>(1);
factorBuffer.MemSetToZero();
exponentBuffer.MemSetToZero();
countBuffer.MemSetToZero();
remainingBuffer.MemSetToZero();
fullyFactoredBuffer.MemSetToZero();
// TODO: There is no need to clear these buffers because the kernel will always assign values within the required bounds.
// factorBuffer.MemSetToZero();
// exponentBuffer.MemSetToZero();
// countBuffer.MemSetToZero();
// remainingBuffer.MemSetToZero();
// fullyFactoredBuffer.MemSetToZero();
kernel(
stream,
@@ -240,6 +242,7 @@ internal static partial class PrimeOrderGpuHeuristics
fullyFactoredBuffer.View.CopyToCPU(ref fullyFactoredFlag, 1);
fullyFactored = fullyFactoredFlag != 0;
// TODO: These buffers shouldn't be disposed but rather reused accross call with the assigned accelerator.
factorBuffer.Dispose();
exponentBuffer.Dispose();
countBuffer.Dispose();
@@ -366,6 +369,9 @@ internal static partial class PrimeOrderGpuHeuristics
var kernel = GetOrderKernel(accelerator);
SmallPrimeDeviceCache cache = GetSmallPrimeDeviceCache(accelerator);
// TODO: These buffers should be created reallocated once and assigned to an accelerator. Callers should use their own
// thread static cache to prevent other threads from using taking accelerators with pre-allocated buffers if they don't
// use them.
var phiFactorBuffer = accelerator.Allocate1D<ulong>(GpuSmallPrimeFactorSlots);
var phiExponentBuffer = accelerator.Allocate1D<int>(GpuSmallPrimeFactorSlots);
var workFactorBuffer = accelerator.Allocate1D<ulong>(GpuSmallPrimeFactorSlots);
@@ -377,6 +383,7 @@ internal static partial class PrimeOrderGpuHeuristics
var resultBuffer = accelerator.Allocate1D<ulong>(1);
var statusBuffer = accelerator.Allocate1D<byte>(1);
// TODO: Remove the cleaning after the order kernel is modified to always set the result.
phiFactorBuffer.MemSetToZero();
phiExponentBuffer.MemSetToZero();
workFactorBuffer.MemSetToZero();
@@ -523,11 +530,16 @@ internal static partial class PrimeOrderGpuHeuristics
Accelerator accelerator = lease.Accelerator;
AcceleratorStream stream = lease.Stream;
var kernel = GetPow2ModKernel(accelerator);
// TODO: These buffers should be allocated once per accelerator and only reallocated when the new length exceeds capacity.
// Modify the callers to use their own pool of buffers per accelerator, so that other threads don't use the accelerators
// with out preallocated buffers. Share the pool with Pow2ModWide kernel.
var exponentBuffer = accelerator.Allocate1D<ulong>(exponents.Length);
var remainderBuffer = accelerator.Allocate1D<ulong>(exponents.Length);
exponentBuffer.View.CopyFromCPU(ref MemoryMarshal.GetReference(exponents), exponents.Length);
remainderBuffer.MemSetToZero();
// Pow2Mod kernel always assigns the required output elements so we don't need to worry about clearing these.
// remainderBuffer.MemSetToZero();
try
{
@@ -579,6 +591,9 @@ internal static partial class PrimeOrderGpuHeuristics
Accelerator accelerator = lease.Accelerator;
AcceleratorStream stream = lease.Stream;
var kernel = GetPow2ModWideKernel(accelerator);
// TODO: These buffers should be allocated once per accelerator and only reallocated when the new length exceeds capacity.
// Modify the callers to use their own pool of buffers per accelerator, so that other threads don't use the accelerators
// with out preallocated buffers. Share the pool with Pow2Mod kernel.
var exponentBuffer = accelerator.Allocate1D<GpuUInt128>(length);
var remainderBuffer = accelerator.Allocate1D<GpuUInt128>(length);
@@ -592,7 +607,8 @@ internal static partial class PrimeOrderGpuHeuristics
}
exponentBuffer.View.CopyFromCPU(ref MemoryMarshal.GetReference(exponentSpan), length);
remainderBuffer.MemSetToZero();
// Pow2ModWide kernel always assigns the required output elements so we don't need to worry about clearing these.
// remainderBuffer.MemSetToZero();
GpuUInt128 modulus = (GpuUInt128)prime;
kernel(stream, length, exponentBuffer.View, modulus, remainderBuffer.View);