[PATCH] D15721: [Sema] Fix ICE on casting a vector of bools to a vector of T

[George Burgess IV via cfe-commits]

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

Clang generally treats booleans as 8-bit types, but lowers them to 1-bit types. This means we currently happily accept C++ code like:

```
typedef __attribute__((__ext_vector_type__(4))) bool BoolVector; 
typedef __attribute__((__ext_vector_type__(1))) int IntVector;

int main() {
  BoolVector bv;
  IntVector iv = (IntVector)bv;
}
```

...And lower it to a cast from a `[4 x i1]` to a `[1 x i32]`. Which gives us a nice greeting in the form of an ICE.

ISTM that `bool`s are only a valid element type in OpenCL vectors. The spec doesn't outline a definitive size for a `bool` (only that it must be able to support a 0 or 1), so I'm assuming that lowering to a vector of `i1` is fine.

http://reviews.llvm.org/D15721

Files:
  lib/Sema/SemaExpr.cpp
  test/CodeGenCXX/bool-vector-conversion.cpp

Index: test/CodeGenCXX/bool-vector-conversion.cpp
===================================================================
--- /dev/null
+++ test/CodeGenCXX/bool-vector-conversion.cpp
@@ -0,0 +1,25 @@
+// RUN: %clang_cc1 %s -emit-llvm -o - | FileCheck %s
+//
+// In many parts of clang, we treat a bool as an i8, but we lower it to an i1.
+// This is usually fine, but it broke things with vectors (e.g. we deemed a cast
+// from a [4 x i1] to a [1 x i32] to be legal, and proceeded to crash shortly
+// afterward).
+
+// Nothing but OpenCL allows vectors of booleans.
+// CHECK-LABEL: @_Z4testv
+void test() {
+  typedef __attribute__((__ext_vector_type__(8))) bool CLVectorBool8;
+  typedef __attribute__((__ext_vector_type__(1))) unsigned char CLVectorInt1;
+
+  // CHECK: store <8 x i1> zeroinitializer
+  CLVectorBool8 bools = (CLVectorBool8)false;
+  // CHECK: store <1 x i8>
+  CLVectorInt1 ints = (CLVectorInt1)bools;
+  // CHECK: store <8 x i1>
+  bools = (CLVectorBool8)ints;
+
+  // Run through the code in CGExprConstant.
+  // CHECK: store <8 x i1> zeroinitializer
+  bools = (CLVectorBool8)(CLVectorInt1)(CLVectorBool8)false;
+}
+
Index: lib/Sema/SemaExpr.cpp
===================================================================
--- lib/Sema/SemaExpr.cpp
+++ lib/Sema/SemaExpr.cpp
@@ -5552,9 +5552,14 @@
   // ASTContext::getTypeSize will return the size rounded up to a
   // power of 2, so instead of using that, we need to use the raw
   // element size multiplied by the element count.
-  uint64_t srcEltSize = Context.getTypeSize(srcEltTy);
-  uint64_t destEltSize = Context.getTypeSize(destEltTy);
-  
+  //
+  // We need to be careful about booleans though; they're lowered to i1s, but
+  // getTypeSize views them as i8s.
+  uint64_t srcEltSize =
+      srcEltTy->isBooleanType() ? 1 : Context.getTypeSize(srcEltTy);
+  uint64_t destEltSize =
+      destEltTy->isBooleanType() ? 1 : Context.getTypeSize(destEltTy);
+
   return (srcLen * srcEltSize == destLen * destEltSize);
 }
 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D15721.43463.patch
Type: text/x-patch
Size: 1996 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20151222/4098ae94/attachment.bin>