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.

Sign up for GitHub

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

Jump to bottom

WIP: separate cpu/gpu namespaces and fix runtest segmentation fault #728

Closed
wants to merge 22 commits into from
Closed

WIP: separate cpu/gpu namespaces and fix runtest segmentation fault #728

wants to merge 22 commits into from

Conversation

valassi
Copy link
Member

@valassi valassi commented Jul 19, 2023

This is a WIP MR that fixes #725, ie the segfault in runtest.

It is a complete fix in ggtt.sa. I keep it in WIP hoever and will probably close it, because I have other MRs that may soon be committed and which this will also modify a lot (@Jooorgen 's HIP MR #718 and my FPE MR # 723, which is actually where I first noticed this issue).

I will most likely merge this into #723. And then discuss with Jorgen which one of HIP or FPE should go in first.

Note: this patch essentially extends the use of the two separate namespaces for CPU and GPU builds (as discussed in #318). While I am still not convinced that it is a good idea to mix both types of builds in a single executable (which is what was causing the segfault #725), at least now this is a bit safer. I am not sure if all issues are fixed, but ceratnly it is much better. A few details

  • Now fptype_sv and cxtype_sv, and also cxtype, are defined in either a CPU or GPU namespace, where they have different definitions
  • In Segfault in testxxx runTest.exe for debug builds (need separate cpu/gpu namespaces) #725 I was reporting not only a segfault, but also a strange mixup where some tests seemed to mix data arrays with different strutures, which is indeed probably a side effect of having different vector sizes mixed
  • I have no idea why we have not observed these errors before, and why they only show up in debug builds...
  • I have also moved to separate GPU/CPU namespaces the Parameters class. For this I had to also build the Parameters class in CUDA in src.
  • The CUDA build in src is done by simply exporting NVCC. This may be a good ides to simplify makefiles eventually in src also for AVX etc.

@valassi
[fpe] in ggtt.sa mgOnGpuVectors.h, add maskand function
896ce27 
The build fails because maskand is also defined in testmisc.cc
@valassi
[fpe] in ggtt.sa mgOnGpuVectors.h, add maskand function also for CUDA
0700769 
So far I was never building for CUDA - now the build succeeds, and the test too!
@valassi
[namespace] in ggtt.sa, start separation of cpu/gpu namespaces: first…
188799c 
… changes in mgOnGpuConfig.h and mgOnGpuVectors.h
@valassi
[namespace] in ggtt.sa, start separation of cpu/gpu namespaces: first…
75c5ffb 
… changes in mgOnGpuCxtypes.h
@valassi
[namespace] in ggtt.sa, start separation of cpu/gpu namespaces: furth…
88ecc64 
…er changes in mgOnGpuCxtypes.h
@valassi
[namespace] in ggtt.sa, start separation of cpu/gpu namespaces: furth…
0743d77 
…er changes in mgOnGpuCxtypes.h
@valassi
[namespace] in ggtt.sa, start separation of cpu/gpu namespaces: furth…
922571f 
…er changes in mgOnGpuCxtypes.h
@valassi
[namespace] in ggtt.sa, start separation of cpu/gpu namespaces: furth…
cdef96e 
…er changes in mgOnGpuCxtypes.h
@valassi
[namespace] in ggtt.sa, start separation of cpu/gpu namespaces: furth…
959e2e4 
…er changes in mgOnGpuCxtypes.h
@valassi
[namespace] in ggtt.sa, start separation of cpu/gpu namespaces: furth…
ef2ef1b 
…er changes in mgOnGpuVectors.h
@valassi
[namespace] in ggtt.sa, first fixes for cpu/gpu namespace in Paramete…
80e35ba 
…rs_sm.h/cc
@valassi
[namespace] in ggtt.sa mgOnGpuFptypes.h, move fpsqrt and other functi…
03855f3 
…ons in the cpu/gpu namespace
@valassi
[namespace] in ggtt.sa mgOnGpuCxtypes.h, move nx2 back to the common …
c42b0ba 
…namespace
@valassi
[namespace] in ggtt.sa MemoryAccessMomenta.h, move the class into the…
8efbc22 
… cpu/gpu namespace
@valassi
[namespace] in ggtt.sa Bridge.h, remove cpu/gpu namespace qualifiers …
f55dda9 
…for memory buffers and a few other types
@valassi
[namespace] in six ggtt.sa MemoryXXX.h, move the class into the cpu/g…
d2c7d52 
…pu namespace
@valassi
[namespace] in ggtt.sa BridgeKernels.cc, fix namespaces for CPPProcess
c771b9b
@valassi
[namespace] in ggtt.sa, further fix for cpu/gpu namespace in Paramete…
1e34192 
…rs_sm.h
@valassi
[namespace] in ggtt.sa, add gParameters_sm.cu and buiold it with CUDA…
e8271b5 
… in src (export NVCC to simplifiy the config; also remove __device__ from operator<<)
@valassi
[namespace] in ggtt.sa, further fix for cpu/gpu namespace in mgOnGpuC…
e96207e 
…xtypes.h
@valassi
[fpe] in ggtt.sa testmisc.ss, remove maskand function as it exists in…
1a89987 
… mgOnGpuVectors.h now
@valassi
[namespace] in ggtt.sa, fix testmisc.cc and testxxx.cc to use differe…
eee0468 
…nt namespaces

This CONCLUDES the cleanup of namespaces in ggtt.sa: everything builds and runs ok

NB: in debug mode, now runTest succeeds! As intended, this fixes the segfault in madgraph5#725
@valassi valassi marked this pull request as draft July 19, 2023 10:22
@valassi valassi self-assigned this Jul 19, 2023
This was referenced Jul 19, 2023
Merged
Closed
@valassi
Copy link
Member Author

valassi commented Jul 19, 2023

This is now merged into #723. I am closing this.

@valassi valassi closed this Jul 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@valassi valassi

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Segfault in testxxx runTest.exe for debug builds (need separate cpu/gpu namespaces)
1 participant
@valassi