hliao marked an inline comment as done. hliao added inline comments.
================ Comment at: clang/lib/Headers/__clang_cuda_runtime_wrapper.h:82-94 #undef __CUDACC__ #if CUDA_VERSION < 9000 #define __CUDABE__ #else +#define __CUDACC__ #define __CUDA_LIBDEVICE__ #endif ---------------- tra wrote: > hliao wrote: > > hliao wrote: > > > tra wrote: > > > > hliao wrote: > > > > > tra wrote: > > > > > > Please add comments on why __CUDACC__ is needed for driver_types.h > > > > > > here? AFAICT, driver_types.h does not have any conditionals that > > > > > > depend on __CUDACC__. What happens if it's not defined. > > > > > > > > > > > > > > > > > `driver_types.h` includes `host_defines.h`, where macros > > > > > `__device_builtin_surface_type__` and > > > > > `__device_builtin_texture_type__` are conditional defined if > > > > > `__CUDACC__`. > > > > > > > > > > The following is extracted from `cuda/crt/host_defines.h` > > > > > > > > > > ``` > > > > > #if !defined(__CUDACC__) > > > > > #define __device_builtin__ > > > > > #define __device_builtin_texture_type__ > > > > > #define __device_builtin_surface_type__ > > > > > #define __cudart_builtin__ > > > > > #else /* defined(__CUDACC__) */ > > > > > #define __device_builtin__ \ > > > > > __location__(device_builtin) > > > > > #define __device_builtin_texture_type__ \ > > > > > __location__(device_builtin_texture_type) > > > > > #define __device_builtin_surface_type__ \ > > > > > __location__(device_builtin_surface_type) > > > > > #define __cudart_builtin__ \ > > > > > __location__(cudart_builtin) > > > > > #endif /* !defined(__CUDACC__) */ > > > > > ``` > > > > My concern is -- what else is going to get defined? There are ~60 > > > > references to __CUDACC__ in CUDA-10.1 headers. The wrappers are fragile > > > > enough that there's a good chance something may break. It does not help > > > > that my CUDA build bot decided to die just after we switched to > > > > work-from-home, so there will be no early warning if something goes > > > > wrong. > > > > > > > > If all we need are the macros above, we may just define them. > > > Let me check all CUDA SDK through their dockers. Redefining sounds good > > > me as wll. > > I checked headers from 7.0 to 10.0, `__device_builtin_texture_type__` and > > `__builtin_builtin_surface_type__` are only defined with that attributes if > > `__CUDACC__` is defined. As we only pre-define `__CUDA_ARCH__` in clang but > > flip `__CUDACC__` on and off in the wrapper headers to selectively reuse > > CUDA's headers. I would hear your suggestion on that. > > BTW, macros like `__device__` are defined regardless of `__CUDACC__` from > > 7.0 to 10.0 as `__location(device)`. `__location__` is defined if > > `__CUDACC__` is present. But, different from `__device__`, > > `__device_builtin_texture_type__` is defined only `__CUDACC__` is defined. > `__device_builtin_texture_type__` is defined in `host_defines.h`, which does > not seem to include any other files or does anything suspicious with > `__CUDACC__` > > It may be OK to move inclusion of `host_defines.h` to the point before > `driver_types.h`, which happens to include the host_defines.h first, and > define __CUDACC__ only around `host_defines.h`. > > An alternative is to add the macros just after inclusion of `host_defines.h` > > In either case please verify that these attributes are the only things that's > changed by diffing output of `clang++ -x cuda /dev/null --cuda-host-only -dD > -E -o -` before and after the change. With `__CUDACC__`, the only difference is the additional attributes added, such as `device_builtin_texture_type`. Attributes like `cudart_builtin` are also defined correctly. That should be used to start the support CUDART features. I revised the change to include `host_defines.h` first and found there's no changes from the one using `driver_types.h`. We should be OK for that change. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76365/new/ https://reviews.llvm.org/D76365 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
