[OpenCL] Allow OpenCL C style vector initialization in C++

[Anastasia]

Summary

I am recreating the review from https://reviews.llvm.org/D59426 that seem to get lost somehow.

The idea is to be able to allow creating vector literals from other vectors.

Example from reported bug PR41010

kernel void test(__global float4 *out)
{
    float4 a = (float4)(1.0f, 2.0f, 3.0f, 4.0f);
    float4 v = (float4)(a.s23, a.s01);
    *out = v;
}

[rjmccall]

Wait, plain C++? Do we currently allow this syntax outside of OpenCL?

[Anastasia]

In D65286#1601404, @rjmccall wrote:

Wait, plain C++? Do we currently allow this syntax outside of OpenCL?

Ok I looked into this again and it seems apparently vector initialization and literals in C++ have different syntax. So I was incorrect, this fix only works for OpenCL. Not sure how important it is to initialize vectors from other vectors in C++ though. In OpenCL C it is common.

[rjmccall]

Okay. It seems reasonable to me to allow OpenCL vector initialization syntax in OpenCL C++, since we're treating OpenCL C++ as an ObjC++-like merge of the language extensions, but we shouldn't go further than that and start changing behavior in the non-OpenCL languages.

[mantognini]

In vector_literals_nested.cl, we have tests for (global) constants. Do you think it would be worth testing those as well in C++ mode? Maybe the two files (vector_literals_nested.cl and vector_literals_valid.cl) should also be merged as most of their content seems duplicated.

In C++, we have the comma operator and therefore the AST is significantly different. Running the content of your test file with -x c++ shows that it is rejected as desired. It could be worth having a negative test to ensure we always reject this in vanilla C++.

Regarding the code itself, I'm not familiar with InitializationSequence/InitListChecker that much, but the patch makes sense I would say.

[Anastasia]

• Merged test/CodeGenOpenCL/vector_literals_nested.cl into test/CodeGenOpenCL/vector_literals_valid.cl
• Added negative test case for C++

[Anastasia]

• Removed another duplicate file and added testing of constant program scope initializers into vector_literals_nested.cl

[Anastasia]

In D65286#1606071, @mantognini wrote:

In vector_literals_nested.cl, we have tests for (global) constants. Do you think it would be worth testing those as well in C++ mode? Maybe the two files (vector_literals_nested.cl and vector_literals_valid.cl) should also be merged as most of their content seems duplicated.

Thanks for pointing out. Indeed there was lots of repetition. I only added one missing test case (line 37 of vector_literals_valid.cl) and also removed another similar file test/SemaOpenCL/vector_literals_const.cl.

In C++, we have the comma operator and therefore the AST is significantly different. Running the content of your test file with -x c++ shows that it is rejected as desired. It could be worth having a negative test to ensure we always reject this in vanilla C++.

Added test case in test/SemaCXX/vector.cpp. However I am now confused what syntax we shouldn't accept exactly. @rjmccall do you think there should be an error on line 341?

Regarding the code itself, I'm not familiar with InitializationSequence/InitListChecker that much, but the patch makes sense I would say.

