madgraph5 / madgraph4gpu

GPU development for the Madgraph5_aMC@NLO event generator software package
30 stars 33 forks source link

WIP: debug CUDA build warnings in the CI #795

Open valassi opened 9 months ago

valassi commented 9 months ago

Debug CUDA build warnings observed in the CI for PR #753

https://github.com/madgraph5/madgraph4gpu/actions/runs/6954971982/job/18922834648?pr=753

/usr/local/cuda//bin/nvcc -fPIC -std=c++17 --forward-unknown-to-host-compiler -Wall -Wshadow -Wextra -I/usr/local/cuda//include/ -DUSE_NVTX --generate-code arch=compute_70,code=compute_70 --generate-code arch=compute_70,code=sm_70 -ccbin /opt/rh/gcc-toolset-12/root/usr/bin/g++ -DMGONGPU_FPTYPE_DOUBLE -DMGONGPU_FPTYPE2_DOUBLE -O3 -ffast-math -use_fast_math -DNDEBUG -lineinfo -c -x cu Parameters_sm.cc -o build.cuda_d_inl0_hrd0/Parameters_sm_cu.o
/usr/local/cuda//include/thrust/detail/execute_with_dependencies.h: In constructor ‘thrust::detail::execute_with_dependencies<BaseSystem, Dependencies>::execute_with_dependencies(const super_t&, Dependencies&& ...)’:
/usr/local/cuda//include/thrust/detail/execute_with_dependencies.h:64:72: warning: declaration of ‘dependencies’ shadows a member of ‘thrust::detail::execute_with_dependencies<BaseSystem, Dependencies>’ [-Wshadow]
   64 |     execute_with_dependencies(super_t const &super, Dependencies && ...dependencies)
      |                                                         ~~~~~~~~~~~~~~~^~~~~~~~~~~~~  
/usr/local/cuda//include/thrust/detail/execute_with_dependencies.h:60:57: note: shadowed declaration is here
   60 |     std::tuple<remove_cvref_t<Dependencies>...> dependencies;
      |                                                         ^~~~~       
hageboeck commented 9 months ago

Not to worry about this, I fixed it upstream. https://github.com/NVIDIA/cccl/pull/334

With some future cuda version this will go away.

valassi commented 9 months ago

Ah wow very good and well done thanks a lot!

Anyway, I think we should add 'fail the build on warning' at some point, I keep this open as a reminder.

Also, lets make sure the CI has the nvidia version where this does not happen.

valassi commented 9 months ago

There is another warning here

ccache g++ -fPIC -std=c++17 -Wall -Wshadow -Wextra  -march=native -DMGONGPU_FPTYPE_DOUBLE -DMGONGPU_FPTYPE2_DOUBLE -I. -I. -I../../src -fopenmp -O3 -ffast-math -DNDEBUG -c counters.cc -o counters.o
counters.cc: In function ‘const char* iimplC2TXT(int)’:
counters.cc:35:3: warning: control reaches end of non-void function [-Wreturn-type]
   35 |   }
      |   ^

Just add it here as a reminder

hageboeck commented 9 months ago

Anyway, I think we should add 'fail the build on warning' at some point, I keep this open as a reminder.

Also, lets make sure the CI has the nvidia version where this does not happen.

Both good suggestions, let's do that! Once the Makefile work is merged, adding the -Werror can be done by just setting environment variables in the CI. 🙂

I don't know, however, in which cuda version the fix will show up ... 🙁