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

Add tests for host only TUs and fix several found issues. #340

Merged
merged 18 commits into from
Dec 13, 2022

Conversation

wmaxey
Copy link
Member

@wmaxey wmaxey commented Dec 8, 2022

No description provided.

@wmaxey wmaxey requested review from griwes, jrhemstad and miscco December 8, 2022 21:52
@@ -9,6 +9,10 @@
#ifndef _CUDA_ARRAY
#define _CUDA_ARRAY

#ifndef __CUDACC_RTC__
# include <cstdlib>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Out of curiosity, what happens when using NVRTC with things like this? Will the functionality that depends on stuff from <cstdlib> just not work?

Copy link
Member Author

Choose a reason for hiding this comment

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

Usually NVRTC defines its own equivalent. It's all just __device__ only.

ROOT_IMAGE: "nvcr.io/nvidia/cuda:11.8.0-devel-ubuntu18.04"
COMPILERS: "g++-5 g++-6 g++-7 clang-7"
COMPILERS: "g++-6 g++-7 clang-7"
Copy link
Collaborator

Choose a reason for hiding this comment

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

AFAIK we settled on gcc-7 as the minimal supported version.

Do we want to drop gcc-6 while we are at it?

Copy link
Member Author

Choose a reason for hiding this comment

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

Will consider it dropping later in another patch. In tandem with #341 would be good timing.

@@ -9,19 +9,19 @@
#ifndef _CUDA_CMATH
#define _CUDA_CMATH

#ifndef __CUDACC_RTC__
#include <math.h>
#include <cmath>
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 considerably scared that we need both of them

Technically one should suffice

Copy link
Member Author

@wmaxey wmaxey Dec 9, 2022

Choose a reason for hiding this comment

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

Sorry, didn't mean to leave it in, we do only need host math.h.

@wmaxey wmaxey merged commit e8dc6b8 into main Dec 13, 2022
@wmaxey wmaxey deleted the enhancement/host_only_tests branch December 13, 2022 18:54
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants