Project

General

Profile

Feature #2259

clang CUDA compilation

Added by Aleksei Iupinov about 2 years ago. Updated over 1 year ago.

Status:
Closed
Priority:
Normal
Category:
build system
Target version:
-
Difficulty:
uncategorized
Close

Description

Feels weird to open a tracking issue for something that has already been merged (8be6f1a, I3543469d9f0fda37c186ba8bb474980018bd5c54) and is only waiting to be covered by Jenkins.

Nevertheless, I have a side report - post-submit coverage of the commit created 338 new gcc warnings. Many of those seem to be related to kernel instances (no previous prototype for function 'nbnxn_kernel_ElecEwQSTab_VdwLJEwCombGeom_VF_prune_cuda'), but there is probably more.


Related issues

Related to GROMACS - Feature #2126: implement native CUDA support in CMakeNew

Associated revisions

Revision 1a244727 (diff)
Added by Szilárd Páll about 2 years ago

Avoid -Wmissing-prototypes warnings in clang CUDA builds

Fixes #2259 partially

Change-Id: Ib47530c89cf4b3af5c2b65cfcfa39b9dfe0148b4

Revision 170839af (diff)
Added by Szilárd Páll about 2 years ago

Fix pair list array assertion

Was caught by the clang CUDA build.

Refs #2259

Change-Id: I8633e48ea5c33225829f92f60c2c785f16e17aca

Revision a67a6b0e (diff)
Added by Aleksei Iupinov almost 2 years ago

Make gpu_utils-test build with GMX_CLANG_CUDA

Same workarounds are applied to libgpu_utilstest as for libgromacs.
Renamed ligbpu_utilstest target to gpu_utilstest_cuda to avoid the
double "lib" prefix in the filename.

Refs #2259, #2293

Change-Id: I16b07a13ce2dca30079a889e2b314483d82d3674

History

#2 Updated by Aleksei Iupinov about 2 years ago

Also, originally I was looking at some confusing warnings created by the texture changes as well. But those are a drop in this sea :-)

#3 Updated by Szilárd Páll about 2 years ago

Aleksei Iupinov wrote:

Feels weird to open a tracking issue for something that has already been merged (8be6f1a, I3543469d9f0fda37c186ba8bb474980018bd5c54) and is only waiting to be covered by Jenkins.

Nothing weird about it, redmine is here (also) to keep a record and manage issues.

Now whether this should be assigned to me by default, is something I'm less sure about. ;)
Some of the issues seem easy to resolve (where the function prototype is obviously incomplete), but I'm not sure what's wrong with the kernels.

#4 Updated by Szilárd Páll about 2 years ago

So the culprit is 2b9cbcb80c, but given the tricky compilation of CUDA code, I'm not sure if this is solvable at all for anything but the dummy kernel which, if a static modifier is added seems to be accepted. The other kernels however can't be static as they're called from another compilation unit.

#5 Updated by Gerrit Code Review Bot about 2 years ago

Gerrit received a related patchset '1' for Issue #2259.
Uploader: Szilárd Páll ()
Change-Id: gromacs~master~Ib47530c89cf4b3af5c2b65cfcfa39b9dfe0148b4
Gerrit URL: https://gerrit.gromacs.org/6998

#6 Updated by Roland Schulz about 2 years ago

But if they are in a different compilation unit why do they not have a declaration in a header?

#7 Updated by Gerrit Code Review Bot about 2 years ago

Gerrit received a related patchset '1' for Issue #2259.
Uploader: Szilárd Páll ()
Change-Id: gromacs~master~I8633e48ea5c33225829f92f60c2c785f16e17aca
Gerrit URL: https://gerrit.gromacs.org/6999

#8 Updated by Aleksei Iupinov about 2 years ago

Kernel flavors are generated from 1-2 headers by having other headers include them multiple times while defining relevant kernel flavor options. In other words, all the source is still in the CUDA headers, so I've just added static inline to the main kernel files, let's see if that works.

Also, I've spotted a define FUNCTION_DECLARATION_ONLY there, which I think should either be exploited or done away with?

#9 Updated by Mark Abraham about 2 years ago

I suggest avoiding calling a commit that enables a generally useful warning a "culprit." ;-) The CPU SIMD code has long declared its functions (see src/gromacs/mdlib/nbnxn_kernels/simd_4xn/nbnxn_kernel_simd_4xn.h etc.). I can imagine scenarios where we might be adding CUDA kernel flavours where it might be nice to get a compilation warning rather than a linking failure. I suggest we declare the kernels for the CUDA kernels, too.

And we should get better at remembering to try [JENKINS] Post-submit on the clang CUDA patch before final submission, too.

#10 Updated by Aleksei Iupinov about 2 years ago

More like "we should make Jenkins create Redmine issues on any newly seen post-submit failures" :-)

#11 Updated by Szilárd Páll about 2 years ago

  • Status changed from New to Resolved

#12 Updated by Szilárd Páll about 2 years ago

Aleksei Iupinov wrote:

More like "we should make Jenkins create Redmine issues on any newly seen post-submit failures" :-)

I agree, I'd prefer jenkins automatically filing an issue.

This is technically resolved, but somewhat related are new warnings emitted by some internal clang code-transformation getting unhappy about texrefs. We don't have an issue for that, do we?

#13 Updated by Mark Abraham about 2 years ago

Szilárd Páll wrote:

Aleksei Iupinov wrote:

More like "we should make Jenkins create Redmine issues on any newly seen post-submit failures" :-)

I agree, I'd prefer jenkins automatically filing an issue.

Shouldn't be too hard - hopefully choose a Jenkins stage to do an action upon newly occurring non-success that is run a script that can have a bot open an issue with a relevant link.

