Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Resolve "warning: copying an object of non-trivial type" #403

Open
vzhurba01 opened this issue Jan 15, 2025 · 0 comments
Open

Resolve "warning: copying an object of non-trivial type" #403

vzhurba01 opened this issue Jan 15, 2025 · 0 comments
Assignees
Labels
bug Something isn't working cuda.bindings Everything related to the cuda.bindings module P1 Medium priority - Should do

Comments

@vzhurba01
Copy link
Collaborator

Today when building cuda.bindings there are two types of warnings and we should conclude on how they are to be handled.

  1. dereferencing type-punned pointer will break strict-aliasing rules
cuda/bindings/_lib/cyruntime/utils.cpp: In function ‘cudaError_t __pyx_f_4cuda_8bindings_4_lib_9cyruntime_5utils_toDriverGraphNodeParams(const cudaGraphNodeParams*, CUgraphNodeParams*)’:
cuda/bindings/_lib/cyruntime/utils.cpp:41075:48: warning: dereferencing type-punned pointer will break strict-aliasing rules [-Wstrict-aliasing]
41075 |     (__pyx_v_driverParams[0]).extSemSignal = (((CUDA_EXT_SEM_SIGNAL_NODE_PARAMS_v2 *)(&__pyx_v_rtParams[0].extSemSignal))[0]);
      |                                               ~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
cuda/bindings/_lib/cyruntime/utils.cpp:41075:48: warning: dereferencing type-punned pointer will break strict-aliasing rules [-Wstrict-aliasing]
cuda/bindings/_lib/cyruntime/utils.cpp:41113:46: warning: dereferencing type-punned pointer will break strict-aliasing rules [-Wstrict-aliasing]
41113 |     (__pyx_v_driverParams[0]).extSemWait = (((CUDA_EXT_SEM_WAIT_NODE_PARAMS_v2 *)(&(__pyx_v_rtParams[0]).extSemWait))[0]);
      |                                             ~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
cuda/bindings/_lib/cyruntime/utils.cpp:41113:46: warning: dereferencing type-punned pointer will break strict-aliasing rules [-Wstrict-aliasing]
  1. copying an object of non-trivial type
cuda/bindings/runtime.cpp: In function ‘int __pyx_pf_4cuda_8bindings_7runtime_25cudaGraphKernelNodeUpdate_10updateData_2__set__(__pyx_obj_4cuda_8bindings_7runtime_cudaGraphKernelNodeUpdate*, __pyx_obj_4cuda_8bindings_7runtime_anon_union8*)’:
cuda/bindings/runtime.cpp:216214:16: warning: ‘void* memcpy(void*, const void*, size_t)’ copying an object of non-trivial type ‘union cudaGraphKernelNodeUpdate::<unnamed>’ from an array of ‘union __pyx_pf_4cuda_8bindings_7runtime_25cudaGraphKernelNodeUpdate_10updateData_2__set__(__pyx_obj_4cuda_8bindings_7runtime_cudaGraphKernelNodeUpdate*, __pyx_obj_4cuda_8bindings_7runtime_anon_union8*)::anon_union8’ [-Wclass-memaccess]
216214 |   (void)(memcpy((&(__pyx_v_self->_ptr[0]).updateData), ((union anon_union8 *)((__pyx_t_4cuda_8bindings_7runtime_void_ptr)__pyx_t_5)), (sizeof((__pyx_v_self->_ptr[0]).updateData))));
       |          ~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
In file included from cuda/bindings/runtime.cpp:1248:
/usr/local/cuda-12.6/include/driver_types.h:3302:11: note: ‘union cudaGraphKernelNodeUpdate::<unnamed>’ declared here
 3302 |     union {
      |           ^

(1) We should ignore this because this is consistent with CUDA Runtime source and this file will be removed anyways in #100
(2) This one is tricky. What's happening is that we are trying to use memcpy on a type that has a non-trivial member (i.e. dim3). There's only one instance of this warning and we could do an simple WAR for this specific case:

diff --git a/cuda_bindings/cuda/bindings/runtime.pyx.in b/cuda_bindings/cuda/bindings/runtime.pyx.in
index e735ee4..0432d8e 100644
--- a/cuda_bindings/cuda/bindings/runtime.pyx.in
+++ b/cuda_bindings/cuda/bindings/runtime.pyx.in
@@ -12131,7 +12131,11 @@ cdef class cudaGraphKernelNodeUpdate:
         return self._updateData
     @updateData.setter
     def updateData(self, updateData not None : anon_union8):
-        string.memcpy(&self._ptr[0].updateData, <cyruntime.anon_union8*><void_ptr>updateData.getPtr(), sizeof(self._ptr[0].updateData))
+        self._ptr[0].updateData.gridDim.gridDim.x = updateData.gridDim.gridDim.x
+        self._ptr[0].updateData.gridDim.gridDim.y = updateData.gridDim.gridDim.y
+        self._ptr[0].updateData.gridDim.gridDim.z = updateData.gridDim.gridDim.z
+        string.memcpy(&self._ptr[0].updateData.param, <cyruntime.anon_struct19*><void_ptr>updateData.param.getPtr(), sizeof(self._ptr[0].updateData.param))
+        string.memcpy(&self._ptr[0].updateData.isEnabled, <unsigned int*><void_ptr>updateData.getPtr(), sizeof(self._ptr[0].updateData.isEnabled))
 {{endif}}
 {{if 'struct cudaLaunchMemSyncDomainMap_st' in found_types}}

What is tricky is how we should be approach these types of errors in the future. Worst case scenario is when you have a non-trivial type that's nested under many layers. Applying the same WAR in this multi-nested scenario would result in each Extension Type interacting with their members across multiple layers, is easier to make a mistake and updating the bindings generator is non-trivial.

A long term solution would perhaps have each Extension Type have a dedicated copy method.

For this issue I suggest we apply the WAR for this warning and any future warnings as long as they remain simple and are rare. The long term solution can be re-investigated when that is no longer the case.

@vzhurba01 vzhurba01 added bug Something isn't working cuda.bindings Everything related to the cuda.bindings module P1 Medium priority - Should do labels Jan 15, 2025
@vzhurba01 vzhurba01 self-assigned this Jan 15, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working cuda.bindings Everything related to the cuda.bindings module P1 Medium priority - Should do
Projects
None yet
Development

No branches or pull requests

1 participant