yaxunl marked 9 inline comments as done.
================
Comment at: lib/CodeGen/CGOpenCLRuntime.cpp:84
@@ +83,3 @@
+ SamplerTy = llvm::PointerType::get(llvm::StructType::create(
+ CGM.getLLVMContext(), "__opencl_sampler_t"),
+ CGM.getContext().getTargetAddressSpace(
----------------
Anastasia wrote:
> Related to this thread:
> http://lists.llvm.org/pipermail/cfe-dev/2016-July/050126.html.
>
> I am guessing leaving the old naming scheme should be fine.
I have some more comments about that. I think we should use
`struct.__opencl_sampler` as the name of sampler type in LLVM IR.
================
Comment at: lib/Sema/SemaInit.cpp:6949
@@ +6948,3 @@
+ // declaration.
+ if (!Var->getInit() || !isa<ImplicitCastExpr>(Var->getInit()))
+ break;
----------------
Anastasia wrote:
> Do we accept declaring global sampler_t variable without an initializer?
no. Since in file scope sampler variable is only allowed in constant addr
space, it has to be initialized. There is a test for it.
================
Comment at: lib/Sema/SemaInit.cpp:6956
@@ +6955,3 @@
+ }
+ }
+
----------------
Anastasia wrote:
> Could you use else here instead of adding TakeLiteral variable to avoid going
> to the next compound statement?
Good catch. Will do.
================
Comment at: lib/Sema/SemaInit.cpp:6965
@@ +6964,3 @@
+ // the initializer.
+ if (!Init->isConstantInitializer(S.Context, false))
+ break;
----------------
Anastasia wrote:
> Should this generate an error?
This has already been diagnosed when parsing the intializer of the variable
declaration.
================
Comment at: test/SemaOpenCL/sampler_t.cl:51
@@ -14,1 +50,3 @@
foo(glb_smp);
+ foo(glb_smp2);
+ foo(glb_smp3);
----------------
Anastasia wrote:
> Are these extra calls below adding any extra testing?
Add the function calls to test
1. No additional error msgs since the error msgs should be emitted for the
variable declarations.
2. no crashes when parsing them
================
Comment at: test/SemaOpenCL/sampler_t.cl:83
@@ -28,2 +82,3 @@
*smp2; //expected-error{{invalid argument type 'sampler_t' to unary
expression}}
+ foo(smp1+1); //expected-error{{invalid operands to binary expression
('sampler_t' and 'int')}}
}
----------------
Anastasia wrote:
> Does this line add extra testing?
Test no crashes when an invalid sampler expression is passed as a sampler
argument.
https://reviews.llvm.org/D21567
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
