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

[BUG]: cuda::std::variant fails to compile w/ clang & libc++ #2724

Open
1 task done
Artem-B opened this issue Nov 6, 2024 · 6 comments
Open
1 task done

[BUG]: cuda::std::variant fails to compile w/ clang & libc++ #2724

Artem-B opened this issue Nov 6, 2024 · 6 comments
Labels
bug Something isn't working right.

Comments

@Artem-B
Copy link
Contributor

Artem-B commented Nov 6, 2024

Is this a duplicate?

Type of Bug

Compile-time Error

Component

libcu++

Describe the bug

emplace_index_init_list_args.pass.cpp fails to compile w/ clang & libc++

It's not clear if the issue is with the test or the way libcu++ interacts w/ the standard library.

How to Reproduce

Reduced reproducer based on the emplace_index_init_list_args.pass.cpp test: https://godbolt.org/z/Yqf66GjEP

Compiles OK with NVCC, and w/ clang and libstdc++. Fails with ambiguous overload resolution with libc++.

Expected behavior

cuda::std::variant should be compileable with both libc++ and libstdc++.

Reproduction link

https://godbolt.org/z/Yqf66GjEP

Operating System

No response

nvidia-smi output

No response

NVCC version

No response

@Artem-B Artem-B added the bug Something isn't working right. label Nov 6, 2024
@miscco
Copy link
Collaborator

miscco commented Nov 7, 2024

This is unfortunately again the standard messing with us.

The standard requires us to go through ::std::construct_at when constructing something during constant evaluation. This is obviously problematic, especially given that libc++ does not properly qualify their function calls (Which is technically correct, because we are using a reserved name which is fully within their purview)

With nvcc we are currently working towards the compiler recognizing a special function ::cuda::std::__device_construct_at to be used on device during constant evaluation. Would that be something clang-cuda could also recognize as an extension to the standard?

@Artem-B
Copy link
Contributor Author

Artem-B commented Nov 7, 2024

Thank you for looking at those issues.

Is there something specific that could be done by libc++ or the standard library headers wrappers we ship with clang?

Clang's header level workaround might be the easiest to implement, but will need, by design, to depend on libc++ implementation details. It may be manageable (and would not be the first time we had to do that), but it's not ideal.

libc++ changes may be more robust, but would need a really good arguments to support them, and I'm not versed in the standard-ese well enough for that. It would help a lot to get libcu++ and libc++ folks to discuss it directly.

If you could outline what needs to be done, I can get the ball rolling on LLVM side.

On a side note, the issue like this makes me wonder if you may already have a laundry list of things in libc++ you'd like to fix/amend/change. I'd be curious to take a peek if such a list exists. I usually run into the issues like these on the tactical level, trying to compile things, and don't have a good big picture. Having some sort of roadmap with "here there be dragons (pun intended)" would help triaging the sharp corners I happen to run into with libcu++/clang/libc++ builds.

@miscco
Copy link
Collaborator

miscco commented Nov 7, 2024

I believe technically we (libcu++) would need to change our naming convention.

From any point of view the standard library is totally within its rights that if they use a function with a reserved name they should not need to qualify it.

I do know that MSVC STL does it, but that is a project wide decision and libc++ just recently dropped their uses of _LIBCPP_VSTD so I am highly doubtful that they would be willing to slam it all over their library again just to support our forbidden behavior.

Note that due to our project structure libc++ would need to fully qualify all calls via ::std:: to avoid any potential clashes

@miscco
Copy link
Collaborator

miscco commented Nov 7, 2024

Regarding the issues we are facing, then everything that is standard mandated to be within namespace std is a big issue.

This involves (not exhaustive):

  • operator<=>
  • construct_at / destruct_at
  • source_location
  • structured bindings (that we did solve)

@Artem-B
Copy link
Contributor Author

Artem-B commented Nov 9, 2024

Note that due to our project structure libc++ would need to fully qualify all calls via ::std:: to avoid any potential clashes

OK. I can see how that might be something that may be hard to convince libc++ about.

On a positive side, most things appear to work (for now, at least), so while it does sound like this may be a recurring issue going forward, we may be able to deal with it on a case by case basis.

@miscco
Copy link
Collaborator

miscco commented Nov 9, 2024

Yeah this is something we can do relatively easy, we just need to rename our internal helper from __construct_at to maybe __cccl_construct_at

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working right.
Projects
None yet
Development

No branches or pull requests

2 participants