[cfe-dev] OpenCL patch: vector literals (alternative patch)
Tanya Lattner
lattner at apple.com
Thu Jul 7 16:41:57 PDT 2011
I'm attaching an alternative patch which handles the missing vector splat case. It passes all the valid and invalid test cases you provided and I included those tests in the patch. It also uses the fix in SemaInit that you included which handles the crash for one invalid case.
Thanks,
Tanya
-------------- next part --------------
A non-text attachment was scrubbed...
Name: OpenCL-VectorLiterals.patch
Type: application/octet-stream
Size: 2855 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20110707/c1e1313d/attachment.obj>
-------------- next part --------------
On Jul 7, 2011, at 8:17 AM, Anton Lokhmotov wrote:
> Hi Tanya,
>
> Many thanks for your prompt review!
>
>> All the support already exists for all the valid test cases
>> except the last one ((float4)( float )). One just has to modify
>> ActOnCastOfParenListExpr which is now BuildVectorLiteral to handle
>> the splat case. I have not modified our patch to match the recent
>> changes in TOT, but will do shortly.
>
> We would be happy with minimal changes to Clang provided that the OpenCL
> requirements are properly met, including handling invalid code. We
> specifically avoided using/modifying/breaking the existing functionality for
> AltiVec because the OpenCL syntax is quite different. Thus, we believe that
> for OpenCL CompilerInvocation::setLangDefaults() should disable Opts.AltiVec
> (and OTOH enable Opts.Bool - but sorry about that slipping into this patch).
>
>> Yes, you are 100% right that the test violates the OpenCL spec
>> (my mistake and haste in making a patch), but its catching a nasty
>> compiler crash and I'd rather try to modify the test instead of just
>> removing it. I've fixed this in TOT.
>
> Unfortunately,
>
> void foo( uchar8 x )
> {
> uchar4 val[4] = {{(uchar4){x.lo}}};
> }
>
> is still invalid in OpenCL C. In particular, OpenCL doesn't allow using
> braces {} in vector literals.
>
> [Actually, I'm quite confused by this test. Since x.lo produces uchar4, the
> cast is superfluous? And then it attempts to initialize an array of vectors
> val[] from vector x.lo. But I actually would expect only val[0] to be
> initialized (backed by my experiments with gcc). But if it's just ensuring
> that the compiler doesn't crash, that's fine (in the AltiVec mode).]
>
>> For the invalid cases, I don't see why one needs to modify the parser,
>> but the last invalid case does crash the compiler which shouldn't ever
>> happen.
>
> Could you please ensure that the patch is applied correctly? It works fine
> with r134483. Specifically, this modification prevents the compiler from
> crashing:
>
> [11:17:50] Vitaly Lugovskiy: diff --git a/lib/Sema/SemaInit.cpp
> b/lib/Sema/SemaInit.cpp
> index 16ba2a2..17af145 100644
> --- a/lib/Sema/SemaInit.cpp
> +++ b/lib/Sema/SemaInit.cpp
> @@ -769,7 +769,8 @@ void InitListChecker::CheckSubElementType(const
> InitializedEntity &Entity,
> // subaggregate, brace elision is assumed and the initializer is
> // considered for the initialization of the first member of
> // the subaggregate.
> - if (ElemType->isAggregateType() || ElemType->isVectorType()) {
> + if (!SemaRef.getLangOptions().OpenCL &&
> + (ElemType->isAggregateType() || ElemType->isVectorType())) {
> CheckImplicitInitList(Entity, IList, ElemType, Index, StructuredList,
> StructuredIndex);
> ++StructuredIndex;
>
> If you provide your modifications to support vector literals, we will run
> our test cases with combinatorial coverage to ensure that the OpenCL
> requirements are met.
>
> Best wishes,
> Anton.
>
>
>
More information about the cfe-dev
mailing list