vpykhtin added a subscriber: vpykhtin.
vpykhtin added a comment.
Hi,
is there an intention to make this SPIR 2.0 (provisional) conformant? This
would require changing resulting builtin names, changing pipe argument to
"struct opencl.pipe*" and adding two trailing "size, alignment" parameters. I
pxli168 added a comment.
Hi Anastasia/Pekka,
https://www.khronos.org/bugzilla/show_bug.cgi?id=1459
Bug report has been sent.
Thanks
Xiuli
http://reviews.llvm.org/D15914
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/c
pxli168 added a comment.
Hi,
Email was sent, hope this can be merged into llvm release3.8.
Thanks
Xiuli
http://reviews.llvm.org/D15914
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commit
pekka.jaaskelainen added a comment.
> @Pekka, do you think it would make sense to include a description of the new
> OpenCL features in the release note too? Or shall we wait for the completion
> of OpenCL 2.0 support (hopefully in next release)? Could do both as well I
> guess. :)
Yes I th
Anastasia added a comment.
Could you please copy us in CC?
@Pekka, do you think it would make sense to include a description of the new
OpenCL features in the release note too? Or shall we wait for the completion of
OpenCL 2.0 support (hopefully in next release)? Could do both as well I guess.
pekka.jaaskelainen accepted this revision.
pekka.jaaskelainen added a comment.
LGTM too. Good job. You can try and ask the release manager if you can sneak it
in. It's rather container patch so there might be a chance, but I don't know
the policy at this stage of the release.
http://reviews.ll
pxli168 added a comment.
Hi pekka,
What is your opinion about this patch?
And how could we ask the release manager to squeeze this patch into LLVM
release 3.8?
The pipe in OpneCL can only be handled by builtin functions, and this patch is
needed for the pipe support.
Thanks
Xiuli
http://revie
pxli168 updated this revision to Diff 45258.
pxli168 added a comment.
Remove some debug lines.
http://reviews.llvm.org/D15914
Files:
include/clang/Basic/Builtins.def
include/clang/Basic/Builtins.h
include/clang/Basic/DiagnosticSemaKinds.td
lib/Basic/Builtins.cpp
lib/CodeGen/CGBuiltin.
Anastasia accepted this revision.
Anastasia added a comment.
This revision is now accepted and ready to land.
LGTM! Could you please remove the line with printf before committing (see
comment above)?
Comment at: lib/CodeGen/CGBuiltin.cpp:1976
@@ +1975,3 @@
+getContext()
pxli168 added inline comments.
Comment at: lib/CodeGen/CGBuiltin.cpp:1974
@@ +1973,3 @@
+// Type of the generic packet parameter.
+unsigned GenericAS =
+getContext().getTargetAddressSpace(LangAS::opencl_generic);
Good idea! This can fit different t
pxli168 updated the summary for this revision.
pxli168 updated this revision to Diff 45234.
pxli168 marked 9 inline comments as done.
pxli168 added a comment.
Fix some small comments problems.
http://reviews.llvm.org/D15914
Files:
include/clang/Basic/Builtins.def
include/clang/Basic/Builtin
Anastasia added a comment.
A few nitpicks here, otherwise it seems we are in a good shape. I hope this
will hit trunk soon. :)
Comment at: lib/CodeGen/CGBuiltin.cpp:1966
@@ -1965,1 +1965,3 @@
}
+
+ case Builtin::BIread_pipe:
Can you put a comment with the s
pxli168 updated this revision to Diff 44965.
pxli168 added a comment.
Forget about get_pipe_num/max_packets builtin.
http://reviews.llvm.org/D15914
Files:
include/clang/Basic/Builtins.def
include/clang/Basic/Builtins.h
include/clang/Basic/DiagnosticSemaKinds.td
lib/Basic/Builtins.cpp
pxli168 marked 2 inline comments as done.
Comment at: lib/Sema/SemaChecking.cpp:378
@@ +377,3 @@
+ }
+
+ return false;
Anastasia wrote:
> I am asking about setArg and not setType!
Oh, I just mistook your meaning. It seems we don't need that as we have to
covert
Anastasia added inline comments.
Comment at: lib/CodeGen/CGBuiltin.cpp:2082
@@ +2081,3 @@
+if (BuiltinID == Builtin::BIget_pipe_num_packets)
+ Name = "get_pipe_num_packets";
+else
prepend "__" here too!
Comment at: lib/Sema/SemaChec
pxli168 updated this revision to Diff 44881.
pxli168 added a comment.
Add prefix "__" to CodeGen builtin function name.
http://reviews.llvm.org/D15914
Files:
include/clang/Basic/Builtins.def
include/clang/Basic/Builtins.h
include/clang/Basic/DiagnosticSemaKinds.td
lib/Basic/Builtins.cpp
pxli168 added a comment.
I think we'd better to ask about to add this in. The pipe type support actually
have no use without builtins to read or write it.
Comment at: lib/CodeGen/CGBuiltin.cpp:1981
@@ +1980,3 @@
+ const char *Name =
+ (BuiltinID == Builtin::BIread
Anastasia added inline comments.
Comment at: lib/CodeGen/CGBuiltin.cpp:1981
@@ +1980,3 @@
+ const char *Name =
+ (BuiltinID == Builtin::BIread_pipe) ? "read_pipe_2" : "write_pipe_2";
+ // Re-Creating the function type for this call, since the original type
-
pekka.jaaskelainen added a comment.
OK. Seems the LLVM 3.8 branching was done, it might be hard to get this in, but
should we try and ask the release manager?
Comment at: lib/CodeGen/CGBuiltin.cpp:1981
@@ +1980,3 @@
+ const char *Name =
+ (BuiltinID == Builtin::BI
pxli168 updated this revision to Diff 44826.
pxli168 added a comment.
Fix format of diag err string, remove useless ':' after expecting.
http://reviews.llvm.org/D15914
Files:
include/clang/Basic/Builtins.def
include/clang/Basic/Builtins.h
include/clang/Basic/DiagnosticSemaKinds.td
lib/B
Anastasia added inline comments.
Comment at: test/SemaOpenCL/invalid-pipe-builtin-cl2.0.cl:37
@@ +36,3 @@
+ write_pipe(p, tmp); // expected-error {{invalid argument type to function
write_pipe (expecting: 'int *')}}
+ read_pipe(p, ptr);// expected-error {{invalid pipe acc
keryell added a subscriber: keryell.
Comment at: include/clang/Basic/Builtins.h:39
@@ -38,2 +38,3 @@
MS_LANG = 0x10, // builtin requires MS mode.
+ OCLC_LANG = 0x20,// builtin for OpenCL C only.
ALL_LANGUAGES = C_LANG | CXX_LANG | OBJC_LANG, // builtin for all languages.
-
pxli168 updated this revision to Diff 44624.
pxli168 added a comment.
Rewrite some comment.
Can this patch be in llvm3.8 release if it is commited after 13th Jan?
http://reviews.llvm.org/D15914
Files:
include/clang/Basic/Builtins.def
include/clang/Basic/Builtins.h
include/clang/Basic/Dia
pxli168 added inline comments.
Comment at: include/clang/Basic/Builtins.h:39
@@ -38,2 +38,3 @@
MS_LANG = 0x10, // builtin requires MS mode.
+ OCLC_LANG = 0x20,// builtin for OpenCL C only.
ALL_LANGUAGES = C_LANG | CXX_LANG | OBJC_LANG, // builtin for all languages.
---
pxli168 marked 4 inline comments as done.
Comment at: include/clang/Basic/Builtins.def:1255
@@ -1254,1 +1254,3 @@
+// OpenCL v2.0 s6.13.16, s9.17.3.5 -- Pipe functions.
+// We need the generic prototype, since the packet type could be anything.
Anastasia wrote:
Anastasia added inline comments.
Comment at: include/clang/Basic/Builtins.def:1255
@@ -1254,1 +1254,3 @@
+// OpenCL v2.0 s6.13.16, s9.17.3.5 -- Pipe functions.
+// We need the generic prototype, since the packet type could be anything.
Could you remove one -?
=
pxli168 updated this revision to Diff 44601.
pxli168 added a comment.
Add negative tests to hint all paths in the Semacheck.
http://reviews.llvm.org/D15914
Files:
include/clang/Basic/Builtins.def
include/clang/Basic/Builtins.h
include/clang/Basic/DiagnosticSemaKinds.td
lib/Basic/Builtin
pxli168 added inline comments.
Comment at: include/clang/Basic/Builtins.def:1260
@@ +1259,3 @@
+
+LANGBUILTIN(reserve_read_pipe, "i.", "tn", OCLC_LANG)
+LANGBUILTIN(reserve_write_pipe, "i.", "tn", OCLC_LANG)
Anastasia wrote:
> Ok, this way it means variable number
pekka.jaaskelainen added inline comments.
Comment at: lib/CodeGen/CGBuiltin.cpp:2033
@@ +2032,3 @@
+ *Arg1 = EmitScalarExpr(E->getArg(1));
+llvm::Type *ReservedIDTy = ConvertType(getContext().OCLReserveIDTy);
+
Anastasia wrote:
> pekka.jaaskelainen wr
Anastasia added inline comments.
Comment at: lib/CodeGen/CGBuiltin.cpp:2033
@@ +2032,3 @@
+ *Arg1 = EmitScalarExpr(E->getArg(1));
+llvm::Type *ReservedIDTy = ConvertType(getContext().OCLReserveIDTy);
+
pekka.jaaskelainen wrote:
> Anastasia wrote:
> >
pekka.jaaskelainen added inline comments.
Comment at: lib/CodeGen/CGBuiltin.cpp:2033
@@ +2032,3 @@
+ *Arg1 = EmitScalarExpr(E->getArg(1));
+llvm::Type *ReservedIDTy = ConvertType(getContext().OCLReserveIDTy);
+
Anastasia wrote:
> Why do we need to mod
Anastasia added inline comments.
Comment at: include/clang/Basic/Builtins.def:1260
@@ +1259,3 @@
+
+LANGBUILTIN(reserve_read_pipe, "i.", "tn", OCLC_LANG)
+LANGBUILTIN(reserve_write_pipe, "i.", "tn", OCLC_LANG)
Ok, this way it means variable number of arg which isn
pxli168 updated the summary for this revision.
pxli168 updated this revision to Diff 44404.
pxli168 added a comment.
1. Add tests for builtin function CodeGen and Semacheck
2. Add TODO for what we need to do about access qualifier
3. Fix some bugs found in writing
Hope we cans squeeze these patch
pekka.jaaskelainen added a comment.
In http://reviews.llvm.org/D15914#322917, @pxli168 wrote:
> By the way, I just got the access to the llvm svn, can I just commit the pipe
> type patch directly as I see you all accepted it. Or I should send it to the
> cfe-commit first?
I'm afraid we won'
pxli168 added a comment.
Then maybe I should hurry up to see if I can finish the Semacheck and this
patch in time.
By the way, I just got the access to the llvm svn, can I just commit the pipe
type patch directly as I see you all accepted it. Or I should send it to the
cfe-commit first?
=
pxli168 added inline comments.
Comment at: include/clang/Basic/Builtins.def:1260
@@ +1259,3 @@
+
+LANGBUILTIN(reserve_read_pipe, "i.", "tn", OCLC_LANG)
+LANGBUILTIN(reserve_write_pipe, "i.", "tn", OCLC_LANG)
Anastasia wrote:
> That's right! I am just saying that w
pekka.jaaskelainen added a comment.
I wonder could we squeeze this in before the next week's LLVM 3.8 branching? It
would be great.
Comment at: lib/CodeGen/CGBuiltin.cpp:2033
@@ +2032,3 @@
+else if (BuiltinID == Builtin::BIwork_group_reserve_write_pipe)
+ Name = "_Z29w
Anastasia added a comment.
Do you have any tests? We won't be able to commit without. Also having them
would help understanding what this modification does.
Comment at: include/clang/Basic/Builtins.def:1260
@@ +1259,3 @@
+
+LANGBUILTIN(reserve_read_pipe, "i.", "tn", OCLC_LANG)
pxli168 updated this revision to Diff 44312.
pxli168 added a comment.
1. Refine the def of BUILTIN with LANGBUITIN with new OCLC_LANG
2. Remove mangler for the hard coded builtin function name
3. Fix some bugs.
http://reviews.llvm.org/D15914
Files:
include/clang/Basic/Builtins.def
include/c
pxli168 added a comment.
So here is the summary about the inline comments:
1. What kind of prototype should codegen output for these functions?
2. What function names should these builtins have in llvm ir?
3. Access for pipe use image's for now, and will be refined.
4. What rules should we follow
Anastasia added inline comments.
Comment at: include/clang/Basic/Builtins.def:1255
@@ -1254,1 +1254,3 @@
+// OpenCL 2.0 Pipe functions.
+// We need the variadic prototype, since the packet type could be anything.
Could we put reference to spec section?
pxli168 created this revision.
pxli168 added reviewers: pekka.jaaskelainen, Anastasia.
pxli168 added subscribers: cfe-commits, bader.
Support for the pipe built-in functions.
The pipe packet type can be user defined structure, we need to handle this in
clang frontend.
This version is based on bad
42 matches
Mail list logo
