Fix matrixVectorOp to verify promoted pointer type is still aligned to vectorized load boundary

[viclafargue]

Fix for rapidsai/cuml#3965
The function did not check for the memory alignment of the pointer provided.

[cjnolet]

@viclafargue, RMM should be aligning allocations by default, which is why this has not been an issue for cuml algorithms. Before RMM, we did manually align allocations, but since RMM does this for us, we shouldn't have to explicitly check this in the algorithms (since the number of places this might need to be done could be exhaustive).

Hopefully the solution proposed here solves the problem for the user.

[viclafargue]

Thanks for finding a solution. That would solve the problem on the Python side. Maybe these should even be done undercover by default when cuML is imported? However, do we have the guarantee that pointers provided to the native API are always memory aligned?

[cjnolet]

However, do we have the guarantee that pointers provided to the native API are always memory aligned?

The assumption here is that users will be using RAPIDS and so we should be able to expect that the RAPIDS memory manager is being used to allocate any pointers that are handed to these prims.

[viclafargue]

This PR is not necessary anymore.

[cjnolet]

I've reopened this pull request after chatting w/ @viclafargue and now that I have a better understanding of how this could be causing the issue in rapidsai/cuml#4199.

Specifically, the matrixVectorOp attempts to make the application of vector elements to the matrix more efficient by loading multiple vector elements into a single CUDA thread. This alone won't cause an issue, but the kernel is further optimizing the reads by loading the vector elements into the read-only L1 cache and, insodoing, might promote the datatype to minimize the number of reads (e.g. when 2x 8byte elements need to be read, it will use 1x 16 byte datatype so that it can perform a single read). It's this promotion of the datatype that assumes the pointer is aligned to the type being promoted. Thus, the pointer alignment needs to be explicitly checked when this promotion occurs. This is an internal assumption, specific to the matrixVectorOp.

[cjnolet]

I think this PR looks good to merge for 21.10 but we should add a nice comment here which summarizes these low-level details and justifies the need for the additional check on pointer alignment. Before knowing myself that the primitive was promoting the type to optimize the reads, I was under the impression the alignment issue was happening from the inputs. It would also be helpful if we added some additional documentation to the TxN_t struct as well. It currently states this: Obviously, it's caller's responsibility to take care of pointer alignment! but it would be helpful to add a sentence or two justifying why (the description of the vectorized op itself isn't bad but it still doesn't make particularly clear why the alignment might sometimes be needed).

[cjnolet]

LGTM

[cjnolet]

@gpucibot merge