Page MenuHomePhabricator

[CUDA][Windows] Partial fix for bug 38811 (Step 2 of 3)
ClosedPublic

Authored by emankov on Fri, Mar 15, 10:42 AM.

Details

Summary

Partial fix for the clang Bug 38811 "Clang fails to compile with CUDA-9.x on Windows".

[Synopsis]
__sptr is a new Microsoft specific modifier.

[Solution]
Replace all __sptr occurrences with ___sptr to eliminate the below clang compilation error on Windows.

In file included from C:\GIT\LLVM\trunk-for-submits\llvm-64-release-vs2017-15.9.5\dist\lib\clang\9.0.0\include\__clang_cuda_runtime_wrapper.h:162:
C:\GIT\LLVM\trunk-for-submits\llvm-64-release-vs2017-15.9.5\dist\lib\clang\9.0.0\include\__clang_cuda_device_functions.h:524:33: error: expected expression
  return __nv_fast_sincosf(__a, __sptr, __cptr);
                                ^

[How to repro]

clang++.exe -x cuda "c:\ProgramData\NVIDIA Corporation\CUDA Samples\v9.0\0_Simple\simplePrintf\simplePrintf.cu" -I"c:\ProgramData\NVIDIA Corporation\CUDA Samples\v9.0\common\inc" --cuda-gpu-arch=sm_50 --cuda-path="C:\Program Files\NVIDIA GPU Computing Toolkit\CUDA\v9.0" -L"c:\Program Files\NVIDIA GPU Computing Toolkit\CUDA\v9.0\lib\x64" -lcudart.lib  -v

Diff Detail

Repository
rC Clang

Event Timeline

emankov created this revision.Fri, Mar 15, 10:42 AM
tra added a comment.Fri, Mar 15, 11:05 AM

___ stands out as a sore thumb and raises unnecessary questions -- "why does it have three underscores, while __cptr is fine with two?".
Perhaps for consistency sake it would be better to replace __sptr -> __s and __cptr -> __c.
Given that we're just passing the args through, we don't really need to have ptr here.

Perhaps for consistency sake it would be better to replace __sptr -> __s and __cptr -> __c.

Well, it came from NVIDIA code, you know, I mean all those double underscores. As long as _sptr is also a keyword reserved by Microsoft, I think __s and __c are a good choice. Moreover, there are also double *__b and int *__c presented as device functions' arguments.

emankov updated this revision to Diff 190859.Fri, Mar 15, 11:36 AM

As long as _sptr is also a keyword reserved by Microsoft, I agree, that float *__s and float *__c are a good choice. Moreover, there are also double *__b and int *__c presented as device functions' arguments.

tra added a comment.Fri, Mar 15, 11:40 AM

Perhaps for consistency sake it would be better to replace __sptr -> __s and __cptr -> __c.

Well, it came from NVIDIA code, you know, I mean all those double underscores.

Not really. Function *signatures* are determined by CUDA, but argument names are not part of them and can be anything. Use of __ prefix is reserved for internal compiler use. Alas, in this case internal use of __sptr by clang happened to clash with the use of __sptr on windows. Neither is wrong.

tra accepted this revision.Fri, Mar 15, 11:41 AM
This revision is now accepted and ready to land.Fri, Mar 15, 11:41 AM

Use of __ prefix is reserved for internal compiler use.

It is generally known. What is unknown, at least for me, is why in those clang's headers __ prefix is used so heavily?

tra added a comment.Fri, Mar 15, 11:58 AM

The intent is to avoid unintentional clashes with the preprocessor macros the user may have defined.
https://reviews.llvm.org/rL260647

The intent is to avoid unintentional clashes with the preprocessor macros the user may have defined.
https://reviews.llvm.org/rL260647

Ok, thanks!

This revision was automatically updated to reflect the committed changes.