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
http://lists.llvm.org/cgi-bin/mailman/listinfo
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
lib/Sema/SemaDe
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: t
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.
An
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
Ope
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')}}
- sub_group_co
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
include/clang/
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
http
pxli168 added a comment.
Hi Anastasia,
It seems the access qualifier attribute may need to be handled like the you
have mentioned in clang-commit that to be in type.
What is you opinion? I can write a patch for that too.
Thanks
Comment at: lib/Sema/SemaDeclAttr.cpp:5804-5816
@
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 pointin
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
include/clang
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 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;
- // w
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];
}
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
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, app
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
lib/S
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: lib/Se
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 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
lib/Sema/SemaDeclAttr.cpp
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 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
lib/Sema/SemaDeclAttr.c
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
> esse
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;
- // w
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
lib/CodeGen
Anastasia added a comment.
I think some tests for new diagnostics are still missing?
Comment at: include/clang/Basic/DiagnosticSemaKinds.td:7676
@@ +7675,3 @@
+def err_opencl_invalid_read_write : Error<
+ "access qualifier read_write can not be used for %0 %select{|before
CL2.
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
lib/Sema/SemaChecking.
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 c
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, w
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.
http://
36 matches
Mail list logo
