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

Remove libcudacxx symlinks #1075

Merged
merged 16 commits into from
Nov 14, 2023
Merged

Remove libcudacxx symlinks #1075

merged 16 commits into from
Nov 14, 2023

Conversation

wmaxey
Copy link
Member

@wmaxey wmaxey commented Nov 9, 2023

Description

closes #143

It deletes the symlinks and restructures the test layout for libcudacxx.
It deletes many libcxx symbol testing artifacts that are currently unused (we can bring them back if we need to).

Checklist

  • New or existing tests cover these changes.
  • The documentation is up to date with these changes.

@wmaxey wmaxey requested review from a team as code owners November 9, 2023 07:42
@wmaxey wmaxey requested review from robertmaynard, ericniebler, jarmak-nv, miscco and griwes and removed request for a team November 9, 2023 07:42
@wmaxey wmaxey requested a review from miscco November 10, 2023 00:46
Copy link
Contributor

@ahendriksen ahendriksen left a comment

Choose a reason for hiding this comment

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

What a huge PR! I see that a lot of changes were in code I recently committed. Sorry for that! I left some questions inline and I also ran CI on H100.

I ran into some failures were not part of the diff, so adding them here:

5: /home/scratch.ahendriksen_gpu/projects/cccl/libcudacxx/test/libcudacxx/cuda/barrier/cp_async_bulk_tensor_generic.h:234:19: error: comparison of integer expressions of different signedness: ‘int’ and ‘long unsigned int’ [-Werror=sign-compare]                            
5:   234 |     for (int i = 0; i < rank-1; ++i) {                                                                                                                                                                                                                               
5:       |                 ~~^~~~~~~~~~~~                                                                                                                                                                                                                                       
5: cc1plus: all warnings being treated as errors                                                                                                                                                                  

Quick fix would be to change to change to: for (int i = 0; i < int(rank-1); ++i)

5: /home/scratch.ahendriksen_gpu/projects/cccl/libcudacxx/test/libcudacxx/cuda/memory_resource/memory_resource.resource_ref/resource_ref.conversion.pass.cpp: In instantiation of ‘void test_conversion_from_resource_ref() [with PropA = property_with_value<short int>; PropB
= property_with_value<int>]’:                                                                                                                                     
5: /home/scratch.ahendriksen_gpu/projects/cccl/libcudacxx/test/libcudacxx/cuda/memory_resource/memory_resource.resource_ref/resource_ref.conversion.pass.cpp:157:94:   required from here
5: /home/scratch.ahendriksen_gpu/projects/cccl/libcudacxx/test/libcudacxx/cuda/memory_resource/memory_resource.resource_ref/resource_ref.conversion.pass.cpp:79:27: error: dereferencing type-punned pointer will break strict-aliasing rules [-Werror=strict-aliasing]
5:    79 |     const auto fake_orig = *reinterpret_cast<Fake_alloc_base*>(&ref_input);                                                            
5:       |                          ~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~                                                       
5: /home/scratch.ahendriksen_gpu/projects/cccl/libcudacxx/test/libcudacxx/cuda/memory_resource/memory_resource.resource_ref/resource_ref.conversion.pass.cpp:79:27: error: dereferencing type-punned pointer will break strict-aliasing rules [-Werror=strict-aliasing]
5: /home/scratch.ahendriksen_gpu/projects/cccl/libcudacxx/test/libcudacxx/cuda/memory_resource/memory_resource.resource_ref/resource_ref.conversion.pass.cpp:80:27: error: dereferencing type-punned pointer will break strict-aliasing rules [-Werror=strict-aliasing]
5:    80 |     const auto fake_conv = *reinterpret_cast<Fake_alloc_base*>(&ref);                                                                            
5:       |                          ~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~                                              
5: /home/scratch.ahendriksen_gpu/projects/cccl/libcudacxx/test/libcudacxx/cuda/memory_resource/memory_resource.resource_ref/resource_ref.conversion.pass.cpp:80:27: error: dereferencing type-punned pointer will break strict-aliasing rules [-Werror=strict-aliasing]
5: /home/scratch.ahendriksen_gpu/projects/cccl/libcudacxx/test/libcudacxx/cuda/memory_resource/memory_resource.resource_ref/resource_ref.conversion.pass.cpp:98:27: error: dereferencing type-punned pointer will break strict-aliasing rules [-Werror=strict-aliasing]
5:    98 |     const auto fake_orig = *reinterpret_cast<Fake_alloc_base*>(&ref_input);                                                               
5:       |                          ~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~                                                              
5: /home/scratch.ahendriksen_gpu/projects/cccl/libcudacxx/test/libcudacxx/cuda/memory_resource/memory_resource.resource_ref/resource_ref.conversion.pass.cpp:98:27: error: dereferencing type-punned pointer will break strict-aliasing rules [-Werror=strict-aliasing]
5: /home/scratch.ahendriksen_gpu/projects/cccl/libcudacxx/test/libcudacxx/cuda/memory_resource/memory_resource.resource_ref/resource_ref.conversion.pass.cpp:99:27: error: dereferencing type-punned pointer will break strict-aliasing rules [-Werror=strict-aliasing]
5:    99 |     const auto fake_conv = *reinterpret_cast<Fake_alloc_base*>(&ref);                                                 
5:       |                          ~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~                                                                  
5: /home/scratch.ahendriksen_gpu/projects/cccl/libcudacxx/test/libcudacxx/cuda/memory_resource/memory_resource.resource_ref/resource_ref.conversion.pass.cpp:99:27: error: dereferencing type-punned pointer will break strict-aliasing rules [-Werror=strict-aliasing]
5: /home/scratch.ahendriksen_gpu/projects/cccl/libcudacxx/test/libcudacxx/cuda/memory_resource/memory_resource.resource_ref/resource_ref.conversion.pass.cpp: In instantiation of ‘void test_conversion_from_resource_ref() [with PropA = property_with_value<short int>; PropB
= property_without_value<int>]’:                                                                                                           
5: /home/scratch.ahendriksen_gpu/projects/cccl/libcudacxx/test/libcudacxx/cuda/memory_resource/memory_resource.resource_ref/resource_ref.conversion.pass.cpp:157:194:   required from here
5: /home/scratch.ahendriksen_gpu/projects/cccl/libcudacxx/test/libcudacxx/cuda/memory_resource/memory_resource.resource_ref/resource_ref.conversion.pass.cpp:79:27: error: dereferencing type-punned pointer will break strict-aliasing rules [-Werror=strict-aliasing]
5:    79 |     const auto fake_orig = *reinterpret_cast<Fake_alloc_base*>(&ref_input);                                                                     
5:       |                          ~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~                                                          
5: /home/scratch.ahendriksen_gpu/projects/cccl/libcudacxx/test/libcudacxx/cuda/memory_resource/memory_resource.resource_ref/resource_ref.conversion.pass.cpp:79:27: error: dereferencing type-punned pointer will break strict-aliasing rules [-Werror=strict-aliasing]
5: /home/scratch.ahendriksen_gpu/projects/cccl/libcudacxx/test/libcudacxx/cuda/memory_resource/memory_resource.resource_ref/resource_ref.conversion.pass.cpp:80:27: error: dereferencing type-punned pointer will break strict-aliasing rules [-Werror=strict-aliasing]
5:    80 |     const auto fake_conv = *reinterpret_cast<Fake_alloc_base*>(&ref);                                                                      
5:       |                          ~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~                                                                           
5: /home/scratch.ahendriksen_gpu/projects/cccl/libcudacxx/test/libcudacxx/cuda/memory_resource/memory_resource.resource_ref/resource_ref.conversion.pass.cpp:80:27: error: dereferencing type-punned pointer will break strict-aliasing rules [-Werror=strict-aliasing]
5: /home/scratch.ahendriksen_gpu/projects/cccl/libcudacxx/test/libcudacxx/cuda/memory_resource/memory_resource.resource_ref/resource_ref.conversion.pass.cpp:98:27: error: dereferencing type-punned pointer will break strict-aliasing rules [-Werror=strict-aliasing]
5:    98 |     const auto fake_orig = *reinterpret_cast<Fake_alloc_base*>(&ref_input);                                                                
5:       |                          ~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~                                                             
5: /home/scratch.ahendriksen_gpu/projects/cccl/libcudacxx/test/libcudacxx/cuda/memory_resource/memory_resource.resource_ref/resource_ref.conversion.pass.cpp:98:27: error: dereferencing type-punned pointer will break strict-aliasing rules [-Werror=strict-aliasing]
5: /home/scratch.ahendriksen_gpu/projects/cccl/libcudacxx/test/libcudacxx/cuda/memory_resource/memory_resource.resource_ref/resource_ref.conversion.pass.cpp:99:27: error: dereferencing type-punned pointer will break strict-aliasing rules [-Werror=strict-aliasing]
5:    99 |     const auto fake_conv = *reinterpret_cast<Fake_alloc_base*>(&ref);                                                               
5:       |                          ~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~                                                      
5: /home/scratch.ahendriksen_gpu/projects/cccl/libcudacxx/test/libcudacxx/cuda/memory_resource/memory_resource.resource_ref/resource_ref.conversion.pass.cpp:99:27: error: dereferencing type-punned pointer will break strict-aliasing rules [-Werror=strict-aliasing]
5: /home/scratch.ahendriksen_gpu/projects/cccl/libcudacxx/test/libcudacxx/cuda/memory_resource/memory_resource.resource_ref/resource_ref.conversion.pass.cpp: In instantiation of ‘void test_conversion_from_resource_ref() [with PropA = property_without_value<short int>; Pro
pB = property_without_value<int>]’:                                                                                                    
5: /home/scratch.ahendriksen_gpu/projects/cccl/libcudacxx/test/libcudacxx/cuda/memory_resource/memory_resource.resource_ref/resource_ref.conversion.pass.cpp:157:297:   required from here
5: /home/scratch.ahendriksen_gpu/projects/cccl/libcudacxx/test/libcudacxx/cuda/memory_resource/memory_resource.resource_ref/resource_ref.conversion.pass.cpp:79:27: error: dereferencing type-punned pointer will break strict-aliasing rules [-Werror=strict-aliasing]
5:    79 |     const auto fake_orig = *reinterpret_cast<Fake_alloc_base*>(&ref_input);                                                                           
5:       |                          ~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~                                                

No idea what this error is about.

Timeouts:

5: UNRESOLVED: libcu++ :: heterogeneous/barrier_abi_v2.pass.cpp (212 of 1795)                                                                                                                                                                                                   
5: UNRESOLVED: libcu++ :: heterogeneous/barrier.pass.cpp (207 of 1795)                                                                                                                                                                                                          

"r"(__size),
"r"(static_cast<_CUDA_VSTD::uint32_t>(__cvta_generic_to_shared(::cuda::device::barrier_native_handle(__bar))))
: "memory");
NV_IF_TARGET(
Copy link
Contributor

Choose a reason for hiding this comment

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

Question: why is the NV_IF_TARGET necessary here? I would have expected

#if (!defined(__CUDA_MINIMUM_ARCH__)) || (defined(__CUDA_MINIMUM_ARCH__) && 900 <= __CUDA_MINIMUM_ARCH__)

to have made sure that the function is not available on older architectures?

Copy link
Member Author

Choose a reason for hiding this comment

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

It's still 'available' on host. So we need to prevent host from seeing it in case it is included in .cpp files.

// Consider adding debug asserts here.
return static_cast<_CUDA_VSTD::uint64_t>(__cvta_generic_to_global(__ptr));
NV_DISPATCH_TARGET(
NV_IS_DEVICE, (
Copy link
Contributor

Choose a reason for hiding this comment

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

Question: do we need NV_DISPATCH_TARGET in device functions as well?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I am also curious about that one

Copy link
Member Author

Choose a reason for hiding this comment

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

Yep, headers should be includable in host only TUs.

@miscco
Copy link
Collaborator

miscco commented Nov 10, 2023

@wmaxey I have pushed fixes for libcudacxx. Please retest them

@jrhemstad jrhemstad added the backport branch/2.3.x For backporting to the 2.3.x release branch label Nov 13, 2023
@wmaxey wmaxey force-pushed the fea/remove_libcudacxx_symlinks branch from f6e5462 to 4d1d613 Compare November 13, 2023 22:26
@wmaxey
Copy link
Member Author

wmaxey commented Nov 13, 2023

@wmaxey I have pushed fixes for libcudacxx. Please retest them

@miscco I had to remove fixes from this branch, we'll open them in another to keep it small.

On that note, I did forget to pull the changes, I really hope you have them as I never had them hit my object store.

@wmaxey wmaxey merged commit 0447ad7 into main Nov 14, 2023
517 checks passed
Copy link
Contributor

Backport failed for branch/2.3.x, because it was unable to cherry-pick the commit(s).

Please cherry-pick the changes locally.

git fetch origin branch/2.3.x
git worktree add -d .worktree/backport-1075-to-branch/2.3.x origin/branch/2.3.x
cd .worktree/backport-1075-to-branch/2.3.x
git checkout -b backport-1075-to-branch/2.3.x
ancref=$(git merge-base 9bbec0cc43170059360bc9483085412ddf8168a7 4d1d613126279ec31a0a8ec2fc9920f649138036)
git cherry-pick -x $ancref..4d1d613126279ec31a0a8ec2fc9920f649138036

@wmaxey wmaxey deleted the fea/remove_libcudacxx_symlinks branch November 14, 2023 01:06
jrhemstad pushed a commit that referenced this pull request Nov 14, 2023
* [backport] Cherry-pick PR #1075 to branch/2.3.x manually

* [backport] Move PTX tests that missed the symlink PR #1098
@jrhemstad jrhemstad removed the backport branch/2.3.x For backporting to the 2.3.x release branch label Nov 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

[FEA]: Remove symlinks from libcudacxx
4 participants