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

Compatibility issues with NVIDIA compilers. #6

Open
olupton opened this issue Jan 17, 2022 · 6 comments
Open

Compatibility issues with NVIDIA compilers. #6

olupton opened this issue Jan 17, 2022 · 6 comments
Assignees
Labels
build build system and portability

Comments

@olupton
Copy link

olupton commented Jan 17, 2022

We have recently noticed two issues with Random123 compatibility with the NVIDIA HPC SDK compilers (formerly PGI).

The first issue is that recent versions (21.11+) of nvc and nvc++ are more enthusiastic about detecting and enabling support for ABM instructions (see: https://forums.developer.nvidia.com/t/21-11-tp-behaviour/198115). This is problematic because Random123 assumes that if ABM/__ABM__ is enabled then intrin.h will be available, which is a poor assumption:

#ifndef R123_USE_INTRIN_H
#ifdef __ABM__
#define R123_USE_INTRIN_H 1
#else
#define R123_USE_INTRIN_H 0
#endif
#endif

This is not correct both on our systems and those of some of our users, and leads to errors such as

"include/Random123/features/sse.h", line 56: catastrophic error #1697: cannot open source file "intrin.h"
  #include <intrin.h>
                     ^

A second issue is that recent versions of nvc and nvc++ support a -cuda option, which is required when compiling for certain types of GPU offload. When invoked in this mode, the compilers define the __CUDACC__ macro, which means they are detected as nvcc instead of PGI-like compilers:

#elif defined(__CUDACC__)
#include "nvccfeatures.h"
#elif defined(__ICC)
#include "iccfeatures.h"
#elif defined(__xlC__) || defined(__ibmxl__)
#include "xlcfeatures.h"
#elif defined(__PGI)
#include "pgccfeatures.h"

In our case this actually sidesteps the first issue, but that appears to just be by accident. Is is less clear if this is really a bug, but it appears to be unintentional behaviour.

(See BlueBrain/CoreNeuron#724 for more information.)

@markmoraes
Copy link

For the first one, does compiling with -DR123_USE_INTRIN_H=0 resolve the problem? I'm pretty sure that ABM change was contributed, will check when I have a moment.

For the second, I suppose I could move the __PGI ifdef earlier, or the CUDACC test later, but this feels fragile. It may be better for me to add checks for explicit definition first (e.g. if defined(R123_USE_NVCC_FEATURES) so that one can avoid the autodetection attempts and find a way through the feature maze.

I'll see if I can set up an nvc/nvc++ test environment.

@markmoraes markmoraes self-assigned this Jan 17, 2022
@markmoraes markmoraes added bug Something isn't working build build system and portability and removed bug Something isn't working labels Jan 17, 2022
@olupton
Copy link
Author

olupton commented Jan 18, 2022

Thanks for taking a look! Yes, for the first one then -DR123_USE_INTRIN_H=0 avoids the problem.

@markmoraes
Copy link

featuresdiff.txt

Does this patch fix the second issue? (autodetection should work, but if you would also care to test with -DR123_USE_AUTOFEATURES=0 -DR123_PGCCFEATURES and ensure that works, that would be helpful in verifying that the future escape route is also available to you e.g. if a future nvc decides to define OPENCL_VERSION, for example

@olupton
Copy link
Author

olupton commented Jan 19, 2022

With that patch on top of main then I confirm that nvc++ -cuda follows the PGI/NVHPC features path, so I run into the first issue as expected. -DR123_USE_AUTOFEATURES=0 -DR123_PGCCFEATURES takes me down the same route.

(and in these tests, -DR123_USE_INTRIN_H=0 still seems to work as a solution to the first problem)

@markmoraes
Copy link

Sounds good, thanks for testing it.

And just to check I'm understanding this correctly, is it satisfactory thatt nvc++ -cuda takes you down the PGI/NVHPC path (i.e. pgfeatures.h)? Wondering if there's also a need for a new nvhpcfeatures.h that combines the old pgfeatures.h as well as cuda definitions?

I also see that we're including various compilers for host code in nvcc features (for host code), so I might need to add the protective R123_USE_AUTOFEATURES checks there too, once I figure out what was actually going on in that particular maze.

@olupton
Copy link
Author

olupton commented Jan 19, 2022

My understanding is that even with -cuda then nvc++ is more like pgc++ than nvcc, so I think that the patch is a step in the right direction.

So far I have not come across any issues that require a mix of PGI/CUDA features for nvc++ -cuda, but I don't say that with a lot of confidence.

Historically we have compiled wrappers around Random123 methods using nvcc/CUDA and then called those from device kernels generated with nvc++/OpenACC, but we are now looking at using Random123 directly in device code using nvc++/OpenMP, so we may uncover something new.

(For what it's worth, we always use gcc as the host compiler for nvcc, even though I understand it is possible to use nvc++ for some combinations of versions.)

@fennm fennm closed this as completed Apr 20, 2023
@fennm fennm reopened this Apr 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build build system and portability
Projects
None yet
Development

No branches or pull requests

3 participants