Ok, thanks. It seems apparently the last time this code was modified in 2015... so might not be easy to remember details for anyone. :(

[mantognini]

LGTM, thanks for the update.

[rjmccall]

In D65286#1606441, @Anastasia wrote:

In D65286#1606071, @mantognini wrote:

In vector_literals_nested.cl, we have tests for (global) constants. Do you think it would be worth testing those as well in C++ mode? Maybe the two files (vector_literals_nested.cl and vector_literals_valid.cl) should also be merged as most of their content seems duplicated.

Thanks for pointing out. Indeed there was lots of repetition. I only added one missing test case (line 37 of vector_literals_valid.cl) and also removed another similar file test/SemaOpenCL/vector_literals_const.cl.

In C++, we have the comma operator and therefore the AST is significantly different. Running the content of your test file with -x c++ shows that it is rejected as desired. It could be worth having a negative test to ensure we always reject this in vanilla C++.

Added test case in test/SemaCXX/vector.cpp. However I am now confused what syntax we shouldn't accept exactly. @rjmccall do you think there should be an error on line 341?

No, it's correct that this is accepted as a vector splat. The warning is good, although it would be even better if we could find a way to warn about this more specifically, like "warning: this syntax does not construct a vector; initial elements are ignored" (needs workshopping, obviously).

[ebrevnov]

Please be aware about build bot failure (http://lab.llvm.org:8011/builders/clang-cmake-thumbv7-full-sh/builds/2185) most likely caused by this change.

[Anastasia]

In D65286#1614239, @ebrevnov wrote:

Please be aware about build bot failure (http://lab.llvm.org:8011/builders/clang-cmake-thumbv7-full-sh/builds/2185) most likely caused by this change.

Thanks! I believe it is now fixed in http://llvm.org/viewvc/llvm-project?view=revision&revision=367823.

cfe/trunk/lib/Sema/SemaInit.cpp

void InitListChecker::CheckSubElementType(const InitializedEntity &Entity,
   if (SemaRef.getLangOpts().CPlusPlus) {
     // C++ [dcl.init.aggr]p2:
     //   Each member is copy-initialized from the corresponding
     //   initializer-clause.
 
     // FIXME: Better EqualLoc?
     InitializationKind Kind =
         InitializationKind::CreateCopy(expr->getBeginLoc(), SourceLocation());
-    InitializationSequence Seq(SemaRef, Entity, Kind, expr,
+
+    // Vector elements can be initialized from other vectors in which case
+    // we need initialization entity with a type of a vector (and not a vector
+    // element!) initializing multiple vector elements.
+    auto TmpEntity =
+        (ElemType->isExtVectorType() && !Entity.getType()->isExtVectorType())
+            ? InitializedEntity::InitializeTemporary(ElemType)
+            : Entity;
+
+    InitializationSequence Seq(SemaRef, TmpEntity, Kind, expr,
                                /*TopLevelOfInitList*/ true);
 
     // C++14 [dcl.init.aggr]p13:
     //   If the assignment-expression can initialize a member, the member is
     //   initialized. Otherwise [...] brace elision is assumed
     //
     // Brace elision is never performed if the element is not an
     // assignment-expression.
     if (Seq || isa<InitListExpr>(expr)) {
       if (!VerifyOnly) {
-        ExprResult Result =
-          Seq.Perform(SemaRef, Entity, Kind, expr);
+        ExprResult Result = Seq.Perform(SemaRef, TmpEntity, Kind, expr);
         if (Result.isInvalid())
           hadError = true;
 
         UpdateStructuredListElement(StructuredList, StructuredIndex,
                                     Result.getAs<Expr>());
       } else if (!Seq)
         hadError = true;
       ++Index;

cfe/trunk/test/CodeGenOpenCL/vector_literals_nested.cl

-// RUN: %clang_cc1 %s -emit-llvm -O3 -o - | FileCheck %s
-
-typedef int int2 __attribute((ext_vector_type(2)));
-typedef int int4 __attribute((ext_vector_type(4)));
-
-__constant const int4 itest1 = (int4)(1, 2, ((int2)(3, 4)));
-// CHECK: constant <4 x i32> <i32 1, i32 2, i32 3, i32 4>
-__constant const int4 itest2 = (int4)(1, 2, ((int2)(3)));
-// CHECK: constant <4 x i32> <i32 1, i32 2, i32 3, i32 3>
-
-typedef float float2 __attribute((ext_vector_type(2)));
-typedef float float4 __attribute((ext_vector_type(4)));
-
-void ftest1(float4 *p) {
-  *p = (float4)(1.1f, 1.2f, ((float2)(1.3f, 1.4f)));
-// CHECK: store <4 x float> <float 0x3FF19999A0000000, float 0x3FF3333340000000, float 0x3FF4CCCCC0000000, float 0x3FF6666660000000>
-}
-
-float4 ftest2(float4 *p) {
-   *p =  (float4)(1.1f, 1.2f, ((float2)(1.3f)));
-// CHECK: store <4 x float> <float 0x3FF19999A0000000, float 0x3FF3333340000000, float 0x3FF4CCCCC0000000, float 0x3FF4CCCCC0000000>
-}
-

cfe/trunk/test/CodeGenOpenCL/vector_literals_valid.cl

-// RUN: %clang_cc1 -emit-llvm %s -o %t
+// RUN: %clang_cc1 -emit-llvm %s -o - -O0 | FileCheck %s
+// RUN: %clang_cc1 -emit-llvm %s -o - -cl-std=clc++ -O0 | FileCheck %s
 
-typedef __attribute__(( ext_vector_type(2) ))  int int2;
-typedef __attribute__(( ext_vector_type(3) ))  int int3;
-typedef __attribute__(( ext_vector_type(4) ))  int int4;
-typedef __attribute__(( ext_vector_type(8) ))  int int8;
-typedef __attribute__(( ext_vector_type(4) ))  float float4;
+typedef __attribute__((ext_vector_type(2))) int int2;
+typedef __attribute__((ext_vector_type(3))) int int3;
+typedef __attribute__((ext_vector_type(4)))  int int4;
+typedef __attribute__((ext_vector_type(8)))  int int8;
+typedef __attribute__((ext_vector_type(4))) float float4;
 
+__constant const int4 c1 = (int4)(1, 2, ((int2)(3)));
+// CHECK: constant <4 x i32> <i32 1, i32 2, i32 3, i32 3>
+
+__constant const int4 c2 = (int4)(1, 2, ((int2)(3, 4)));
+// CHECK: constant <4 x i32> <i32 1, i32 2, i32 3, i32 4>
+
 void vector_literals_valid() {
-  int4 a_1_1_1_1 = (int4)(1,2,3,4);
-  int4 a_2_1_1 = (int4)((int2)(1,2),3,4);
-  int4 a_1_2_1 = (int4)(1,(int2)(2,3),4);
-  int4 a_1_1_2 = (int4)(1,2,(int2)(3,4));
-  int4 a_2_2 = (int4)((int2)(1,2),(int2)(3,4));
-  int4 a_3_1 = (int4)((int3)(1,2,3),4);
-  int4 a_1_3 = (int4)(1,(int3)(2,3,4));
+  //CHECK: insertelement <4 x i32> <i32 1, i32 2, i32 undef, i32 undef>, i32 %{{.+}}, i32 2
+  //CHECK: insertelement <4 x i32> %{{.+}}, i32 %{{.+}}, i32 3
+  int4 a_1_1_1_1 = (int4)(1, 2, c1.s2, c2.s3);
+
+  //CHECK: store <2 x i32> <i32 1, i32 2>, <2 x i32>*
+  //CHECK: shufflevector <2 x i32> %{{[0-9]+}}, <2 x i32> undef, <4 x i32> <i32 0, i32 1, i32 undef, i32 undef>
+  //CHECK: shufflevector <4 x i32> %{{.+}}, <4 x i32> undef, <4 x i32> <i32 0, i32 1, i32 undef, i32 undef>
+  //CHECK: insertelement <4 x i32> %{{.+}}, i32 3, i32 2
+  //CHECK: insertelement <4 x i32> %{{.+}}, i32 4, i32 3
+  int4 a_2_1_1 = (int4)((int2)(1, 2), 3, 4);
+
+  //CHECK: store <2 x i32> <i32 2, i32 3>, <2 x i32>*
+  //CHECK: shufflevector <2 x i32> %{{[0-9]+}}, <2 x i32> undef, <4 x i32> <i32 0, i32 1, i32 undef, i32 undef>
+  //CHECK: shufflevector <4 x i32> <i32 1, i32 undef, i32 undef, i32 undef>, <4 x i32> %{{.+}}, <4 x i32> <i32 0, i32 4, i32 5, i32 undef>
+  //CHECK: insertelement <4 x i32> %{{.+}}, i32 4, i32 3
+  int4 a_1_2_1 = (int4)(1, (int2)(2, 3), 4);
+
+  //CHECK: store <2 x i32> <i32 3, i32 4>, <2 x i32>*
+  //CHECK: shufflevector <2 x i32> %{{[0-9]+}}, <2 x i32> undef, <4 x i32> <i32 0, i32 1, i32 undef, i32 undef>
+  //CHECK: shufflevector <4 x i32> <i32 1, i32 2, i32 undef, i32 undef>, <4 x i32> %{{.+}}, <4 x i32> <i32 0, i32 1, i32 4, i32 5>
+  int4 a_1_1_2 = (int4)(1, 2, (int2)(3, 4));
+
+  //CHECK: store <2 x i32> <i32 1, i32 2>, <2 x i32>*
+  //CHECK: shufflevector <2 x i32> %{{[0-9]+}}, <2 x i32> undef, <4 x i32> <i32 0, i32 1, i32 undef, i32 undef>
+  //CHECK: shufflevector <4 x i32> %{{.+}}, <4 x i32> undef, <4 x i32> <i32 0, i32 1, i32 undef, i32 undef>
+  //CHECK: shufflevector <4 x i32> %{{.+}}, <4 x i32> <i32 3, i32 3, i32 undef, i32 undef>, <4 x i32> <i32 0, i32 1, i32 4, i32 5>
+  int4 a_2_2 = (int4)((int2)(1, 2), (int2)(3));
+
+  //CHECK: store <4 x i32> <i32 2, i32 3, i32 4, i32 undef>, <4 x i32>*
+  //CHECK: shufflevector <4 x i32> %{{.+}}, <4 x i32> undef, <3 x i32> <i32 0, i32 1, i32 2>
+  //CHECK: shufflevector <3 x i32> %{{.+}}, <3 x i32> undef, <4 x i32> <i32 0, i32 1, i32 2, i32 undef>
+  //CHECK: shufflevector <4 x i32> <i32 1, i32 undef, i32 undef, i32 undef>, <4 x i32> %{{.+}}, <4 x i32> <i32 0, i32 4, i32 5, i32 6>
+  int4 a_1_3 = (int4)(1, (int3)(2, 3, 4));
+
+  //CHECK: store <4 x i32> <i32 1, i32 1, i32 1, i32 1>, <4 x i32>* %a
   int4 a = (int4)(1);
-  int8 b = (int8)(1,2,a.xy,a);
-  float4 V2 = (float4) (1);
+
+  //CHECK: load <4 x i32>, <4 x i32>* %a, align 16
+  //CHECK: shufflevector <4 x i32> %{{[0-9]+}}, <4 x i32> undef, <2 x i32> <i32 0, i32 1>
+  //CHECK: shufflevector <2 x i32> %{{[0-9]+}}, <2 x i32> undef, <8 x i32> <i32 0, i32 1, i32 undef, i32 undef, i32 undef, i32 undef, i32 undef, i32 undef>
+  //CHECK: shufflevector <8 x i32> <i32 1, i32 2, i32 undef, i32 undef, i32 undef, i32 undef, i32 undef, i32 undef>, <8 x i32> %{{.+}}, <8 x i32> <i32 0, i32 1, i32 8, i32 9, i32 undef, i32 undef, i32 undef, i32 undef>
+  //CHECK: load <4 x i32>, <4 x i32>* %a, align 16
+  //CHECK: shufflevector <4 x i32> %{{[0-9]+}}, <4 x i32> undef, <8 x i32> <i32 0, i32 1, i32 2, i32 3, i32 undef, i32 undef, i32 undef, i32 undef>
+  //CHECK: shufflevector <8 x i32> %{{.+}}, <8 x i32> %{{.+}}, <8 x i32> <i32 0, i32 1, i32 2, i32 3, i32 8, i32 9, i32 10, i32 11>
+  int8 b = (int8)(1, 2, a.xy, a);
+
+  //CHECK: store <4 x float> <float 1.000000e+00, float 1.000000e+00, float 1.000000e+00, float 1.000000e+00>, <4 x float>* %V2
+  float4 V2 = (float4)(1);
 }
-
-

cfe/trunk/test/SemaCXX/vector.cpp

void Init() {
   // expected-note@+1 {{in instantiation of template class 'Templates::PR15730<8, int>' requested here}}
   const PR15730<8, int>::type PR15730_1 = {};
   // expected-error@304 {{vector size too large}}
   // expected-note@+1 {{in instantiation of template class 'Templates::PR15730<8, char>' requested here}}
   const PR15730<8, char>::type2 PR15730_2 = {};
 }
 
 } // namespace Templates
+
+typedef int inte2 __attribute__((__ext_vector_type__(2)));
+
+void test_vector_literal(inte4 res) {
+  inte2 a = (inte2)(1, 2); //expected-warning{{expression result unused}}
+  inte4 b = (inte4)(a, a); //expected-error{{C-style cast from vector 'inte2' (vector of 2 'int' values) to vector 'inte4' (vector of 4 'int' values) of different size}} //expected-warning{{expression result unused}}
+}
+

cfe/trunk/test/SemaOpenCL/vector_literals_const.cl

-// RUN: %clang_cc1 %s -verify -pedantic -fsyntax-only
-// expected-no-diagnostics
-
-typedef int int2 __attribute((ext_vector_type(2)));
-typedef int int3 __attribute((ext_vector_type(3)));
-typedef int int4 __attribute((ext_vector_type(4)));
-
-__constant int4 i_1_1_1_1 = (int4)(1,2,3,4);
-__constant int4 i_2_1_1 = (int4)((int2)(1,2),3,4);
-__constant int4 i_1_2_1 = (int4)(1,(int2)(2,3),4);
-__constant int4 i_1_1_2 = (int4)(1,2,(int2)(3,4));
-__constant int4 i_2_2 = (int4)((int2)(1,2),(int2)(3,4));
-__constant int4 i_3_1 = (int4)((int3)(1,2,3),4);
-__constant int4 i_1_3 = (int4)(1,(int3)(2,3,4));
-
-typedef float float2 __attribute((ext_vector_type(2)));
-typedef float float3 __attribute((ext_vector_type(3)));
-typedef float float4 __attribute((ext_vector_type(4)));
-
-__constant float4 f_1_1_1_1 = (float4)(1,2,3,4);
-__constant float4 f_2_1_1 = (float4)((float2)(1,2),3,4);
-__constant float4 f_1_2_1 = (float4)(1,(float2)(2,3),4);
-__constant float4 f_1_1_2 = (float4)(1,2,(float2)(3,4));
-__constant float4 f_2_2 = (float4)((float2)(1,2),(float2)(3,4));
-__constant float4 f_3_1 = (float4)((float3)(1,2,3),4);
-__constant float4 f_1_3 = (float4)(1,(float3)(2,3,4));
-