NVIDIA / cccl

CUDA Core Compute Libraries
https://nvidia.github.io/cccl/
Other
1.2k stars 148 forks source link

[BUG]: incompatible redefinition of macro "__ELF__" with NVC++ host compiler #1995

Open dkolsen-pgi opened 3 months ago

dkolsen-pgi commented 3 months ago

Is this a duplicate?

Type of Bug

Something else

Component

libcu++

Describe the bug

When NVC++ 24.7 (soon to be released) or dev is the host compiler for NVCC and a CCCL header is included (probably not every header, but a good number of them), then NVC++ will issue a warning about the macro __ELF__ being redefined while it is doing its preprocessing.

$ cat elf.cpp
#include <cuda/std/type_traits>
int main() { }
$ /proj/cuda/12.5/Linux_x86_64/bin/nvcc -ccbin /proj/nv/Linux_x86_64/24.7/compilers/bin/nvc++ -x cu elf.cpp
"/proj/cuda/12.5/Linux_x86_64/bin/../targets/x86_64-linux/include/cuda/std/detail/libcxx/include/__config", line 130: warning: incompatible redefinition of macro "__ELF__" [bad_macro_redef]
      #define __ELF__
              ^

Remark: individual warnings can be suppressed with "--diag_suppress <warning-name>"

"/proj/cuda/12.5/Linux_x86_64/bin/../targets/x86_64-linux/include/cuda/std/detail/libcxx/include/__config", line 130: warning: incompatible redefinition of macro "__ELF__" [bad_macro_redef]
      #define __ELF__
              ^

Remark: individual warnings can be suppressed with "--diag_suppress <warning-name>"

In HPC SDK 24.7, we fixed an NVC++ bug where __ELF__ is not defined. Now __ELF__ is always predefined by the compiler as 1 on all Linux targets. The CCCL header <cuda/std/detail/libcxx/include/__config> defines __ELF__ as empty when the compiler is NVC++ or NVRTC.

I am guessing, but haven't confirmed, that the system header pragmas are ignored when NVC++ is doing preprocessing only. That's why we see this preprocessor-related warning when NVC++ is the host compiler for NVCC, but not with nvc++ -stdpar.

This problems starts with NVC++ 24.7. It doesn't show up with older versions of NVC++. It shows up with all versions of NVCC, back to at least CUDA 11.0.

How to Reproduce

Compile a file that includes some CCCL headers with NVCC with NVC++ 24.7 as the host compiler.

Expected behavior

CCCL should be fixed to avoid the warning. (Even though a change to NVC++ triggered the warning, NVC++ isn't doing anything wrong. The incorrect code is in CCCL.)

This block of code in cuda/std/detail/libcxx/include/__config:

#if (defined(_CCCL_COMPILER_NVHPC) && defined(__linux__)) \
 || defined(_CCCL_COMPILER_NVRTC)
    #define __ELF__
#endif

can be changed to any of these:

#if !defined(__ELF__) && \
  (defined(_CCCL_COMPILER_NVHPC) && defined(__linux__)) \
  || defined(_CCCL_COMPILER_NVRTC)
    #define __ELF__
#endif
#if (defined(_CCCL_COMPILER_NVHPC) && defined(__linux__)) \
 || defined(_CCCL_COMPILER_NVRTC)
    #undef __ELF__
    #define __ELF__
#endif
#if (defined(_CCCL_COMPILER_NVHPC) && defined(__linux__)) \
 || defined(_CCCL_COMPILER_NVRTC)
    #define __ELF__ 1
#endif

I believe that any of those changes should work. I think combining 1 and 3 works best:

#if !defined(__ELF__) && \
 (defined(_CCCL_COMPILER_NVHPC) && defined(__linux__)) \
 || defined(_CCCL_COMPILER_NVRTC)
    #define __ELF__ 1
#endif

because it is the safest change (checking that __ELF__ is not defined before defining it) and it defines __ELF__ correctly (1 instead of empty).

Reproduction link

No response

Operating System

Linux

nvidia-smi output

No response

NVCC version

Any NVCC version. It is the NVC++ version that makes a difference.

bernhardmgruber commented 3 months ago

The #define __ELF__ seemed to have been introduced in 2939fce44a4f22a81c976f88734c9115c5cedb64, maybe @griwes can help here.

griwes commented 3 months ago

That was work to make libunwind and libcxxabi compile with nvc++, so the define should be safe to drop entirely if it is now causing trouble; we'll see if anything unexpected depends on it in CI.

dkolsen-pgi commented 3 months ago

Older versions of NVC++ don't define __ELF__, and we want to give users the freedom to mix and match NVCC and NVC++ versions (within reason). Unless the reason for needing __ELF__ to be defined no longer applies, the define shouldn't be dropped entirely.

miscco commented 2 months ago

I believe we should keep the definition in place when necessary.

I have opened #2018 to only define the macro when it is not present.

That retains the current behavior and lets us play nice with all nvc++ versions