Enable TensorPrimitives to perform in-place operations

[stephentoub]

Some operations would produce incorrect results if the same span was passed as both an input and an output. When vectorization was employed but the span's length wasn't a perfect multiple of a vector, we'd do the standard trick of performing one last operation on the last vector's worth of data; however, that relies on the operation being idempotent, and if a previous operation has overwritten input with a new value due to the same memory being used for input and output, some operations won't be idempotent. This fixes that by masking off the already processed elements. It adds tests to validate in-place use works, and it updates the docs to carve out this valid overlapping.

Contributes to #92219

[ghost]

Tagging subscribers to this area: @dotnet/area-system-numerics
See info in area-owners.md if you want to be subscribed.

Issue Details

---

Some operations would produce incorrect results if the same span was passed as both an input and an output. When vectorization was employed but the span's length wasn't a perfect multiple of a vector, we'd do the standard trick of performing one last operation on the last vector's worth of data; however, that relies on the operation being idempotent, and if a previous operation has overwritten input with a new value due to the same memory being used for input and output, some operations won't be idempotent. This fixes that by masking off the already processed elements. It adds tests to validate in-place use works, and it updates the docs to carve out this valid overlapping.

Contributes to #92219

Author:	stephentoub
Assignees:	stephentoub
Labels:	area-System.Numerics
Milestone:	-

[EgorBo]

Any particular reason we can't handle overlapped pointers? We can throw an exception (the check is cheap, isn't) or just use scalar fallback? It's probably fine to expect ML workloads to be non-aliased, but it makes it unsafe to use in a general case I presume?

[tannergooding]

There's little benefit and a lot of drawbacks, even for scalar code paths.

If your output overlaps your input after the input start, then you have a self mutating algorithm even for the scalar side.

So it's easier just to leave it as UB and tell users "don't do that".

[EgorBo]

If your output overlaps your input after the input start, then you have a self mutating algorithm even for the scalar side. So it's easier just to leave it as UB and tell users "don't do that".

I don't get why it's bad - what if I want to Subtract a[i - 1] from a[i] in my algorithm - data overlaps. An exception telling me "you should go and make a copy of your data" sounds way better than a silent UB to me.

even for the scalar side.

I don't see any problems with that. Thats how auto-vectorization works in C++, memset,memcpy etc - we check whether pointers don't overlap (if we can't tell that from a general alias analysis) and do simd, otherwise - scalar fallback.

We do that in BCL as well, e.g. Buffer.Memmove:

        internal static void Memmove(ref byte dest, ref byte src, nuint len)
        {
            // P/Invoke into the native version when the buffers are overlapping.
            if (((nuint)(nint)Unsafe.ByteOffset(ref src, ref dest) < len) || ((nuint)(nint)Unsafe.ByteOffset(ref dest, ref src) < len))
            {

[EgorBo]

Burst solved this via [NoAlias] attributes: https://docs.unity3d.com/Packages/com.unity.burst@1.4/manual/docs/OptimizationGuidelines-Aliasing.html

It seems like silent UB is the worst option honestly

[tannergooding]

An exception telling me "you should go and make a copy of your data" sounds way better than a silent UB to me.

We could explicitly throw, yes. I don't have a preference.

I don't see any problems with that. Thats how auto-vectorization works in C++, memset,memcpy etc - we check whether pointers don't overlap (if we can't tell that from a general alias analysis) and do simd, otherwise - scalar fallback.

A lot of those are explicitly UB (particularly memcpy) or only have a single input (memset). memmove is one of the only places where overlapping input/output is well defined and only because it can be well-defined since it's a single input/output and only doing memory copying, so the output overwriting inputs isn't a concern (because it is spec'd to work as if a copy to an intermediate buffer happened first).

[EgorBo]

A lot of those are explicitly UB (particularly memcpy)

That's why in many places memcpy is just an alias of memmove for safety reasons. .NET doesn't even have an API for it, it's Buffer.MemoryCopy and NativeMemory.Copy are both memmove

[stephentoub]

This PR isn't introducing the undefined behavior, and rather is making some more defined, so I'm going to merge, and we can subsequently decide to follow-up with either leaving it as is, throwing for any other kinds of overlap, or making it work.

[tannergooding]

That's why in many places memcpy is just an alias of memmove for safety reasons.

Places doing it for convenience or simplicity doesn't mean it's something that can be relied upon.

The do it because it's trivial in the case of single input/output to handle it using SIMD with a minor additional code path that remains accelerated. It basically just requires caching an extra value in a temporary SIMD register.

For cases like Abs or similar, that would likewise be trivial. For cases like Add, that becomes immensely complex due to having to consider all the ways the destination could overlap with either input.

I'd be in favor of simply throwing if the UB is that much of a concern. It's what we do for a few other APIs.