ianhinder / Kranc

A Mathematica package for generating code for solving time dependent partial differential equations
http://kranccode.org
GNU General Public License v2.0
28 stars 10 forks source link

'restrict' not always available #62

Closed knarrff closed 11 years ago

knarrff commented 12 years ago

GenericFD/src/GenericFD.h contains 'restrict' keywords which prevent compilation when not known to the compiler. They are redefined for C++ code and Cactus compilation, but this fails for CUDA compilation.

ianhinder commented 12 years ago

I compile CUDA files which include GenericFD.h using nvcc, and I don't get any complaints about the "restrict" keyword. What version of nvcc are you using? It's also possible that some CaKernel headers that I include have #defined restrict away, but I don't think that is the case.

ianhinder commented 12 years ago

I now run into this problem as well.

knarrff commented 12 years ago

What about #ifdef CCTK_RESTRICT #define restrict CCTK_RESTRICT #endif?

ianhinder commented 12 years ago

It successfully compiled with

CUCC_EXTRA_FLAGS = --compiler-options "-D restrict=__restrict"

nvcc calls gcc to compile C++ code. There is no "restrict" keyword in C++, but __restrict can be used with GCC when compiling C++.

eschnett commented 12 years ago

The Cactus autoconf stage examines the C and the C++ compiler, but does not examine the CUDA compiler. This means that one has to manually specify good CUDA options.

In addition, it may be that cctk_Config.h (which doesn't know about CUDA) mis-interprets CUDA as C instead of C++, which is probably a closer match.

This is a Cactus problem, not a Kranc problem.

knarrff commented 12 years ago

On Thu, May 03, 2012 at 07:14:54AM -0700, Erik Schnetter wrote:

The Cactus autoconf stage examines the C and the C++ compiler, but does not examine the CUDA compiler. This means that one has to manually specify good CUDA options.

In addition, it may be that cctk_Config.h (which doesn't know about CUDA) mis-interprets CUDA as C instead of C++, which is probably a closer match.

This is a Cactus problem, not a Kranc problem.

Ian and me arrived at the same conclusion this morning. Cactus should provide a better configuration for CUDA. However, for the moment the workaround is probably ok.

Frank

knarrff commented 12 years ago

On Thu, May 03, 2012 at 07:14:54AM -0700, Erik Schnetter wrote:

This is a Cactus problem, not a Kranc problem.

However: Kranc contains code which, if not compiled by the Cactus and C++ assumes 'restrict' being present, which is not always the case. Cactus may or not contain the logic to test to the keyword, but Kranc doesn't use it for non-C++ code at the moment.

Frank

eschnett commented 12 years ago

Kranc should use the restrict qualifier even for CUDA kernels. In fact, the CaKernel infrastructure uses restrict as well.

knarrff commented 12 years ago

On Thu, May 03, 2012 at 08:35:35AM -0700, Erik Schnetter wrote:

Kranc should use the restrict qualifier even for CUDA kernels. In fact, the CaKernel infrastructure uses restrict as well.

I agree. Could Kranc rely on Cactus macros for restrict? Then Cactus' job would be to declare them correctly for C, C++ and CUDA, separately.

Frank

eschnett commented 12 years ago

Yes, it should. Kranc should then use CCTK_RESTRICT instead of restrict.

-erik

On Thu, May 3, 2012 at 11:39 AM, Frank Löffler reply@reply.github.com wrote:

On Thu, May 03, 2012 at 08:35:35AM -0700, Erik Schnetter wrote:

Kranc should use the restrict qualifier even for CUDA kernels. In fact, the CaKernel infrastructure uses restrict as well.

I agree. Could Kranc rely on Cactus macros for restrict? Then Cactus' job would be to declare them correctly for C, C++ and CUDA, separately.

Frank


Reply to this email directly or view it on GitHub: https://github.com/ianhinder/Kranc/issues/62#issuecomment-5490554

Erik Schnetter schnetter@gmail.com http://www.perimeterinstitute.ca/personal/eschnetter/

ianhinder commented 12 years ago

As far as I can tell, restrict is not a C++ keyword, at least it is not in GCC (http://gcc.gnu.org/onlinedocs/gcc/Restricted-Pointers.html). Since GenericFD.h must be included from files compiled by a C++ compiler, we cannot use the restrict keyword there unless we define it ourselves. Since Cactus provides CCTK_RESTRICT, which is presumably defined to be "restrict if supported, else nothing", one option would be to use it. However, this is ugly. I note that Carpet also uses "restrict", even though it is not in the C++ standard. Does it define it itself? We should be consistent, and do the same thing in Kranc and Carpet. We should also modify CaKernel to do the same thing.

eschnett commented 12 years ago

For a long time, "inline" was not C keyword, but Cactus (and many other programs) defined it sensibly via autoconf. I suggest to do the same with "restrict".

In Carpet, I define "restrict" depending on the autoconf findings. The flesh does not do so (citing potential problems if someone wanted to use a variable called "restrict") and only defines CCTK_RESTRICT. Personally, I find this overly cautious.

We should defined "restrict" in the flesh; the code is there (cctk_Config.h.in) but commented out. If not, we have to use CCTK_RESTRICT and live with the ugliness.

The "restrict" keyword is the reason that C and C++ are these days as fast as Fortran. Without it, compilers cannot prove that pointer targets do not alias, and have to add additional load/store instructions and have to avoid certain optimisations.

ianhinder commented 12 years ago

Does the flesh pass in grid function pointers with this keyword, or do you have to assert this yourself in the code?

I agree that we should define restrict in the flesh - that seems like a sensible thing to do in an HPC framework. Do you want to create a ticket for this?

knarrff commented 12 years ago

https://trac.einsteintoolkit.org/ticket/875

eschnett commented 12 years ago

Ian: Yes, the flesh uses CCTK_RESTRICT in CCTK_ARGUMENTS.

eschnett commented 12 years ago

I believe the flesh now #defines restrict in both C and C++.

Can this ticket be closed?

ianhinder commented 11 years ago

According to ticket https://trac.einsteintoolkit.org/ticket/875, this has not been fixed for CUDA. There is another ticket, https://trac.einsteintoolkit.org/ticket/891, about adding support for CUDA as well, and as far as I know, this has not been implemented. However, the decision about what to do has been made, and I don't see this as a Kranc issue any more, so I am closing this ticket.