[cfe-dev] OpenCL patch: vector literals

[Anton Lokhmotov]

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.