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

Improve compiler detection defines #1320

Merged
merged 8 commits into from
Feb 14, 2024

Conversation

Yaraslaut
Copy link
Contributor

@Yaraslaut Yaraslaut commented Jan 24, 2024

Description

closes #572

  • replace legacy defines for compilers with definitions for __cccl__confg such as _CCCL_COMPILER_[CLANG|GCC|MSVC ...

Checklist

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

Copy link

copy-pr-bot bot commented Jan 24, 2024

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@Yaraslaut Yaraslaut changed the title Improvement/compiler detection defines Improve compiler detection defines Jan 24, 2024
@Yaraslaut Yaraslaut marked this pull request as ready for review January 25, 2024 06:06
@Yaraslaut Yaraslaut requested review from a team as code owners January 25, 2024 06:06
cub/CMakeLists.txt Outdated Show resolved Hide resolved
cub/cmake/header_test.in Outdated Show resolved Hide resolved
cub/cub/util_compiler.cuh Outdated Show resolved Hide resolved
cub/cub/util_compiler.cuh Outdated Show resolved Hide resolved
cub/cub/util_compiler.cuh Outdated Show resolved Hide resolved
thrust/testing/reverse.cu Outdated Show resolved Hide resolved
thrust/testing/set_difference.cu Outdated Show resolved Hide resolved
thrust/thrust/detail/alignment.h Outdated Show resolved Hide resolved
thrust/thrust/detail/config/compiler.h Outdated Show resolved Hide resolved
thrust/thrust/detail/config/compiler.h Outdated Show resolved Hide resolved
@Yaraslaut Yaraslaut force-pushed the improvement/compiler_detection_defines branch from 3d6c6c4 to 9646d32 Compare January 25, 2024 17:00
@Yaraslaut Yaraslaut force-pushed the improvement/compiler_detection_defines branch from 9646d32 to b56c2af Compare January 25, 2024 17:16
@Yaraslaut
Copy link
Contributor Author

@miscco i updated PR

@Yaraslaut Yaraslaut requested a review from miscco January 25, 2024 17:16
cub/cub/util_compiler.cuh Outdated Show resolved Hide resolved
cub/cub/util_compiler.cuh Outdated Show resolved Hide resolved
cub/cub/util_compiler.cuh Outdated Show resolved Hide resolved
thrust/thrust/detail/static_assert.h Outdated Show resolved Hide resolved
@Yaraslaut Yaraslaut force-pushed the improvement/compiler_detection_defines branch from c17a2f5 to df32822 Compare January 31, 2024 09:20
@Yaraslaut Yaraslaut requested a review from miscco January 31, 2024 13:15
Copy link
Collaborator

@miscco miscco left a comment

Choose a reason for hiding this comment

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

This looks good. As this touches a lot of code I would like a second review from @allisonvacanti or @gevtushenko

cub/cub/util_compiler.cuh Outdated Show resolved Hide resolved
@miscco
Copy link
Collaborator

miscco commented Feb 1, 2024

/ok to test

@Yaraslaut
Copy link
Contributor Author

@miscco can you please rerun tests?

@miscco
Copy link
Collaborator

miscco commented Feb 1, 2024

/ok to test

@miscco
Copy link
Collaborator

miscco commented Feb 8, 2024

/ok to test

@miscco miscco self-requested a review February 8, 2024 13:32
@miscco miscco added feature request New feature or request. thrust For all items related to Thrust. cub For all items related to CUB libcu++ For all items related to libcu++ infrastructure Shared CMake, github, etc infrastructure labels Feb 8, 2024
Copy link
Collaborator

@alliepiper alliepiper left a comment

Choose a reason for hiding this comment

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

LGTM

@miscco miscco enabled auto-merge (squash) February 14, 2024 14:10
@miscco
Copy link
Collaborator

miscco commented Feb 14, 2024

/ok to test

@miscco miscco merged commit 0e9d834 into NVIDIA:main Feb 14, 2024
537 checks passed
dkolsen-pgi added a commit to dkolsen-pgi/cccl that referenced this pull request Feb 21, 2024
PR NVIDIA#1320 broke compilation with NVC++ when `<thrust/random.h>` was the
first header to be included.  This code that was there previously
```
    defined(_CCCL_COMPILER_CLANG) || \
    defined(_CCCL_COMPILER_ICC)
```
does not include `<stdint.h>` when NVC++ is the compiler, but the file
uses types defined in `<stdint.h>`, such as:
```
typedef ::int8_t   int8_t;
typedef ::int16_t  int16_t;
```

Fix this problem by using the same `#if` condition for including
`<stdint.h>` as for using the ``::intN_t` types.
miscco pushed a commit that referenced this pull request Feb 21, 2024
PR #1320 broke compilation with NVC++ when `<thrust/random.h>` was the
first header to be included.  This code that was there previously
```
    defined(_CCCL_COMPILER_CLANG) || \
    defined(_CCCL_COMPILER_ICC)
```
does not include `<stdint.h>` when NVC++ is the compiler, but the file
uses types defined in `<stdint.h>`, such as:
```
typedef ::int8_t   int8_t;
typedef ::int16_t  int16_t;
```

Fix this problem by using the same `#if` condition for including
`<stdint.h>` as for using the ``::intN_t` types.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cub For all items related to CUB feature request New feature or request. infrastructure Shared CMake, github, etc infrastructure libcu++ For all items related to libcu++ thrust For all items related to Thrust.
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

[FEA]: Consolidate logic for compiler detection
3 participants