[PATCH] D20133: [OpenCL] Fix __builtin_astype for vec3 types.

[Anastasia Stulova via cfe-commits]

Anastasia added inline comments.

================
Comment at: lib/CodeGen/CGExprScalar.cpp:3394
@@ +3393,3 @@
+  if (NumElementsDst == 4)
+    Args.push_back(llvm::UndefValue::get(CGF.Int32Ty));
+  llvm::Constant *Mask = llvm::ConstantVector::get(Args);
----------------
should this be 3 unstead of undef?

================
Comment at: lib/CodeGen/CGExprScalar.cpp:3428
@@ -3429,3 +3427,3 @@
   }
 
   return Builder.CreateBitCast(Src, DstTy, "astype");
----------------
I see. Not related to your change, but I was just wondering if it would be better to change this to a Clang builtin with a custom check at some point. It would be easier to understand and we can avoid all this parsing/AST handling complications.

================
Comment at: test/CodeGenOpenCL/as_type.cl:8
@@ +7,3 @@
+
+//CHECK: define spir_func <3 x i8> @f1(<4 x i8> %[[x:.*]])
+//CHECK: %[[astype:.*]] = shufflevector <4 x i8> %[[x]], <4 x i8> undef, <3 x i32> <i32 0, i32 1, i32 2>
----------------
So what happens if the number of bytes don't match?

================
Comment at: test/CodeGenOpenCL/as_type.cl:46
@@ +45,3 @@
+//CHECK: define spir_func i32 @f6(<4 x i8> %[[x:.*]])
+//CHECK: %[[astype]] = bitcast <4 x i8> %[[x]] to i32
+//CHECK: ret i32 %[[astype]]
----------------
Would it make sense to check that shufflevector is not generated?

================
Comment at: test/CodeGenOpenCL/as_type.cl:53
@@ +52,3 @@
+//CHECK: define spir_func <3 x i8> @f7(<3 x i8> %[[x:.*]])
+//CHECK: ret <3 x i8> %[[x]]
+char3 f7(char3 x) {
----------------
Could we add CHECK-NOT bitcast here?


http://reviews.llvm.org/D20133