We must already have some bot for the Gerrit posting, but maybe we want a new bot account for this.

I don't know if Jenkins provides a way to notice a newly-occurring non-success.

Also, the script likely won't be able to notice a subsequent issue arising, unless maybe we can somehow notice jobs in the matrix changing status.

In order to improve the timeliness of fixes, the script still relies on people watching Redmine activity and choosing to respond. That would work for me, but for which other likely fixers is the convenience of a Redmine issue worth the time to test, implement, and maintain that posting script?

This is technically resolved, but somewhat related are new warnings emitted by some internal clang code-transformation getting unhappy about texrefs. We don't have an issue for that, do we?

Nobody made an issue, although two fixes have been proposed.

#14 Updated by Szilárd Páll about 2 years ago

Mark Abraham wrote:> Szilárd Páll wrote:

Aleksei Iupinov wrote:

More like "we should make Jenkins create Redmine issues on any newly seen post-submit failures" :-)

I agree, I'd prefer jenkins automatically filing an issue.

Shouldn't be too hard - hopefully choose a Jenkins stage to do an action upon newly occurring non-success that is run a script that can have a bot open an issue with a relevant link.

We must already have some bot for the Gerrit posting, but maybe we want a new bot account for this.

I don't know if Jenkins provides a way to notice a newly-occurring non-success.

The matrix jobs have, among the post-build actions, email notification capability -- which is what I have been trying to set up. I know Teemu pointed out that the matrix job's post-build actions do not make sense (? quoting freely), but admittedly don't get why is that not the correct approach.

In any case, AFAIK redmine can create issues from emails received or via the REST API. The email would be easy to set up if the matrix job email notifications would work (as well as the latter with jenkins-generated POST URLs) .

Also, the script likely won't be able to notice a subsequent issue arising, unless maybe we can somehow notice jobs in the matrix changing status.

That's true, so we'd like to detect at least blue -> yellow / red (and ideally also yellow / red -> "more" yellow / red?) transitions.

In order to improve the timeliness of fixes, the script still relies on people watching Redmine activity and choosing to respond. That would work for me, but for which other likely fixers is the convenience of a Redmine issue worth the time to test, implement, and maintain that posting script?

Timeliness is not my only concern. As I noted before, having a record of a post-submit regression (+updates on whether it's being looked at or worked on, as well as links to proposed fixes in gerrit) seems important and should ensure avoiding duplicate effort.

#15 Updated by Aleksei Iupinov almost 2 years ago

The clang_cuda postsubmit is now broken with introduction of libgpu_utilstest target in I9367d0f996de04c21312cef2081cc08148f80561

03:16:29 [  2%] Building NVCC (Device) object src/gromacs/gpu_utils/tests/CMakeFiles/libgpu_utilstest.dir/libgpu_utilstest_generated_devicetransfers.cu.o
03:16:29 nvcc warning : The 'compute_20', 'sm_20', and 'sm_21' architectures are deprecated, and may be removed in a future release (Use -Wno-deprecated-gpu-targets to suppress warning).
03:16:29 nvcc fatal   : The version ('40001') of the host compiler ('clang') is not supported

Another argument towards rolling own cuda_add_library/FindCUDA/whatever, to avoid duplicating crutches.
I'll see if I am capable of producing a quick fix.

#16 Updated by Gerrit Code Review Bot almost 2 years ago

Gerrit received a related patchset '4' for Issue #2259.
Uploader: Aleksei Iupinov ()
Change-Id: gromacs~master~I16b07a13ce2dca30079a889e2b314483d82d3674
Gerrit URL: https://gerrit.gromacs.org/7171

#17 Updated by Szilárd Páll almost 2 years ago

Aleksei Iupinov wrote:

The clang_cuda postsubmit is now broken with introduction of libgpu_utilstest target in I9367d0f996de04c21312cef2081cc08148f80561

[...]

Another argument towards rolling own cuda_add_library/FindCUDA/whatever, to avoid duplicating crutches.

Are you suggesting that we should add clang CUDA functionality into FindCUDA? I'm not sure that's maintenance-wise ideal.

#18 Updated by Aleksei Iupinov almost 2 years ago

No, just mentioning this is an option if we have to pile up hacks in one too many files.

#19 Updated by Mark Abraham almost 2 years ago

It'll be tempting to require cmake 3.8 for CUDA support for GROMACS 2019, and to use the native CUDA, to get rid of this nasty stuff. Hopefully Clang + CUDA works with that?

#20 Updated by Szilárd Páll almost 2 years ago

Mark Abraham wrote:

It'll be tempting to require cmake 3.8 for CUDA support for GROMACS 2019, and to use the native CUDA, to get rid of this nasty stuff.

Are you referring to having to create a static archive with the CUDA wrapper macros? I did not realize that was a big deal (other than having had to figure out the workaround)?

Hopefully Clang + CUDA works with that?

I don't think so and in any case there isn't a huge need, AFAICT. clang itself has native support so there it wouldn't add a lot (it may allow removing maybe 15-20 lines of our code).

#21 Updated by Mark Abraham almost 2 years ago

Szilárd Páll wrote:

Mark Abraham wrote:

It'll be tempting to require cmake 3.8 for CUDA support for GROMACS 2019, and to use the native CUDA, to get rid of this nasty stuff.

Are you referring to having to create a static archive with the CUDA wrapper macros?

Many of these debates center around coping with hackery in either FindCUDA or maybe our code. Native CUDA support should work more smoothly.

#22 Updated by Mark Abraham over 1 year ago

  • Related to Feature #2126: implement native CUDA support in CMake added

#23 Updated by Mark Abraham over 1 year ago

  • Status changed from Resolved to Closed

Also available in: Atom PDF