linjamaki marked 2 inline comments as done.
linjamaki added inline comments.
================
Comment at: clang/lib/Basic/Targets.cpp:609
}
+ case llvm::Triple::spirv32: {
+ if (os != llvm::Triple::UnknownOS ||
----------------
Anastasia wrote:
> I wonder how complete is the support of logical addressing SPIR-V triple? It
> seems like you don't test it in the clang invocation at the moment and it is
> therefore missing from `TargetInfo`.
>
> Do you have plans to implement it in the subsequent patches? If not it might
> be better to leave it out for now.
I removed the spirvlogical triple. AFAIK, this triple has zero sized pointers
and I don't know if the LLVM is ready for that yet.
================
Comment at: clang/test/CodeGenOpenCL/spirv32_target.cl:12
+kernel void foo(global long *arg) {
+ int res1[sizeof(my_st) == 12 ? 1 : -1];
+ int res2[sizeof(void *) == 4 ? 1 : -1];
----------------
Anastasia wrote:
> Are these lines tested somehow? You could change this into C++ for OpenCL
> test and use `static_assert` or find some other ways to test this...
>
> However, this testing seems to overlap with the lines at the end... Could you
> please elaborate on the intent of this test?
>
> Also if you don't plan this to be fundamentally different from testing of
> 64bit triple I think this should be merged with `spirv64_target.cl`. It will
> make things easier for the maintenance and further evolution.
I used spir{32,64}_target.cl as a base for checking codegen for SPIR-V with the
only difference being the triple check. The lines give an compile error (e.g.
error: 'res1' declared as an array with a negative size) if the sizeof
operators give unexpected result.
I'll merge this file with the spirv64_target.cl.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D109144/new/
https://reviews.llvm.org/D109144
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
