PR for llvm/llvm-project#57544

[llvmbot]

resolves llvm/llvm-project#57544

[tru]

@yxsamliu should this be backported to 17.x?

[tru]

cc @AaronBallman @cor3ntin @erichkeane since it touches clang

[AaronBallman]

Why is this the correct fix instead of messing with __noinline__? We already document __noinline__ as a CUDA keyword intended to avoid diagnostics with __attribute__((__noinline__)), so this change seems questionable on its face.

[yxsamliu]

Why is this the correct fix instead of messing with __noinline__? We already document __noinline__ as a CUDA keyword intended to avoid diagnostics with __attribute__((__noinline__)), so this change seems questionable on its face.

I think this is to workaround nvcc header which defines __noinline__ in a way that clang cannot handle.

@Artem-B

[AaronBallman]

Why is this the correct fix instead of messing with __noinline__? We already document __noinline__ as a CUDA keyword intended to avoid diagnostics with __attribute__((__noinline__)), so this change seems questionable on its face.

I think this is to workaround nvcc header which defines __noinline__ in a way that clang cannot handle.

@Artem-B

As best I could tell from the original patch discussion, I think that's correct, but it seems this has been a long-standing NVCC bug that should be resolved in their headers rather than expecting compilers to work around it, yes? CC @erichkeane since this is attributes related.

[erichkeane]

I don't have a good solution here, it seems that this is the NVCC headers leaking the definition unfortunately, and I've reached out to the CUDA team at NVIDIA to see if this is something that could be fixed in the future. However, that obviously doesn't fix the past, so I suspect we'll have to put this in anyway.

[AaronBallman]

I don't have a good solution here, it seems that this is the NVCC headers leaking the definition unfortunately, and I've reached out to the CUDA team at NVIDIA to see if this is something that could be fixed in the future. However, that obviously doesn't fix the past, so I suspect we'll have to put this in anyway.

Thanks for reaching out! And yeah, you're right, I think we're going to need this regardless.

[AaronBallman]

LGTM

[yxsamliu]

@yxsamliu should this be backported to 17.x?

LGTM

[Artem-B]

I think we're going to need this regardless.

That's, unfortunately, true.

Fun tidbit. NVCC itself does run into this problem and apparently deals with this by un-doing this mess. When it sees __attribute__((__attribute__(("noinline")))) it transforms it back to __attribute__(("noinline")).

[Artem-B]

For the reference: NVIDIA/thrust#1703 (comment)