Anastasia added inline comments.
================
Comment at: lib/CodeGen/CGOpenCLRuntime.cpp:83
@@ +82,3 @@
+ SamplerTy = llvm::PointerType::get(llvm::StructType::create(
+ CGM.getLLVMContext(), "__sampler"),
+ CGM.getContext().getTargetAddressSpace(
----------------
I am wondering if we could at least name it __opencl_sampler_t. I kind of don't
like that we have inconsistency with the rest here.
Btw, the types in Debug are all named opencl_<type>. Perhaps we could correct
them consistently across?
================
Comment at: lib/CodeGen/CodeGenModule.cpp:4309
@@ +4308,3 @@
+ // in SPIR spec v1.2 and also opencl-c.h
+ unsigned AddressingMode = (0x0E & SamplerValue) >> 1;
+ unsigned FilterMode = (0x30 & SamplerValue) >> 4;
----------------
Yes sounds good!
================
Comment at: lib/Sema/SemaInit.cpp:6907
@@ -6906,3 +6907,1 @@
case SK_OCLSamplerInit: {
- assert(Step->Type->isSamplerT() &&
- "Sampler initialization on non-sampler type.");
----------------
But I am guessing that holds for most of the assertions. :)
================
Comment at: lib/Sema/SemaInit.cpp:6915
@@ -6915,3 +6914,3 @@
<< SourceType;
- } else if (Entity.getKind() != InitializedEntity::EK_Variable) {
- llvm_unreachable("Invalid EntityKind!");
+ } else if (const DeclRefExpr *DRE = dyn_cast<DeclRefExpr>(Init)) {
+ Init = const_cast<Expr*>(cast<VarDecl>(DRE->getDecl())->getInit());
----------------
Should you also check for type of Expr to be sampler?
Btw is this covered in testing?
================
Comment at: test/CodeGenOpenCL/sampler.cl:1
@@ +1,2 @@
+// RUN: %clang_cc1 %s -emit-llvm -triple spir-unknown-unknown -o - -O0
-Wspir-compat -verify -DCHECK_SAMPLER_VALUE | FileCheck %s
+// RUN: %clang_cc1 %s -emit-llvm -triple spir-unknown-unknown -o - -O0 -verify
| FileCheck %s
----------------
I think we can give this warning for spir target or -Wspir-compat.
================
Comment at: test/CodeGenOpenCL/sampler.cl:17
@@ +16,3 @@
+#ifdef CHECK_SAMPLER_VALUE
+// expected-warning@-2{{sampler initializer has invalid Filter Mode bits}}
+#endif
----------------
This should go into SemaOpenCL tests!
================
Comment at: test/CodeGenOpenCL/sampler.cl:27
@@ +26,3 @@
+
+void fnc4smp(sampler_t s) {}
+// CHECK: define spir_func void @fnc4smp(%__sampler addrspace(2)* %
----------------
Could we add CHECK-NOT respectively?
================
Comment at: test/CodeGenOpenCL/sampler.cl:37
@@ +36,3 @@
+
+ fnc4smp(smp);
+ // CHECK: [[SAMP:%[0-9]+]] = call %__sampler addrspace(2)*
@__translate_sampler_initializer(i32 19)
----------------
Can we add another call with smp and check that
__translate_sampler_initializeris is not called again?
http://reviews.llvm.org/D21567
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
