Anastasia added inline comments.

================
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];
----------------
linjamaki wrote:
> 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.
> 
Ok, you could consider adding `-verify` and 

`//expected-no-diagnostics`

Even though FileCheck should not succeed but it is going to be more clear that 
some functionality is tested via diagnostics for the future modifications. 

However, you seem to be testing pointer size in the two lines below which also 
seem to be tested via `//CHECK` directives... not sure whether both are needed.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D109144/new/

https://reviews.llvm.org/D109144

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to