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

Use direct linking to vecgeom's CUDA target to match AdePT #346

Merged
merged 1 commit into from
Feb 9, 2025

Conversation

drbenmorgan
Copy link
Member

Reported by @SeverinDiederichs, trying to build AdePT with a static VecGeom results in a failure of the test_g4vg_link buildtime test. This wasn't spotted in the initial PR because my setup had used a shared build of VecGeom. The underlying cause is likely a corner case in cuda_rdc_target_link_libraries not linking in everything from VecGeom in the right order, or failing to do a final device link in the static case. This does not affect linking of AdePT itself with VecGeom and G4VG, where the explicit links to the VecGeom libs are done in the same way as this fix.

Use direct call to target_link_libraries for the G4VG link test, explicitly linking to both G4VG and the Vecgeomcuda targets. This gives a correct link whether VecGeom was built with static or shared libs, and matches what is done elsewhere in AdePT.

Reported by @SeverinDiederichs, trying to build AdePT with a static
VecGeom results in a failure of the `test_g4vg_link` buildtime test.
Likely caused by a corner case in `cuda_rdc_target_link_libraries`
not linking in everything from VecGeom in the right order, or failing
to device link in the static case. Linking of AdePT itself with
VecGeom and G4VG all fine.

Use direct call to target_link_libraries for the G4VG link test,
explicitly linking to both G4VG and the Vecgeomcuda targets. This gives
a correct link whether VecGeom was built with static or shared libs.
@phsft-bot
Copy link

Can one of the admins verify this patch?

Copy link
Collaborator

@SeverinDiederichs SeverinDiederichs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the quick fix!

@SeverinDiederichs SeverinDiederichs merged commit 466ee9c into apt-sim:master Feb 9, 2025
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants