aaron.ballman accepted this revision.
aaron.ballman added a comment.
LGTM, thanks!
http://reviews.llvm.org/D16040
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Anastasia accepted this revision.
Anastasia added a comment.
This revision is now accepted and ready to land.
LGTM!
http://reviews.llvm.org/D16040
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
pxli168 updated this revision to Diff 49007.
http://reviews.llvm.org/D16040
Files:
include/clang/Basic/Attr.td
include/clang/Basic/AttrDocs.td
include/clang/Basic/DiagnosticSemaKinds.td
lib/CodeGen/CodeGenFunction.cpp
lib/Parse/ParseDecl.cpp
lib/Sema/SemaChecking.cpp
pxli168 marked 2 inline comments as done.
pxli168 added a comment.
Remove test case for access quilifier in
test/SemaOpenCL/invalid-kernel-attrs.cl.
Due to the patch http://reviews.llvm.org/D17437.
read_only can only be used in parameters with pipe and image type.
Comment at:
Anastasia added inline comments.
Comment at: include/clang/Basic/AttrDocs.td:1572
@@ +1571,3 @@
+The __read_only, __write_only, __read_write, read_only, write_only and
+read_write names are reserved for use as access qualifiers and shall not be
+used otherwise.
aaron.ballman added a comment.
Mostly minor comments, but I like this approach!
Comment at: include/clang/Basic/DiagnosticSemaKinds.td:7713
@@ +7712,3 @@
+def err_opencl_invalid_read_write : Error<
+ "access qualifier read_write can not be used for %0 %select{|earlier than
Anastasia added inline comments.
Comment at: test/SemaOpenCL/invalid-pipe-builtin-cl2.0.cl:24
@@ -23,3 +23,3 @@
work_group_commit_read_pipe(p, tmp);// expected-error{{invalid argument
type to function work_group_commit_read_pipe (expecting 'reserve_id_t')}}
-
Anastasia added a comment.
A few small comments!
Comment at: include/clang/Basic/AttrDocs.td:1572
@@ +1571,3 @@
+The __read_only, __write_only, __read_write, read_only, write_only and
+read_write names are reserved for use as access qualifiers and shall not be
+used otherwise.
pxli168 added inline comments.
Comment at: lib/Sema/SemaDeclAttr.cpp:5046
@@ +5045,3 @@
+ if (D->hasAttr()) {
+S.Diag(Attr.getLoc(), diag::err_opencl_multiple_access_qualifiers)
+<< D->getSourceRange();
Anastasia wrote:
> Yes, I think attribute would
Anastasia added inline comments.
Comment at: lib/Sema/SemaDeclAttr.cpp:5046
@@ +5045,3 @@
+ if (D->hasAttr()) {
+S.Diag(Attr.getLoc(), diag::err_opencl_multiple_access_qualifiers)
+<< D->getSourceRange();
Yes, I think attribute would make more sense.
pxli168 updated this revision to Diff 48452.
pxli168 added a comment.
Refine the pipe parse to solve the problem the attribute for pipe will be
handled twice within Declarator
http://reviews.llvm.org/D16040
Files:
include/clang/Basic/Attr.td
include/clang/Basic/AttrDocs.td
pxli168 planned changes to this revision.
pxli168 added a comment.
It seems it is related with the pipe type. I am still working on how to fix
this problem.
http://reviews.llvm.org/D16040
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
aaron.ballman added inline comments.
Comment at: lib/Sema/SemaDeclAttr.cpp:5046
@@ +5045,3 @@
+ if (D->hasAttr()) {
+S.Diag(D->getLocation(), diag::err_opencl_multiple_access_qualifiers)
+<< D->getSourceRange();
That's why I think it should be
pxli168 requested a review of this revision.
pxli168 added a comment.
Need to refine access qualifier with pipe type.
Comment at: lib/Sema/SemaDeclAttr.cpp:5052
@@ +5051,3 @@
+ if (D->hasAttr()) {
+S.Diag(D->getLocation(), diag::err_opencl_multiple_access_qualifiers)
+
pxli168 updated this revision to Diff 48046.
pxli168 marked 3 inline comments as done.
pxli168 added a comment.
Update doc for OpenCL access qualifier, still find where to handle access
qualifier for pipe type.
http://reviews.llvm.org/D16040
Files:
include/clang/Basic/Attr.td
pekka.jaaskelainen accepted this revision.
pekka.jaaskelainen added a comment.
Otherwise LGTM.
Comment at: lib/Sema/SemaChecking.cpp:267
@@ -266,3 +266,3 @@
/// Returns OpenCL access qual.
// TODO: Refine OpenCLImageAccessAttr to OpenCLAccessAttr since pipe can use
// it too
aaron.ballman added a subscriber: aaron.ballman.
aaron.ballman added a reviewer: aaron.ballman.
Comment at: include/clang/Basic/Attr.td:671
@@ -670,3 +670,3 @@
Keyword<"write_only">]>];
let Documentation = [Undocumented];
}
Anastasia added inline comments.
Comment at: lib/Sema/SemaDeclAttr.cpp:5823
@@ -5788,8 +5822,3 @@
- // Walk the declarator structure, applying decl attributes that were in a
type
- // position to the decl itself. This handles cases like:
- // int *__attr__(x)** D;
- //
aaron.ballman added inline comments.
Comment at: lib/Sema/SemaDeclAttr.cpp:5823
@@ -5788,8 +5822,3 @@
- // Walk the declarator structure, applying decl attributes that were in a
type
- // position to the decl itself. This handles cases like:
- // int *__attr__(x)** D;
-
Anastasia accepted this revision.
Anastasia added a comment.
This revision is now accepted and ready to land.
LGTM!
@Pekka, any comments here? We will finalize it if not.
Comment at: lib/Sema/SemaDeclAttr.cpp:5823
@@ -5788,8 +5822,3 @@
- // Walk the declarator structure,
pxli168 updated this revision to Diff 47455.
pxli168 marked 5 inline comments as done.
http://reviews.llvm.org/D16040
Files:
include/clang/Basic/Attr.td
include/clang/Basic/DiagnosticSemaKinds.td
lib/CodeGen/CodeGenFunction.cpp
lib/Sema/SemaChecking.cpp
lib/Sema/SemaDeclAttr.cpp
Anastasia added a comment.
Adding a few final comments, otherwise, looks good!
Comment at: lib/Sema/SemaDeclAttr.cpp:5050
@@ +5049,3 @@
+
+ // Check if there only one access qualifier
+ if (D->hasAttr()) {
there is only one
Comment at:
Anastasia added inline comments.
Comment at: lib/Sema/SemaDeclAttr.cpp:5067
@@ +5066,3 @@
+ if (DeclTy->isPipeType() ||
+ (S.getLangOpts().OpenCLVersion < 200 && DeclTy->isImageType())) {
+S.Diag(D->getLocation(), diag::err_opencl_invalid_read_write)
pxli168 updated this revision to Diff 47073.
pxli168 added a comment.
Make some optimization
http://reviews.llvm.org/D16040
Files:
include/clang/Basic/Attr.td
include/clang/Basic/DiagnosticSemaKinds.td
lib/CodeGen/CodeGenFunction.cpp
lib/Sema/SemaChecking.cpp
pxli168 added inline comments.
Comment at: lib/Sema/SemaDeclAttr.cpp:5067
@@ +5066,3 @@
+ if (DeclTy->isPipeType() ||
+ (S.getLangOpts().OpenCLVersion < 200 && DeclTy->isImageType())) {
+S.Diag(D->getLocation(), diag::err_opencl_invalid_read_write)
pxli168 added inline comments.
Comment at: lib/Sema/SemaDeclAttr.cpp:5067
@@ +5066,3 @@
+ if (DeclTy->isPipeType() ||
+ (S.getLangOpts().OpenCLVersion < 200 && DeclTy->isImageType())) {
+S.Diag(D->getLocation(), diag::err_opencl_invalid_read_write)
pxli168 updated this revision to Diff 46865.
pxli168 added a comment.
Remove repeat test case.
http://reviews.llvm.org/D16040
Files:
include/clang/Basic/Attr.td
include/clang/Basic/DiagnosticSemaKinds.td
lib/CodeGen/CodeGenFunction.cpp
lib/Sema/SemaChecking.cpp
Anastasia added inline comments.
Comment at: lib/Sema/SemaDeclAttr.cpp:5705
@@ -5669,8 +5704,3 @@
- // Walk the declarator structure, applying decl attributes that were in a
type
- // position to the decl itself. This handles cases like:
- // int *__attr__(x)** D;
- //
pxli168 added inline comments.
Comment at: test/Parser/opencl-image-access.cl:9
@@ -7,1 +8,3 @@
+#if CL20
__kernel void f__rw(__read_write image2d_t a) { }
+#endif
Anastasia wrote:
> Btw, I can see that read_write is now accepted even if -cl-std=CL1.1. So
>
pxli168 updated this revision to Diff 45681.
pxli168 marked 4 inline comments as done.
pxli168 added a comment.
Rewrite some comments and add missing invalid test case.
http://reviews.llvm.org/D16040
Files:
include/clang/Basic/Attr.td
include/clang/Basic/DiagnosticSemaKinds.td
pxli168 added inline comments.
Comment at: lib/Sema/SemaDeclAttr.cpp:4934
@@ +4933,3 @@
+const Type *DeclTy = PDecl->getType().getCanonicalType().getTypePtr();
+if (AccessAttr->isReadWrite()) {
+ if (DeclTy->isPipeType() ||
Anastasia wrote:
> In the
pxli168 updated this revision to Diff 44600.
pxli168 added a comment.
Fix destruct problem and fix style in inline comment.
http://reviews.llvm.org/D16040
Files:
include/clang/Basic/Attr.td
include/clang/Basic/DiagnosticSemaKinds.td
lib/CodeGen/CodeGenFunction.cpp
Anastasia added inline comments.
Comment at: lib/Sema/SemaDeclAttr.cpp:4934
@@ +4933,3 @@
+const Type *DeclTy = PDecl->getType().getCanonicalType().getTypePtr();
+if (AccessAttr->isReadWrite()) {
+ if (DeclTy->isPipeType() ||
In the case of failure,
pxli168 created this revision.
pxli168 added reviewers: Anastasia, pekka.jaaskelainen.
pxli168 added a subscriber: cfe-commits.
OpenCL access qualifiers are now not only used for image types, refine it to
avoid misleading,
Add semacheck for OpenCL access qualifier as well as test caees.
34 matches
Mail list logo
