[PATCH] D47201: [CUDA] Implement nv_weak attribute for functions
Hahnfeld abandoned this revision. Hahnfeld added a comment. See https://reviews.llvm.org/D47804 Repository: rC Clang https://reviews.llvm.org/D47201 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47201: [CUDA] Implement nv_weak attribute for functions
tra added a comment. I've experimented a bit and I think that we may not need this patch at all. As far as I can tell, nv_weak is only applicable to __device__ functions. It's ignored for __global__ kernels and is apparently forbidden for data. For __device__ functions nvcc produces .weak attribute in PTX. Using plain old __attribute__((weak)) does exactly the same. Considering that nv_weak is only used inside CUDA SDK headers, substituting weak in place of nv_weak will result in correct PTX, which is all we really need. I don't see much benefit turning it into full blown attribute just to mimic an internal CUDA implementation detail we don't care all that much about. Now, replacing it in CUDA headers for all CUDA versions we support may be tricky. Let me give it a try. I'll send a patch, if I manage to make it work. Repository: rC Clang https://reviews.llvm.org/D47201 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47201: [CUDA] Implement nv_weak attribute for functions
aaron.ballman added a comment. In https://reviews.llvm.org/D47201#1119947, @Hahnfeld wrote: > In https://reviews.llvm.org/D47201#1119254, @aaron.ballman wrote: > > > In https://reviews.llvm.org/D47201#1119249, @tra wrote: > > > > > IIUIC, nv_weak is a synonym for weak (why, oh why did they need > > > it?) > > > You may need to hunt down and change few other places that deal with the > > > weak attribute. > > > E.g.: > > > https://github.com/llvm-project/llvm-project-20170507/blob/master/clang/lib/AST/Decl.cpp#L4267 > > > > > > https://github.com/llvm-project/llvm-project-20170507/blob/master/clang/lib/CodeGen/ItaniumCXXABI.cpp#L3045 > > > > > > If it is truly a synonym for weak, then a better implementation would be to > > make no semantic distinction between the two attributes -- just add new > > spellings to weak. If you need to make minor distinctions between the > > spellings, you can do it using accessors on the attribute. > > > I first went with this approach but then thought it would be better to > restrict the new attribute as much as possible. That's why I added a > completely new one which is only applicable to functions, but not to > variables and `CXXRecord`s. Let me know if you'd prefer `nv_weak` to be a > full alias of `weak` and I'll revert to what @aaron.ballman suggested. I don't know enough about nv_weak's semantics to definitively say one way or the other -- I can find no documentation on this attribute (official or otherwise). However, based purely on the changes made here, I'd likely add the accessors and only go with a single semantic attribute. You can check for the proper subjects by looking at the parsed attribute kind in SemaDeclAttr.cpp and restricting the subjects there (it's a bit less declarative this way, but we don't have a way to map subject lists to spellings like we do for accessors because it's not a common need). Repository: rC Clang https://reviews.llvm.org/D47201 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47201: [CUDA] Implement nv_weak attribute for functions
Hahnfeld added a comment. In https://reviews.llvm.org/D47201#1119254, @aaron.ballman wrote: > In https://reviews.llvm.org/D47201#1119249, @tra wrote: > > > IIUIC, nv_weak is a synonym for weak (why, oh why did they need > > it?) > > You may need to hunt down and change few other places that deal with the > > weak attribute. > > E.g.: > > https://github.com/llvm-project/llvm-project-20170507/blob/master/clang/lib/AST/Decl.cpp#L4267 > > > > https://github.com/llvm-project/llvm-project-20170507/blob/master/clang/lib/CodeGen/ItaniumCXXABI.cpp#L3045 > > > If it is truly a synonym for weak, then a better implementation would be to > make no semantic distinction between the two attributes -- just add new > spellings to weak. If you need to make minor distinctions between the > spellings, you can do it using accessors on the attribute. I first went with this approach but then thought it would be better to restrict the new attribute as much as possible. That's why I added a completely new one which is only applicable to functions, but not to variables and `CXXRecord`s. Let me know if you'd prefer `nv_weak` to be a full alias of `weak` and I'll revert to what @aaron.ballman suggested. Repository: rC Clang https://reviews.llvm.org/D47201 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47201: [CUDA] Implement nv_weak attribute for functions
aaron.ballman added a comment. In https://reviews.llvm.org/D47201#1119249, @tra wrote: > IIUIC, nv_weak is a synonym for weak (why, oh why did they need > it?) > You may need to hunt down and change few other places that deal with the > weak attribute. > E.g.: > https://github.com/llvm-project/llvm-project-20170507/blob/master/clang/lib/AST/Decl.cpp#L4267 > > https://github.com/llvm-project/llvm-project-20170507/blob/master/clang/lib/CodeGen/ItaniumCXXABI.cpp#L3045 If it is truly a synonym for weak, then a better implementation would be to make no semantic distinction between the two attributes -- just add new spellings to weak. If you need to make minor distinctions between the spellings, you can do it using accessors on the attribute. Comment at: include/clang/Basic/Attr.td:1515 let LangOpts = [CUDA]; + let Documentation = [Undocumented]; } No new, undocumented attributes, please. Repository: rC Clang https://reviews.llvm.org/D47201 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47201: [CUDA] Implement nv_weak attribute for functions
tra added a comment. IIUIC, nv_weak is a synonym for weak (why, oh why did they need it?) You may need to hunt down and change few other places that deal with the weak attribute. E.g.: https://github.com/llvm-project/llvm-project-20170507/blob/master/clang/lib/AST/Decl.cpp#L4267 https://github.com/llvm-project/llvm-project-20170507/blob/master/clang/lib/CodeGen/ItaniumCXXABI.cpp#L3045 Repository: rC Clang https://reviews.llvm.org/D47201 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47201: [CUDA] Implement nv_weak attribute for functions
Hahnfeld added a comment. Ping Repository: rC Clang https://reviews.llvm.org/D47201 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47201: [CUDA] Implement nv_weak attribute for functions
Hahnfeld created this revision. Hahnfeld added a reviewer: tra. Herald added a subscriber: cfe-commits. This is needed for relocatable device code with CUDA 9 and later. Before this patch, linking two or more object files resulted in "Multiple definition" errors for a group of functions from cuda_device_runtime_api.h which are annoted with "nv_weak". CUDA headers already used this attribute in earlier releases, but until CUDA 8.0 the only definitions in cuda_device_runtime_api.h were conditional under `defined(__CUDABE__)` which is explicitly undefined in Clang's wrapper. However since CUDA 9.0 this has changed to `!defined(__CUDACC_RTC__)`. Trying to add that define resulted in errors that nvrtc_device_runtime.h could not be found. Reported by Andrea Bocci! Repository: rC Clang https://reviews.llvm.org/D47201 Files: include/clang/Basic/Attr.td include/clang/Basic/DiagnosticSemaKinds.td lib/CodeGen/CodeGenModule.cpp lib/Sema/SemaDecl.cpp lib/Sema/SemaDeclAttr.cpp test/CodeGenCUDA/nv_weak.cu test/SemaCUDA/attr-declspec.cu test/SemaCUDA/attr-nv_weak.cu test/SemaCUDA/attributes-on-non-cuda.cu Index: test/SemaCUDA/attributes-on-non-cuda.cu === --- test/SemaCUDA/attributes-on-non-cuda.cu +++ test/SemaCUDA/attributes-on-non-cuda.cu @@ -7,11 +7,12 @@ // RUN: %clang_cc1 -DEXPECT_WARNINGS -fsyntax-only -verify -x c %s #if defined(EXPECT_WARNINGS) -// expected-warning@+12 {{'device' attribute ignored}} -// expected-warning@+12 {{'global' attribute ignored}} -// expected-warning@+12 {{'constant' attribute ignored}} -// expected-warning@+12 {{'shared' attribute ignored}} -// expected-warning@+12 {{'host' attribute ignored}} +// expected-warning@+13 {{'device' attribute ignored}} +// expected-warning@+13 {{'global' attribute ignored}} +// expected-warning@+13 {{'constant' attribute ignored}} +// expected-warning@+13 {{'shared' attribute ignored}} +// expected-warning@+13 {{'host' attribute ignored}} +// expected-warning@+13 {{'nv_weak' attribute ignored}} // // NOTE: IgnoredAttr in clang which is used for the rest of // attributes ignores LangOpts, so there are no warnings. @@ -24,11 +25,11 @@ __attribute__((constant)) int* g_constant; __attribute__((shared)) float *g_shared; __attribute__((host)) void f_host(); +__attribute__((nv_weak)) void f_nv_weak(); __attribute__((device_builtin)) void f_device_builtin(); typedef __attribute__((device_builtin)) const void *t_device_builtin; enum __attribute__((device_builtin)) e_device_builtin {E}; __attribute__((device_builtin)) int v_device_builtin; __attribute__((cudart_builtin)) void f_cudart_builtin(); -__attribute__((nv_weak)) void f_nv_weak(); __attribute__((device_builtin_surface_type)) unsigned long long surface_var; __attribute__((device_builtin_texture_type)) unsigned long long texture_var; Index: test/SemaCUDA/attr-nv_weak.cu === --- /dev/null +++ test/SemaCUDA/attr-nv_weak.cu @@ -0,0 +1,14 @@ +// RUN: %clang_cc1 -verify -fsyntax-only %s + +extern int f0() __attribute__((nv_weak)); +extern int g0 __attribute__((nv_weak)); // expected-warning {{'nv_weak' attribute only applies to functions}} +int f1() __attribute__((nv_weak)); +int g1 __attribute__((nv_weak)); // expected-warning {{'nv_weak' attribute only applies to functions}} + + +struct __attribute__((nv_weak)) s0 {}; // expected-warning {{'nv_weak' attribute only applies to functions}} + +static int f() __attribute__((nv_weak)); // expected-error {{nv_weak declaration cannot have internal linkage}} + +static void pr14946_f(); +void pr14946_f() __attribute__((nv_weak)); // expected-error {{nv_weak declaration cannot have internal linkage}} Index: test/SemaCUDA/attr-declspec.cu === --- test/SemaCUDA/attr-declspec.cu +++ test/SemaCUDA/attr-declspec.cu @@ -6,11 +6,12 @@ // RUN: %clang_cc1 -DEXPECT_WARNINGS -fms-extensions -fsyntax-only -verify -x c %s #if defined(EXPECT_WARNINGS) -// expected-warning@+12 {{'__device__' attribute ignored}} -// expected-warning@+12 {{'__global__' attribute ignored}} -// expected-warning@+12 {{'__constant__' attribute ignored}} -// expected-warning@+12 {{'__shared__' attribute ignored}} -// expected-warning@+12 {{'__host__' attribute ignored}} +// expected-warning@+13 {{'__device__' attribute ignored}} +// expected-warning@+13 {{'__global__' attribute ignored}} +// expected-warning@+13 {{'__constant__' attribute ignored}} +// expected-warning@+13 {{'__shared__' attribute ignored}} +// expected-warning@+13 {{'__host__' attribute ignored}} +// expected-warning@+13 {{'__nv_weak__' attribute ignored}} // // (Currently we don't for the other attributes. They are implemented with // IgnoredAttr, which is ignored irrespective of any LangOpts.) @@ -23,12 +24,11 @@ __declspec(__constant__) int* g_constant; __declspec(__shared__) float *g_shared; __declsp