Re: [PATCH] D14877: Fix ICE on lowering of constexpr vector splats

[George Burgess IV via cfe-commits]

george.burgess.iv marked 2 inline comments as done.
george.burgess.iv added a comment.

Ping :)


http://reviews.llvm.org/D14877



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D14877: Fix ICE on lowering of constexpr vector splats

[Richard Smith via cfe-commits]

rsmith accepted this revision.
This revision is now accepted and ready to land.


Comment at: lib/CodeGen/CGExprScalar.cpp:816-817
@@ +815,4 @@
+// the same as the vector's element type (sans qualifiers)
+assert(DstType->castAs()->getElementType().getTypePtr() ==
+   SrcType.getTypePtr() &&
+   "Splatted expr doesn't match with vector element type?");

The TypePtr can still have qualifiers in it in some cases. Use 
`ASTContext::hasSameType`.


http://reviews.llvm.org/D14877



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D14877: Fix ICE on lowering of constexpr vector splats

[George Burgess IV via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL257559: [Bugfix] Fix ICE on constexpr vector splat. 
(authored by gbiv).

Changed prior to commit:
  http://reviews.llvm.org/D14877?vs=44187=44701#toc

Repository:
  rL LLVM

http://reviews.llvm.org/D14877

Files:
  cfe/trunk/include/clang/AST/ASTContext.h
  cfe/trunk/include/clang/AST/OperationKinds.h
  cfe/trunk/include/clang/Sema/Sema.h
  cfe/trunk/lib/AST/Expr.cpp
  cfe/trunk/lib/AST/ExprConstant.cpp
  cfe/trunk/lib/CodeGen/CGExpr.cpp
  cfe/trunk/lib/CodeGen/CGExprAgg.cpp
  cfe/trunk/lib/CodeGen/CGExprComplex.cpp
  cfe/trunk/lib/CodeGen/CGExprConstant.cpp
  cfe/trunk/lib/CodeGen/CGExprScalar.cpp
  cfe/trunk/lib/Edit/RewriteObjCFoundationAPI.cpp
  cfe/trunk/lib/Sema/SemaCast.cpp
  cfe/trunk/lib/Sema/SemaChecking.cpp
  cfe/trunk/lib/Sema/SemaExpr.cpp
  cfe/trunk/lib/Sema/SemaExprCXX.cpp
  cfe/trunk/lib/Sema/SemaOverload.cpp
  cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineC.cpp
  cfe/trunk/test/CodeGenCXX/builtins-systemz-zvector.cpp
  cfe/trunk/test/CodeGenCXX/vector-splat-conversion.cpp
  cfe/trunk/test/CodeGenOpenCL/bool_cast.cl

Index: cfe/trunk/include/clang/AST/ASTContext.h
===
--- cfe/trunk/include/clang/AST/ASTContext.h
+++ cfe/trunk/include/clang/AST/ASTContext.h
@@ -2283,9 +2283,13 @@
   /// \brief Make an APSInt of the appropriate width and signedness for the
   /// given \p Value and integer \p Type.
   llvm::APSInt MakeIntValue(uint64_t Value, QualType Type) const {
-llvm::APSInt Res(getIntWidth(Type), 
- !Type->isSignedIntegerOrEnumerationType());
+// If Type is a signed integer type larger than 64 bits, we need to be sure
+// to sign extend Res appropriately.
+llvm::APSInt Res(64, !Type->isSignedIntegerOrEnumerationType());
 Res = Value;
+unsigned Width = getIntWidth(Type);
+if (Width != Res.getBitWidth())
+  return Res.extOrTrunc(Width);
 return Res;
   }
 
Index: cfe/trunk/include/clang/AST/OperationKinds.h
===
--- cfe/trunk/include/clang/AST/OperationKinds.h
+++ cfe/trunk/include/clang/AST/OperationKinds.h
@@ -185,7 +185,11 @@
   /// CK_FloatingToBoolean - Floating point to boolean.
   ///(bool) f
   CK_FloatingToBoolean,
-
+
+  // CK_BooleanToSignedIntegral - Convert a boolean to -1 or 0 for true and
+  // false, respectively.
+  CK_BooleanToSignedIntegral,
+
   /// CK_FloatingCast - Casting between floating types of different size.
   ///(double) f
   ///(float) ld
Index: cfe/trunk/include/clang/Sema/Sema.h
===
--- cfe/trunk/include/clang/Sema/Sema.h
+++ cfe/trunk/include/clang/Sema/Sema.h
@@ -8589,6 +8589,10 @@
   bool CheckVectorCast(SourceRange R, QualType VectorTy, QualType Ty,
CastKind );
 
+  /// \brief Prepare `SplattedExpr` for a vector splat operation, adding
+  /// implicit casts if necessary.
+  ExprResult prepareVectorSplat(QualType VectorTy, Expr *SplattedExpr);
+
   // CheckExtVectorCast - check type constraints for extended vectors.
   // Since vectors are an extension, there are no C standard reference for this.
   // We allow casting between vectors and integer datatypes of the same size,
Index: cfe/trunk/test/CodeGenCXX/vector-splat-conversion.cpp
===
--- cfe/trunk/test/CodeGenCXX/vector-splat-conversion.cpp
+++ cfe/trunk/test/CodeGenCXX/vector-splat-conversion.cpp
@@ -1,19 +1,51 @@
 // RUN: %clang_cc1 %s -triple arm64-apple-ios8.1.0 -std=c++11 -emit-llvm -o - | FileCheck %s
-// rdar://2762
 
 typedef __attribute__((__ext_vector_type__(8))) float vector_float8;
 
 typedef vector_float8 float8;
 
-void MandelbrotPolyCalcSIMD8()
-{
-constexpr float8   v4 = 4.0;  // value to compare against abs(z)^2, to see if bounded
-float8 vABS;
-auto vLT  = vABS < v4;
+// rdar://2762
+// CHECK-LABEL: define void @_Z23MandelbrotPolyCalcSIMD8v
+void MandelbrotPolyCalcSIMD8() {
+  constexpr float8 v4 = 4.0;  // value to compare against abs(z)^2, to see if bounded
+  float8 vABS;
+  auto vLT  = vABS < v4;
+  // CHECK: store <8 x float>
+  // CHECK: [[ZERO:%.*]] = load <8 x float>, <8 x float>* [[VARBS:%.*]]
+  // CHECK: [[CMP:%.*]] = fcmp olt <8 x float> [[ZERO]]
+  // CHECK: [[SEXT:%.*]] = sext <8 x i1> [[CMP]] to <8 x i32>
+  // CHECK: store <8 x i32> [[SEXT]], <8 x i32>* [[VLT:%.*]]
 }
 
-// CHECK: store <8 x float> 
-// CHECK: [[ZERO:%.*]] = load <8 x float>, <8 x float>* [[VARBS:%.*]]
-// CHECK: [[CMP:%.*]] = fcmp olt <8 x float> [[ZERO]]
-// CHECK: [[SEXT:%.*]] = sext <8 x i1> [[CMP]] to <8 x i32>
-// CHECK: store <8 x i32> [[SEXT]], <8 x i32>* [[VLT:%.*]]
+typedef __attribute__((__ext_vector_type__(4))) int int4;
+typedef __attribute__((__ext_vector_type__(4))) float float4;
+typedef __attribute__((__ext_vector_type__(4))) 

Re: [PATCH] D14877: Fix ICE on lowering of constexpr vector splats

[George Burgess IV via cfe-commits]

george.burgess.iv added a comment.

> Maybe we could remove CK_BooleanToSignedFloating and model that conversion as 
> a sequence of CK_BooleanToSignedIntegral followed by CK_IntegralToFloating? I 
> don't imagine it's a common operation, so the simpler AST representation is 
> probably worth more than the minor memory savings.


SGTM.


http://reviews.llvm.org/D14877



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D14877: Fix ICE on lowering of constexpr vector splats

[George Burgess IV via cfe-commits]

george.burgess.iv updated this revision to Diff 44187.
george.burgess.iv marked an inline comment as done.
george.burgess.iv added a comment.

Addressed all feedback, and added tests for `bool` -> `[n x i128]` case.


http://reviews.llvm.org/D14877

Files:
  include/clang/AST/ASTContext.h
  include/clang/AST/OperationKinds.h
  include/clang/Sema/Sema.h
  lib/AST/Expr.cpp
  lib/AST/ExprConstant.cpp
  lib/CodeGen/CGExpr.cpp
  lib/CodeGen/CGExprAgg.cpp
  lib/CodeGen/CGExprComplex.cpp
  lib/CodeGen/CGExprConstant.cpp
  lib/CodeGen/CGExprScalar.cpp
  lib/Edit/RewriteObjCFoundationAPI.cpp
  lib/Sema/SemaCast.cpp
  lib/Sema/SemaChecking.cpp
  lib/Sema/SemaExpr.cpp
  lib/Sema/SemaExprCXX.cpp
  lib/Sema/SemaOverload.cpp
  lib/StaticAnalyzer/Core/ExprEngineC.cpp
  test/CodeGenCXX/builtins-systemz-zvector.cpp
  test/CodeGenCXX/vector-splat-conversion.cpp
  test/CodeGenOpenCL/bool_cast.cl

Index: test/CodeGenOpenCL/bool_cast.cl
===
--- test/CodeGenOpenCL/bool_cast.cl
+++ test/CodeGenOpenCL/bool_cast.cl
@@ -2,7 +2,9 @@
 
 typedef unsigned char uchar4 __attribute((ext_vector_type(4)));
 typedef unsigned int int4 __attribute((ext_vector_type(4)));
+typedef float float4 __attribute((ext_vector_type(4)));
 
+// CHECK-LABEL: define void @ker()
 void kernel ker() {
   bool t = true;
   int4 vec4 = (int4)t;
@@ -24,4 +26,8 @@
   unsigned char c;
   c = (unsigned char)true;
 // CHECK: store i8 1, i8* %c, align 1
+
+  float4 vf;
+  vf = (float4)true;
+// CHECK: store <4 x float> 
 }
Index: test/CodeGenCXX/vector-splat-conversion.cpp
===
--- test/CodeGenCXX/vector-splat-conversion.cpp
+++ test/CodeGenCXX/vector-splat-conversion.cpp
@@ -1,19 +1,51 @@
 // RUN: %clang_cc1 %s -triple arm64-apple-ios8.1.0 -std=c++11 -emit-llvm -o - | FileCheck %s
-// rdar://2762
 
 typedef __attribute__((__ext_vector_type__(8))) float vector_float8;
 
 typedef vector_float8 float8;
 
-void MandelbrotPolyCalcSIMD8()
-{
-constexpr float8   v4 = 4.0;  // value to compare against abs(z)^2, to see if bounded
-float8 vABS;
-auto vLT  = vABS < v4;
+// rdar://2762
+// CHECK-LABEL: define void @_Z23MandelbrotPolyCalcSIMD8v
+void MandelbrotPolyCalcSIMD8() {
+  constexpr float8 v4 = 4.0;  // value to compare against abs(z)^2, to see if bounded
+  float8 vABS;
+  auto vLT  = vABS < v4;
+  // CHECK: store <8 x float>
+  // CHECK: [[ZERO:%.*]] = load <8 x float>, <8 x float>* [[VARBS:%.*]]
+  // CHECK: [[CMP:%.*]] = fcmp olt <8 x float> [[ZERO]]
+  // CHECK: [[SEXT:%.*]] = sext <8 x i1> [[CMP]] to <8 x i32>
+  // CHECK: store <8 x i32> [[SEXT]], <8 x i32>* [[VLT:%.*]]
 }
 
-// CHECK: store <8 x float> 
-// CHECK: [[ZERO:%.*]] = load <8 x float>, <8 x float>* [[VARBS:%.*]]
-// CHECK: [[CMP:%.*]] = fcmp olt <8 x float> [[ZERO]]
-// CHECK: [[SEXT:%.*]] = sext <8 x i1> [[CMP]] to <8 x i32>
-// CHECK: store <8 x i32> [[SEXT]], <8 x i32>* [[VLT:%.*]]
+typedef __attribute__((__ext_vector_type__(4))) int int4;
+typedef __attribute__((__ext_vector_type__(4))) float float4;
+typedef __attribute__((__ext_vector_type__(4))) __int128 bigint4;
+
+// CHECK-LABEL: define void @_Z14BoolConversionv
+void BoolConversion() {
+  // CHECK: store <4 x i32> 
+  int4 intsT = (int4)true;
+  // CHECK: store <4 x i32> zeroinitializer
+  int4 intsF = (int4)false;
+  // CHECK: store <4 x float> 
+  float4 floatsT = (float4)true;
+  // CHECK: store <4 x float> zeroinitializer
+  float4 floatsF = (float4)false;
+  // CHECK: store <4 x i128> 
+  bigint4 bigintsT = (bigint4)true;
+  // CHECK: store <4 x i128> zeroinitializer
+  bigint4 bigintsF = (bigint4)false;
+
+  // CHECK: store <4 x i32> 
+  constexpr int4 cIntsT = (int4)true;
+  // CHECK: store <4 x i32> zeroinitializer
+  constexpr int4 cIntsF = (int4)false;
+  // CHECK: store <4 x float> 
+  constexpr float4 cFloatsT = (float4)true;
+  // CHECK: store <4 x float> zeroinitializer
+  constexpr float4 cFloatsF = (float4)false;
+  // CHECK: store <4 x i128> 
+  constexpr bigint4 cBigintsT = (bigint4)true;
+  // CHECK: store <4 x i128> zeroinitializer
+  constexpr bigint4 cBigintsF = (bigint4)false;
+}
Index: test/CodeGenCXX/builtins-systemz-zvector.cpp
===
--- /dev/null
+++ test/CodeGenCXX/builtins-systemz-zvector.cpp
@@ -0,0 +1,50 @@
+// REQUIRES: systemz-registered-target
+// RUN: %clang_cc1 -target-cpu z13 -triple s390x-linux-gnu \
+// RUN: -fzvector -fno-lax-vector-conversions -std=c++11 \
+// RUN: -Wall -Wno-unused -Werror -emit-llvm %s -o - | FileCheck %s
+
+bool gb;
+
+// There was an issue where we weren't properly converting constexprs to
+// vectors with elements of the appropriate width. (e.g.
+// (vector signed short)0 would be lowered as [4 x i32] in some cases)
+
+// CHECK-LABEL: @_Z8testIntsDv4_i
+void testInts(vector int VI) {
+  constexpr vector int CI1 = (vector int)0LL;
+  // CHECK: icmp
+  gb = (VI == CI1)[0];
+
+  

Re: [PATCH] D14877: Fix ICE on lowering of constexpr vector splats

[Richard Smith via cfe-commits]

rsmith added a comment.

Maybe we could remove `CK_BooleanToSignedFloating` and model that conversion as 
a sequence of `CK_BooleanToSignedIntegral` followed by `CK_IntegralToFloating`? 
I don't imagine it's a common operation, so the simpler AST representation is 
probably worth more than the minor memory savings.



Comment at: lib/AST/ExprConstant.cpp:7794
@@ -7790,1 +7793,3 @@
+  IntResult = (uint64_t)-1;
+return Success(IntResult, E);
   }

I think this will produce the wrong value when the destination type of the cast 
is `__int128`. We should probably fix this by making `ASTContext::MakeIntValue` 
sign-extend if necessary when the target type is signed.


http://reviews.llvm.org/D14877



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D14877: Fix ICE on lowering of constexpr vector splats

[George Burgess IV via cfe-commits]

george.burgess.iv added a comment.

Ping :)


http://reviews.llvm.org/D14877



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D14877: Fix ICE on lowering of constexpr vector splats

[George Burgess IV via cfe-commits]

george.burgess.iv added a comment.

Ping :)


http://reviews.llvm.org/D14877



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D14877: Fix ICE on lowering of constexpr vector splats

[George Burgess IV via cfe-commits]

george.burgess.iv updated this revision to Diff 42683.
george.burgess.iv marked an inline comment as done.
george.burgess.iv added a comment.

> Changes to ExprConstant and CGExprConstant appear to be pure cleanups; please 
> check those in as a separate change


r255314, thanks.

> When this is done, you should also be able to remove the IgnoreImpCasts and 
> EmitScalarConversion calls in the CK_VectorSplat handling in 
> ScalarExprEmitter::VisitCastExpr.


Yup!

Aside: I can't figure out how to get c++11 constexpr evaluation in to work with 
OpenCL (vector expressions apparently aren't considered ICEs in non-c++11 
mode). So, the bit of special code in ExprConstant is untested at the moment. 
I'm open to suggestions for how to test it; I tried things like `int 
arr[((int4)true)[0] == -1 ? 1 : -1];`/tricks with `enable_if`, but wasn't quite 
able to get it to do what I wanted. :)


http://reviews.llvm.org/D14877

Files:
  include/clang/AST/OperationKinds.h
  include/clang/Sema/Sema.h
  lib/AST/Expr.cpp
  lib/AST/ExprConstant.cpp
  lib/CodeGen/CGExpr.cpp
  lib/CodeGen/CGExprAgg.cpp
  lib/CodeGen/CGExprComplex.cpp
  lib/CodeGen/CGExprConstant.cpp
  lib/CodeGen/CGExprScalar.cpp
  lib/Edit/RewriteObjCFoundationAPI.cpp
  lib/Sema/SemaCast.cpp
  lib/Sema/SemaChecking.cpp
  lib/Sema/SemaExpr.cpp
  lib/Sema/SemaExprCXX.cpp
  lib/Sema/SemaOverload.cpp
  lib/StaticAnalyzer/Core/ExprEngineC.cpp
  test/CodeGenCXX/builtins-systemz-zvector.cpp
  test/CodeGenOpenCL/bool_cast.cl

Index: test/CodeGenOpenCL/bool_cast.cl
===
--- test/CodeGenOpenCL/bool_cast.cl
+++ test/CodeGenOpenCL/bool_cast.cl
@@ -2,7 +2,9 @@
 
 typedef unsigned char uchar4 __attribute((ext_vector_type(4)));
 typedef unsigned int int4 __attribute((ext_vector_type(4)));
+typedef float float4 __attribute((ext_vector_type(4)));
 
+// CHECK-LABEL: define void @ker()
 void kernel ker() {
   bool t = true;
   int4 vec4 = (int4)t;
@@ -24,4 +26,8 @@
   unsigned char c;
   c = (unsigned char)true;
 // CHECK: store i8 1, i8* %c, align 1
+
+  float4 vf;
+  vf = (float4)true;
+// CHECK: store <4 x float> 
 }
Index: test/CodeGenCXX/builtins-systemz-zvector.cpp
===
--- /dev/null
+++ test/CodeGenCXX/builtins-systemz-zvector.cpp
@@ -0,0 +1,50 @@
+// REQUIRES: systemz-registered-target
+// RUN: %clang_cc1 -target-cpu z13 -triple s390x-linux-gnu \
+// RUN: -fzvector -fno-lax-vector-conversions -std=c++11 \
+// RUN: -Wall -Wno-unused -Werror -emit-llvm %s -o - | FileCheck %s
+
+bool gb;
+
+// There was an issue where we weren't properly converting constexprs to
+// vectors with elements of the appropriate width. (e.g.
+// (vector signed short)0 would be lowered as [4 x i32] in some cases)
+
+// CHECK-LABEL: @_Z8testIntsDv4_i
+void testInts(vector int VI) {
+  constexpr vector int CI1 = (vector int)0LL;
+  // CHECK: icmp
+  gb = (VI == CI1)[0];
+
+  // Likewise for float inits.
+  constexpr vector int CI2 = (vector int)char(0);
+  // CHECK: icmp
+  gb = (VI == CI2)[0];
+
+  constexpr vector int CF1 = (vector int)0.0;
+  // CHECK: icmp
+  gb = (VI == CF1)[0];
+
+  constexpr vector int CF2 = (vector int)0.0f;
+  // CHECK: icmp
+  gb = (VI == CF2)[0];
+}
+
+// CHECK-LABEL: @_Z10testFloatsDv2_d
+void testFloats(vector double VD) {
+  constexpr vector double CI1 = (vector double)0LL;
+  // CHECK: fcmp
+  gb = (VD == CI1)[0];
+
+  // Likewise for float inits.
+  constexpr vector double CI2 = (vector double)char(0);
+  // CHECK: fcmp
+  gb = (VD == CI2)[0];
+
+  constexpr vector double CF1 = (vector double)0.0;
+  // CHECK: fcmp
+  gb = (VD == CF1)[0];
+
+  constexpr vector double CF2 = (vector double)0.0f;
+  // CHECK: fcmp
+  gb = (VD == CF2)[0];
+}
Index: lib/StaticAnalyzer/Core/ExprEngineC.cpp
===
--- lib/StaticAnalyzer/Core/ExprEngineC.cpp
+++ lib/StaticAnalyzer/Core/ExprEngineC.cpp
@@ -317,6 +317,8 @@
   case CK_BitCast:
   case CK_AddressSpaceConversion:
   case CK_IntegralCast:
+  case CK_BooleanToSignedIntegral:
+  case CK_BooleanToSignedFloating:
   case CK_NullToPointer:
   case CK_IntegralToPointer:
   case CK_PointerToIntegral:
@@ -345,6 +347,10 @@
 // Delegate to SValBuilder to process.
 SVal V = state->getSVal(Ex, LCtx);
 V = svalBuilder.evalCast(V, T, ExTy);
+// Negate the result if we're treating the boolean as a signed i1
+if (CastE->getCastKind() == CK_BooleanToSignedFloating ||
+CastE->getCastKind() == CK_BooleanToSignedIntegral)
+  V = evalMinus(V);
 state = state->BindExpr(CastE, LCtx, V);
 Bldr.generateNode(CastE, Pred, state);
 continue;
Index: lib/Sema/SemaOverload.cpp
===
--- lib/Sema/SemaOverload.cpp
+++ lib/Sema/SemaOverload.cpp
@@ -258,6 +258,8 @@
 case CK_IntegralCast:
 case 

Re: [PATCH] D14877: Fix ICE on lowering of constexpr vector splats

[George Burgess IV via cfe-commits]

george.burgess.iv added inline comments.


Comment at: lib/Sema/SemaExpr.cpp:5576
@@ +5575,3 @@
+return ExprError();
+  return ImpCastExprToType(CastExprRes.get(), DestElemTy, CK);
+}

rsmith wrote:
> Looking at `ScalarExprEmitter::VisitCastExpr`, it seems like we are supposed 
> to do something slightly bizarre if the source type is `bool` and we're in 
> OpenCL mode -- in that case we're supposed to convert `true` to -1 instead of 
> 1. In order for ExprConstant to get that case right, we should emit the 
> appropriate implicit cast for that conversion here -- maybe add a 
> `CK_SignedBooleanToIntegral` and a `CK_SignedBooleanToFloating`; I don't see 
> any nice way to express this with our existing cast kinds.
...And this behavior is only present when splatting. Joy.


http://reviews.llvm.org/D14877



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D14877: Fix ICE on lowering of constexpr vector splats

[George Burgess IV via cfe-commits]

george.burgess.iv updated this revision to Diff 42686.
george.burgess.iv added a comment.

TIL you can use OpenCL vectors in C++ files. Fixed a bug related to not knowing 
this + added tests for the constexpr cases.

Also, is there a better way to do the `bool` -> `APFloat` conversion in 
ExprConstant, lines 8108-8110? I'm assuming that float semantics matter, 
otherwise `Result = APFloat(-BoolResult)` seems nicer.


http://reviews.llvm.org/D14877

Files:
  include/clang/AST/OperationKinds.h
  include/clang/Sema/Sema.h
  lib/AST/Expr.cpp
  lib/AST/ExprConstant.cpp
  lib/CodeGen/CGExpr.cpp
  lib/CodeGen/CGExprAgg.cpp
  lib/CodeGen/CGExprComplex.cpp
  lib/CodeGen/CGExprConstant.cpp
  lib/CodeGen/CGExprScalar.cpp
  lib/Edit/RewriteObjCFoundationAPI.cpp
  lib/Sema/SemaCast.cpp
  lib/Sema/SemaChecking.cpp
  lib/Sema/SemaExpr.cpp
  lib/Sema/SemaExprCXX.cpp
  lib/Sema/SemaOverload.cpp
  lib/StaticAnalyzer/Core/ExprEngineC.cpp
  test/CodeGenCXX/builtins-systemz-zvector.cpp
  test/CodeGenCXX/vector-splat-conversion.cpp
  test/CodeGenOpenCL/bool_cast.cl

Index: test/CodeGenOpenCL/bool_cast.cl
===
--- test/CodeGenOpenCL/bool_cast.cl
+++ test/CodeGenOpenCL/bool_cast.cl
@@ -2,7 +2,9 @@
 
 typedef unsigned char uchar4 __attribute((ext_vector_type(4)));
 typedef unsigned int int4 __attribute((ext_vector_type(4)));
+typedef float float4 __attribute((ext_vector_type(4)));
 
+// CHECK-LABEL: define void @ker()
 void kernel ker() {
   bool t = true;
   int4 vec4 = (int4)t;
@@ -24,4 +26,8 @@
   unsigned char c;
   c = (unsigned char)true;
 // CHECK: store i8 1, i8* %c, align 1
+
+  float4 vf;
+  vf = (float4)true;
+// CHECK: store <4 x float> 
 }
Index: test/CodeGenCXX/vector-splat-conversion.cpp
===
--- test/CodeGenCXX/vector-splat-conversion.cpp
+++ test/CodeGenCXX/vector-splat-conversion.cpp
@@ -1,19 +1,42 @@
 // RUN: %clang_cc1 %s -triple arm64-apple-ios8.1.0 -std=c++11 -emit-llvm -o - | FileCheck %s
-// rdar://2762
 
 typedef __attribute__((__ext_vector_type__(8))) float vector_float8;
 
 typedef vector_float8 float8;
 
-void MandelbrotPolyCalcSIMD8()
-{
-constexpr float8   v4 = 4.0;  // value to compare against abs(z)^2, to see if bounded
-float8 vABS;
-auto vLT  = vABS < v4;
+// rdar://2762
+// CHECK-LABEL: define void @_Z23MandelbrotPolyCalcSIMD8v
+void MandelbrotPolyCalcSIMD8() {
+  constexpr float8 v4 = 4.0;  // value to compare against abs(z)^2, to see if bounded
+  float8 vABS;
+  auto vLT  = vABS < v4;
+  // CHECK: store <8 x float> 
+  // CHECK: [[ZERO:%.*]] = load <8 x float>, <8 x float>* [[VARBS:%.*]]
+  // CHECK: [[CMP:%.*]] = fcmp olt <8 x float> [[ZERO]]
+  // CHECK: [[SEXT:%.*]] = sext <8 x i1> [[CMP]] to <8 x i32>
+  // CHECK: store <8 x i32> [[SEXT]], <8 x i32>* [[VLT:%.*]]
 }
 
-// CHECK: store <8 x float> 
-// CHECK: [[ZERO:%.*]] = load <8 x float>, <8 x float>* [[VARBS:%.*]]
-// CHECK: [[CMP:%.*]] = fcmp olt <8 x float> [[ZERO]]
-// CHECK: [[SEXT:%.*]] = sext <8 x i1> [[CMP]] to <8 x i32>
-// CHECK: store <8 x i32> [[SEXT]], <8 x i32>* [[VLT:%.*]]
+typedef __attribute__((__ext_vector_type__(4))) int int4;
+typedef __attribute__((__ext_vector_type__(4))) float float4;
+
+// CHECK-LABEL: define void @_Z14BoolConversionv
+void BoolConversion() {
+  // CHECK: store <4 x i32> 
+  int4 intsT = (int4)true;
+  // CHECK: store <4 x i32> zeroinitializer
+  int4 intsF = (int4)false;
+  // CHECK: store <4 x float> 
+  float4 floatsT = (float4)true;
+  // CHECK: store <4 x float> zeroinitializer
+  float4 floatsF = (float4)false;
+
+  // CHECK: store <4 x i32> 
+  constexpr int4 cIntsT = (int4)true;
+  // CHECK: store <4 x i32> zeroinitializer
+  constexpr int4 cIntsF = (int4)false;
+  // CHECK: store <4 x float> 
+  constexpr float4 cFloatsT = (float4)true;
+  // CHECK: store <4 x float> zeroinitializer
+  constexpr float4 cFloatsF = (float4)false;
+}
Index: test/CodeGenCXX/builtins-systemz-zvector.cpp
===
--- /dev/null
+++ test/CodeGenCXX/builtins-systemz-zvector.cpp
@@ -0,0 +1,50 @@
+// REQUIRES: systemz-registered-target
+// RUN: %clang_cc1 -target-cpu z13 -triple s390x-linux-gnu \
+// RUN: -fzvector -fno-lax-vector-conversions -std=c++11 \
+// RUN: -Wall -Wno-unused -Werror -emit-llvm %s -o - | FileCheck %s
+
+bool gb;
+
+// There was an issue where we weren't properly converting constexprs to
+// vectors with elements of the appropriate width. (e.g.
+// (vector signed short)0 would be lowered as [4 x i32] in some cases)
+
+// CHECK-LABEL: @_Z8testIntsDv4_i
+void testInts(vector int VI) {
+  constexpr vector int CI1 = (vector int)0LL;
+  // CHECK: icmp
+  gb = (VI == CI1)[0];
+
+  // Likewise for float inits.
+  constexpr vector int CI2 = (vector int)char(0);
+  // CHECK: icmp
+  gb = (VI == CI2)[0];
+
+  constexpr vector int CF1 = (vector int)0.0;
+  // CHECK: icmp
+  gb = (VI == CF1)[0];
+
+  

Re: [PATCH] D14877: Fix ICE on lowering of constexpr vector splats

[Richard Smith via cfe-commits]

rsmith added a comment.

Changes to ExprConstant and CGExprConstant appear to be pure cleanups; please 
check those in as a separate change.

When this is done, you should also be able to remove the `IgnoreImpCasts` and 
`EmitScalarConversion` calls in the `CK_VectorSplat` handling in 
`ScalarExprEmitter::VisitCastExpr`.



Comment at: lib/Sema/SemaExpr.cpp:5576
@@ +5575,3 @@
+return ExprError();
+  return ImpCastExprToType(CastExprRes.get(), DestElemTy, CK);
+}

Looking at `ScalarExprEmitter::VisitCastExpr`, it seems like we are supposed to 
do something slightly bizarre if the source type is `bool` and we're in OpenCL 
mode -- in that case we're supposed to convert `true` to -1 instead of 1. In 
order for ExprConstant to get that case right, we should emit the appropriate 
implicit cast for that conversion here -- maybe add a 
`CK_SignedBooleanToIntegral` and a `CK_SignedBooleanToFloating`; I don't see 
any nice way to express this with our existing cast kinds.


http://reviews.llvm.org/D14877



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D14877: Fix ICE on lowering of constexpr vector splats

[George Burgess IV via cfe-commits]

george.burgess.iv added a comment.

Ping :)


http://reviews.llvm.org/D14877



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D14877: Fix ICE on lowering of constexpr vector splats

[George Burgess IV via cfe-commits]

george.burgess.iv updated this revision to Diff 40895.
george.burgess.iv added a comment.

We now add implicit casts to splatted literals, as Richard suggested, instead 
of trying to handle this as a special case in `ExprConstant`.


http://reviews.llvm.org/D14877

Files:
  include/clang/Sema/Sema.h
  lib/AST/ExprConstant.cpp
  lib/CodeGen/CGExprConstant.cpp
  lib/Sema/SemaCast.cpp
  lib/Sema/SemaExpr.cpp
  lib/Sema/SemaExprCXX.cpp
  test/CodeGenCXX/builtins-systemz-zvector.cpp

Index: test/CodeGenCXX/builtins-systemz-zvector.cpp
===
--- /dev/null
+++ test/CodeGenCXX/builtins-systemz-zvector.cpp
@@ -0,0 +1,50 @@
+// REQUIRES: systemz-registered-target
+// RUN: %clang_cc1 -target-cpu z13 -triple s390x-linux-gnu \
+// RUN: -fzvector -fno-lax-vector-conversions -std=c++11 \
+// RUN: -Wall -Wno-unused -Werror -emit-llvm %s -o - | FileCheck %s
+
+bool gb;
+
+// There was an issue where we weren't properly converting constexprs to
+// vectors with elements of the appropriate width. (e.g.
+// (vector signed short)0 would be lowered as [4 x i32] in some cases)
+
+// CHECK-LABEL: @_Z8testIntsDv4_i
+void testInts(vector int VI) {
+  constexpr vector int CI1 = (vector int)0LL;
+  // CHECK: icmp
+  gb = (VI == CI1)[0];
+
+  // Likewise for float inits.
+  constexpr vector int CI2 = (vector int)char(0);
+  // CHECK: icmp
+  gb = (VI == CI2)[0];
+
+  constexpr vector int CF1 = (vector int)0.0;
+  // CHECK: icmp
+  gb = (VI == CF1)[0];
+
+  constexpr vector int CF2 = (vector int)0.0f;
+  // CHECK: icmp
+  gb = (VI == CF2)[0];
+}
+
+// CHECK-LABEL: @_Z10testFloatsDv2_d
+void testFloats(vector double VD) {
+  constexpr vector double CI1 = (vector double)0LL;
+  // CHECK: fcmp
+  gb = (VD == CI1)[0];
+
+  // Likewise for float inits.
+  constexpr vector double CI2 = (vector double)char(0);
+  // CHECK: fcmp
+  gb = (VD == CI2)[0];
+
+  constexpr vector double CF1 = (vector double)0.0;
+  // CHECK: fcmp
+  gb = (VD == CF1)[0];
+
+  constexpr vector double CF2 = (vector double)0.0f;
+  // CHECK: fcmp
+  gb = (VD == CF2)[0];
+}
Index: lib/Sema/SemaExprCXX.cpp
===
--- lib/Sema/SemaExprCXX.cpp
+++ lib/Sema/SemaExprCXX.cpp
@@ -3352,15 +3352,9 @@
 
   case ICK_Vector_Splat:
 // Vector splat from any arithmetic type to a vector.
-// Cast to the element type.
 {
-  QualType elType = ToType->getAs()->getElementType();
-  if (elType != From->getType()) {
-ExprResult E = From;
-From = ImpCastExprToType(From, elType,
- PrepareScalarCast(E, elType)).get();
-  }
-  From = ImpCastExprToType(From, ToType, CK_VectorSplat,
+  Expr *Elem = prepareVectorSplat(ToType, From).get();
+  From = ImpCastExprToType(Elem, ToType, CK_VectorSplat,
VK_RValue, /*BasePath=*/nullptr, CCK).get();
 }
 break;
Index: lib/Sema/SemaExpr.cpp
===
--- lib/Sema/SemaExpr.cpp
+++ lib/Sema/SemaExpr.cpp
@@ -5563,6 +5563,19 @@
   return false;
 }
 
+ExprResult Sema::prepareVectorSplat(QualType VectorTy, Expr *SplattedExpr) {
+  QualType DestElemTy = VectorTy->castAs()->getElementType();
+
+  if (DestElemTy == SplattedExpr->getType())
+return SplattedExpr;
+
+  ExprResult CastExprRes = SplattedExpr;
+  CastKind CK = PrepareScalarCast(CastExprRes, DestElemTy);
+  if (CastExprRes.isInvalid())
+return ExprError();
+  return ImpCastExprToType(CastExprRes.get(), DestElemTy, CK);
+}
+
 ExprResult Sema::CheckExtVectorCast(SourceRange R, QualType DestTy,
 Expr *CastExpr, CastKind ) {
   assert(DestTy->isExtVectorType() && "Not an extended vector type!");
@@ -5593,15 +5606,8 @@
 diag::err_invalid_conversion_between_vector_and_scalar)
   << DestTy << SrcTy << R;
 
-  QualType DestElemTy = DestTy->getAs()->getElementType();
-  ExprResult CastExprRes = CastExpr;
-  CastKind CK = PrepareScalarCast(CastExprRes, DestElemTy);
-  if (CastExprRes.isInvalid())
-return ExprError();
-  CastExpr = ImpCastExprToType(CastExprRes.get(), DestElemTy, CK).get();
-
   Kind = CK_VectorSplat;
-  return CastExpr;
+  return prepareVectorSplat(DestTy, CastExpr);
 }
 
 ExprResult
@@ -6942,13 +6948,9 @@
 if (RHSType->isExtVectorType())
   return Incompatible;
 if (RHSType->isArithmeticType()) {
-  // CK_VectorSplat does T -> vector T, so first cast to the
-  // element type.
-  QualType elType = cast(LHSType)->getElementType();
-  if (elType != RHSType && ConvertRHS) {
-Kind = PrepareScalarCast(RHS, elType);
-RHS = ImpCastExprToType(RHS.get(), elType, Kind);
-  }
+  // CK_VectorSplat does T -> vector T, so first cast to the element type.
+  if (ConvertRHS)
+RHS = prepareVectorSplat(LHSType, RHS.get());
   Kind = CK_VectorSplat;
   return Compatible;
 }
@@ 

Re: [PATCH] D14877: Fix ICE on lowering of constexpr vector splats

[George Burgess IV via cfe-commits]

george.burgess.iv added inline comments.


Comment at: lib/CodeGen/CGExprConstant.cpp:1362-1363
@@ -1360,3 +1361,4 @@
+Inits[I] = llvm::ConstantFP::get(VMContext, Elt.getFloat());
   else
-Inits.push_back(llvm::ConstantFP::get(VMContext, Elt.getFloat()));
+llvm_unreachable("unsupported vector element type");
 }

majnemer wrote:
> Is this unreachable for vectors of pointer type?
Might you have an example of how to use a vector of pointers in clang? My 
knowledge of vectors as a whole is super limited, so I'm probably missing 
something obvious, but I can't seem to get any of the vector types mentioned 
here 
http://clang.llvm.org/docs/LanguageExtensions.html#vectors-and-extended-vectors 
to accept a pointer element type, nor is my grep-fu good enough to find e.g. a 
`vector (int*)` in `test/`. 


http://reviews.llvm.org/D14877



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D14877: Fix ICE on lowering of constexpr vector splats

[Richard Smith via cfe-commits]

It would seem cleaner to build an ImplicitCastExpr node in Sema between the
operand and the splat node.
On Nov 20, 2015 11:04 AM, "George Burgess IV" 
wrote:

> george.burgess.iv created this revision.
> george.burgess.iv added a reviewer: rsmith.
> george.burgess.iv added a subscriber: cfe-commits.
>
> When evaluating constexpr vector splats, we weren't doing appropriate type
> conversions on the literal we were splatting, causing assertion failures in
> cases like:
>
> ```
> void foo(vector float FloatInput, vector short ShortInput) {
>   (void)(FloatInput == (vector float)0); // OK
>   (void)(ShortInput == (vector short)0); // OK
>
>   constexpr vector float Floats = (vector float)0;
>   (void)(FloatInput == Floats); // ICE -- fcmp between [4 x i32] and [4 x
> f32]
>
>   constexpr vector short Shorts = (vector short)0;
>   (void)(ShortInput == Shorts); // ICE -- fcmp between vec of i16 and vec
> of i32
> }
> ```
>
> (The same issue applied for cases like `(vector short)0`; it would be
> lowered as a vector of `i32`.)
>
> This patch fixes these in ExprConstant rather than CodeGen, because it
> allows us to more sanely model overflow/complain to the user/... in the
> evaluator.
>
> This patch also contains a few generic code cleanliness changes. I'm happy
> to drop any/all of them if we decide they're not helpful. :)
>
> http://reviews.llvm.org/D14877
>
> Files:
>   lib/AST/ExprConstant.cpp
>   lib/CodeGen/CGExprConstant.cpp
>   test/CodeGenCXX/builtins-systemz-zvector.cpp
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D14877: Fix ICE on lowering of constexpr vector splats

[David Majnemer via cfe-commits]

majnemer added a subscriber: majnemer.


Comment at: lib/CodeGen/CGExprConstant.cpp:1362-1363
@@ -1360,3 +1361,4 @@
+Inits[I] = llvm::ConstantFP::get(VMContext, Elt.getFloat());
   else
-Inits.push_back(llvm::ConstantFP::get(VMContext, Elt.getFloat()));
+llvm_unreachable("unsupported vector element type");
 }

Is this unreachable for vectors of pointer type?


http://reviews.llvm.org/D14877



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D14877: Fix ICE on lowering of constexpr vector splats

[George Burgess IV via cfe-commits]

george.burgess.iv created this revision.
george.burgess.iv added a reviewer: rsmith.
george.burgess.iv added a subscriber: cfe-commits.

When evaluating constexpr vector splats, we weren't doing appropriate type 
conversions on the literal we were splatting, causing assertion failures in 
cases like:

```
void foo(vector float FloatInput, vector short ShortInput) {
  (void)(FloatInput == (vector float)0); // OK
  (void)(ShortInput == (vector short)0); // OK

  constexpr vector float Floats = (vector float)0;
  (void)(FloatInput == Floats); // ICE -- fcmp between [4 x i32] and [4 x f32]

  constexpr vector short Shorts = (vector short)0;
  (void)(ShortInput == Shorts); // ICE -- fcmp between vec of i16 and vec of i32
}
```

(The same issue applied for cases like `(vector short)0`; it would be lowered 
as a vector of `i32`.)

This patch fixes these in ExprConstant rather than CodeGen, because it allows 
us to more sanely model overflow/complain to the user/... in the evaluator. 

This patch also contains a few generic code cleanliness changes. I'm happy to 
drop any/all of them if we decide they're not helpful. :)

http://reviews.llvm.org/D14877

Files:
  lib/AST/ExprConstant.cpp
  lib/CodeGen/CGExprConstant.cpp
  test/CodeGenCXX/builtins-systemz-zvector.cpp

Index: test/CodeGenCXX/builtins-systemz-zvector.cpp
===
--- /dev/null
+++ test/CodeGenCXX/builtins-systemz-zvector.cpp
@@ -0,0 +1,52 @@
+// REQUIRES: systemz-registered-target
+// RUN: %clang_cc1 -target-cpu z13 -triple s390x-linux-gnu \
+// RUN: -fzvector -fno-lax-vector-conversions -std=c++11 \
+// RUN: -Wall -Wno-unused -Werror -emit-llvm %s -o - | FileCheck %s
+
+#include 
+
+bool gb;
+
+// There was an issue where we weren't properly converting constexprs to
+// vectors with elements of the appropriate width. (e.g.
+// (vector signed short)0 would be lowered as [4 x i32] in some cases)
+
+// CHECK-LABEL: @_Z8testIntsDv4_i
+void testInts(vector int VI) {
+  constexpr vector int CI1 = (vector int)0LL;
+  // CHECK: icmp
+  gb = (VI == CI1)[0];
+
+  // Likewise for float inits.
+  constexpr vector int CI2 = (vector int)char(0);
+  // CHECK: icmp
+  gb = (VI == CI2)[0];
+
+  constexpr vector int CF1 = (vector int)0.0;
+  // CHECK: icmp
+  gb = (VI == CF1)[0];
+
+  constexpr vector int CF2 = (vector int)0.0f;
+  // CHECK: icmp
+  gb = (VI == CF2)[0];
+}
+
+// CHECK-LABEL: @_Z10testFloatsDv2_d
+void testFloats(vector double VD) {
+  constexpr vector double CI1 = (vector double)0LL;
+  // CHECK: fcmp
+  gb = (VD == CI1)[0];
+
+  // Likewise for float inits.
+  constexpr vector double CI2 = (vector double)char(0);
+  // CHECK: fcmp
+  gb = (VD == CI2)[0];
+
+  constexpr vector double CF1 = (vector double)0.0;
+  // CHECK: fcmp
+  gb = (VD == CF1)[0];
+
+  constexpr vector double CF2 = (vector double)0.0f;
+  // CHECK: fcmp
+  gb = (VD == CF2)[0];
+}
Index: lib/CodeGen/CGExprConstant.cpp
===
--- lib/CodeGen/CGExprConstant.cpp
+++ lib/CodeGen/CGExprConstant.cpp
@@ -1350,15 +1350,17 @@
 return llvm::ConstantStruct::get(STy, Complex);
   }
   case APValue::Vector: {
-SmallVector Inits;
 unsigned NumElts = Value.getVectorLength();
+SmallVector Inits(NumElts);
 
-for (unsigned i = 0; i != NumElts; ++i) {
-  const APValue  = Value.getVectorElt(i);
+for (unsigned I = 0; I != NumElts; ++I) {
+  const APValue  = Value.getVectorElt(I);
   if (Elt.isInt())
-Inits.push_back(llvm::ConstantInt::get(VMContext, Elt.getInt()));
+Inits[I] = llvm::ConstantInt::get(VMContext, Elt.getInt());
+  else if (Elt.isFloat())
+Inits[I] = llvm::ConstantFP::get(VMContext, Elt.getFloat());
   else
-Inits.push_back(llvm::ConstantFP::get(VMContext, Elt.getFloat()));
+llvm_unreachable("unsupported vector element type");
 }
 return llvm::ConstantVector::get(Inits);
   }
Index: lib/AST/ExprConstant.cpp
===
--- lib/AST/ExprConstant.cpp
+++ lib/AST/ExprConstant.cpp
@@ -1150,7 +1150,7 @@
 static bool EvaluateMemberPointer(const Expr *E, MemberPtr ,
   EvalInfo );
 static bool EvaluateTemporary(const Expr *E, LValue , EvalInfo );
-static bool EvaluateInteger(const Expr *E, APSInt  , EvalInfo );
+static bool EvaluateInteger(const Expr *E, APSInt , EvalInfo );
 static bool EvaluateIntegerOrLValue(const Expr *E, APValue ,
 EvalInfo );
 static bool EvaluateFloat(const Expr *E, APFloat , EvalInfo );
@@ -1575,7 +1575,7 @@
 
 static APSInt HandleIntToIntCast(EvalInfo , const Expr *E,
  QualType DestType, QualType SrcType,
- APSInt ) {
+ const APSInt ) {
   unsigned DestWidth = Info.Ctx.getIntWidth(DestType);
   APSInt Result = Value;
   // Figure out if this is a