[PATCH] D47201: [CUDA] Implement nv_weak attribute for functions

2018-06-05 Thread Jonas Hahnfeld via Phabricator via cfe-commits
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

2018-06-05 Thread Artem Belevich via Phabricator via cfe-commits
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

2018-06-04 Thread Aaron Ballman via Phabricator via cfe-commits
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

2018-06-02 Thread Jonas Hahnfeld via Phabricator via cfe-commits
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

2018-06-01 Thread Aaron Ballman via Phabricator via cfe-commits
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

2018-06-01 Thread Artem Belevich via Phabricator via cfe-commits
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

2018-05-31 Thread Jonas Hahnfeld via Phabricator via cfe-commits
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

2018-05-22 Thread Jonas Hahnfeld via Phabricator via cfe-commits
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