[cfe-dev] OpenCL & SPIR specific types - proposal and patch
Tanya Lattner
lattner at apple.com
Mon Oct 29 23:55:51 PDT 2012
Sorry for the delay, I was out of town.
+static bool TryOCLSamplerInitialization(Sema &S,
+ InitializationSequence &Sequence,
+ QualType DestType) {
+ if (!S.getLangOpts().OpenCL || !DestType->isSamplerT())
+ return false;
+
+ Sequence.AddOCLSamplerInitStep(DestType);
+}
generates a new warning since it doesn't return anything after the first check.
There are also a bunch of warnings generated because TST_image1d_t are not handled in switch statements. Is there other code missing from this patch? For example, CIndexUSRs.cpp, CIndex.cpp, SemaTemplateVariadic.cpp.
I also don't believe sampler is quite right. Running the test, I see:
opencl_types.cl:3:11: error: initializing 'sampler_t' with an expression of incompatible type
'int'
sampler_t glb_smp = 7;
^ ~
opencl_types.cl:25:12: error: initializing 'sampler_t' with an expression of incompatible type
'int'
sampler_t smp = 5;
Also, I don't think this implementation supports function overloading (which we use all over the place for builtins). So for example:
void __OVERLOAD__ bar(image1d_t X) {
}
void __OVERLOAD__ bar(image2d_t X) {
}
this causes the compiler to crash.
Assertion failed: (OldFn->isDeclaration() && "Shouldn't replace non-declaration"), function EmitGlobalFunctionDefinition, file CodeGenModule.cpp, line 1908.
0 clang 0x00000001057e91fe PrintStackTrace(void*) + 46
1 clang 0x00000001057e97a9 SignalHandler(int) + 297
2 libsystem_c.dylib 0x00007fff8aee78ea _sigtramp + 26
3 libsystem_c.dylib 0x00007fff5c9b74e8 _sigtramp + 18446744072932359192
4 clang 0x00000001057e94cb raise + 27
5 clang 0x00000001057e9582 abort + 18
6 clang 0x00000001057e9561 __assert_rtn + 129
7 clang 0x0000000103619a98 clang::CodeGen::CodeGenModule::EmitGlobalFunctionDefinition(clang::GlobalDecl) + 424
8 clang 0x0000000103616da2 clang::CodeGen::CodeGenModule::EmitGlobalDefinition(clang::GlobalDecl) + 498
9 clang 0x000000010361901e clang::CodeGen::CodeGenModule::EmitGlobal(clang::GlobalDecl) + 782
10 clang 0x000000010361f98d clang::CodeGen::CodeGenModule::EmitTopLevelDecl(clang::Decl*) + 269
11 clang 0x0000000103664684 (anonymous namespace)::CodeGeneratorImpl::HandleTopLevelDecl(clang::DeclGroupRef) + 100
12 clang 0x0000000103607d75 clang::BackendConsumer::HandleTopLevelDecl(clang::DeclGroupRef) + 181
13 clang 0x000000010367852b clang::ParseAST(clang::Sema&, bool, bool) + 507
14 clang 0x000000010330f7f8 clang::ASTFrontendAction::ExecuteAction() + 312
15 clang 0x0000000103607252 clang::CodeGenAction::ExecuteAction() + 1266
16 clang 0x000000010330f40b clang::FrontendAction::Execute() + 235
17 clang 0x00000001032d766f clang::CompilerInstance::ExecuteAction(clang::FrontendAction&) + 863
18 clang 0x000000010325d4f7 clang::ExecuteCompilerInvocation(clang::CompilerInstance*) + 999
19 clang 0x0000000103245d66 cc1_main(char const**, char const**, char const*, void*) + 1094
20 clang 0x0000000103256c89 main + 473
21 libdyld.dylib 0x00007fff95e387e1 start + 0
22 libdyld.dylib 0x0000000000000007 start + 18446603338001446950
Stack dump:
0. Program arguments: ../../Debug+Asserts/bin/clang -cc1 -emit-llvm -O0 -o - x.cl
1. <eof> parser at end of file
2. x.cl:9:20: LLVM IR generation of declaration 'read_imagef'
3. x.cl:9:20: Generating code for declaration 'read_imagef'
Illegal instruction
I'm also not clear on if sampler's should be mangled too. I had done this in my implementation of sampler.
How much testing have you done with this patch? Is it just on the test case provided? Do you have a reference implementation in Clang of OpenCL that passes conformance?
-Tanya
On Oct 24, 2012, at 4:36 PM, "Benyei, Guy" <guy.benyei at intel.com> wrote:
> Hi all,
> Please review this patch – I think I’ve fixed every controversial part of the original patch. The sampler type is now represented as i32, but it becomes a clang builtin type to enable efficient restriction checking. Also fixed every remark I’ve got for the different versions of the patch.
>
> Thanks
>
> <image001.png>
>
> From: cfe-dev-bounces at cs.uiuc.edu [mailto:cfe-dev-bounces at cs.uiuc.edu] On Behalf Of Benyei, Guy
> Sent: Sunday, October 21, 2012 16:00
> To: Tanya Lattner
> Cc: Richard Smith; cfe-dev at cs.uiuc.edu; Anton.Lokhmotov at arm.com
> Subject: Re: [cfe-dev] OpenCL & SPIR specific types - proposal and patch
>
> Tanya,
> I’ve fixed the width/alignment and the debug info remarks, and also removed the sampler initialization function for now.
>
> The main idea in passing through the OpenCLRuntime for initializing samplers is enabling the initialization of non-integer vendor specific samplers later on. It’s not required for the current implementation, but it will be needed later on to support other representations of the sampler type, like in SPIR.
>
> I prefer to leave sampler_t implemented with an i32 representation rather than removing it – the built-in types are needed for clean restriction checking.
>
> Thanks
> Guy
> <image001.png>
>
> From: Tanya Lattner [mailto:lattner at apple.com]
> Sent: Saturday, October 20, 2012 07:45
> To: Benyei, Guy
> Cc: Richard Smith; cfe-dev at cs.uiuc.edu; Villmow, Micah; Anton.Lokhmotov at arm.com
> Subject: Re: [cfe-dev] OpenCL & SPIR specific types - proposal and patch
>
> Guy,
>
>
> + case BuiltinType::OCLSampler:
> + case BuiltinType::OCLEvent:
> + case BuiltinType::OCLImage1d:
> + case BuiltinType::OCLImage1dArray:
> + case BuiltinType::OCLImage1dBuffer:
> + case BuiltinType::OCLImage2d:
> + case BuiltinType::OCLImage2dArray:
> + case BuiltinType::OCLImage3d:
> + // Currently these types are pointers to opaque types.
> + Width = Target->getPointerWidth(0);
> + Align = Target->getPointerAlign(0);
> + break;
> }
>
> Is wrong if sampler is i32 I think.
>
> Also, probably wrong:
> + case BuiltinType::OCLSampler:
> + return getOrCreateStructPtrType("opencl_sampler_t", OCLSamplerDITy);
>
> Why do this?
> + } else if (type->isSamplerT()) {
> + CGM.getOpenCLRuntime().initializeOpenCLSampler(this, lvalue.getAddress(), EmitScalarExpr(init));
> }
>
> I also don't understand why you need to go through the OpenCLRuntime for this.
> + if (D->getType()->isSamplerT()) {
> + getOpenCLRuntime().initializeOpenCLSampler(NULL, GV, Init);
> + } else {
> + GV->setInitializer(Init);
> + }
>
> I would rather just see sampler ripped out for now. I have no issues with the images/event code.
>
> -Tanya
>
>
> …
>
>
> <opencl_types6.patch>
> ---------------------------------------------------------------------
> Intel Israel (74) Limited
>
> This e-mail and any attachments may contain confidential material for
> the sole use of the intended recipient(s). Any review or distribution
> by others is strictly prohibited. If you are not the intended
> recipient, please contact the sender and delete all copies.
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20121029/32beb313/attachment.html>
More information about the cfe-dev
mailing list