Skip to content
This repository has been archived by the owner on Mar 21, 2024. It is now read-only.

Fix NVCC test failures involving long double #165

Closed
wants to merge 1 commit into from

Conversation

wmaxey
Copy link
Member

@wmaxey wmaxey commented May 17, 2021

This patch fixes NVCC's complaints about using long double with some type checks:

//    Make sure the types are right
    static_assert ( cuda::std::is_same<decltype( 3h   ), cuda::std::chrono::hours>::value, "" );
    static_assert ( cuda::std::is_same<decltype( 3min ), cuda::std::chrono::minutes>::value, "" );
    static_assert ( cuda::std::is_same<decltype( 3s   ), cuda::std::chrono::seconds>::value, "" );
    static_assert ( cuda::std::is_same<decltype( 3ms  ), cuda::std::chrono::milliseconds>::value, "" );
    static_assert ( cuda::std::is_same<decltype( 3us  ), cuda::std::chrono::microseconds>::value, "" );
    static_assert ( cuda::std::is_same<decltype( 3ns  ), cuda::std::chrono::nanoseconds>::value, "" );

Original error:

2021-05-17 19:15:51+00:00| Error #20208-D: 'long double' is treated as 'double' in device code
2021-05-17 19:15:51+00:00| 
2021-05-17 19:15:51+00:00| Error #20208-D: 'long double' is treated as 'double' in device code
2021-05-17 19:15:51+00:00| 
2021-05-17 19:15:51+00:00| Error #20208-D: 'long double' is treated as 'double' in device code
2021-05-17 19:15:51+00:00| 
2021-05-17 19:15:51+00:00| Error #20208-D: 'long double' is treated as 'double' in device code
2021-05-17 19:15:51+00:00| 
2021-05-17 19:15:51+00:00| Error #20208-D: 'long double' is treated as 'double' in device code
2021-05-17 19:15:51+00:00| 
2021-05-17 19:15:51+00:00| Error #20208-D: 'long double' is treated as 'double' in device code
2021-05-17 19:15:51+00:00| 
2021-05-17 19:15:51+00:00| 6 errors detected in the compilation of "/sw/gpgpu/libcudacxx/.upstream-tests/test/std/utilities/time/time.duration/time.duration.literals/literals2.pass.cpp".
2021-05-17 19:15:51+00:00| # --error 0x1 --

According to the draft for chrono literals, the duration representation is unspecified, so this should be fine, and makes NVCC happy.
http://eel.is/c++draft/time.duration.literals

@wmaxey wmaxey requested review from griwes and jrhemstad May 17, 2021 23:11
@griwes
Copy link
Collaborator

griwes commented May 17, 2021

@brycelelbach this is an abi break, but probably one we don't care about. Do you agree with this assessment?

@brycelelbach
Copy link
Collaborator

It doesn't seem like a big deal, but it is an ABI break. We are reving the ABI in 2.0.0 for complex anyways, so we might as well hold this off until then.

So I would:

  • First, disable the failing parts of the test, then integrate that to 1.6.0.
  • Then, I would fix it by making these double instead of long double and target that for 2.0.0

But keep in mind user defined literals must take a long double.

@brycelelbach brycelelbach added this to the 2.0.0 milestone May 18, 2021
@brycelelbach
Copy link
Collaborator

This looks good.

@brycelelbach
Copy link
Collaborator

Actually, maybe we move the ABI break from 2.0.0 to 1.6.0.

@brycelelbach brycelelbach modified the milestones: 2.0.0, 1.6.0 May 18, 2021
@brycelelbach brycelelbach added P0: must have Absolutely must ship with the milestone. bug: functional Does not work as intended. labels May 18, 2021
@wmaxey
Copy link
Member Author

wmaxey commented Jul 8, 2021

Closing as this change is included in #172.

@wmaxey wmaxey closed this Jul 8, 2021
@miscco miscco deleted the bugfix/chrono_literals branch May 23, 2023 06:35
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug: functional Does not work as intended. P0: must have Absolutely must ship with the milestone.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants