[PATCH] D24467: Fix an error after D21678

2016-09-12 Thread Vladimir Yakovlev via cfe-commits 
vbyakovlcl created this revision.
vbyakovlcl added reviewers: ahatanak, aaron.ballman.
vbyakovlcl added subscribers: andreybokhanko, cfe-commits.
vbyakovlcl set the repository for this revision to rL LLVM.
vbyakovlcl changed the visibility of this Differential Revision from "Public 
(No Login Required)" to "All Users".
Herald added a subscriber: aemerson.

This fixes an error and gcc incompatibilities in vector shifts reported by 
Akira Hatanaka

--
From: Akira Hatanaka 
Date: Fri, Sep 2, 2016 at 3:00 AM
Subject: Re: [PATCH] D21678: Fix For pr28288 - Error message in shift of vector 
values
To: vladimi...@gmail.com, ulrich.weig...@de.ibm.com, amjad.ab...@intel.com, 
guy.ben...@intel.com, aaron.ball...@gmail.com
Cc: ahata...@gmail.com, andreybokha...@gmail.com, anastasia.stul...@arm.com, 
dmitry.poluk...@gmail.com, cfe-commits@lists.llvm.org


ahatanak added a subscriber: ahatanak.
ahatanak added a comment.

This patch causes clang to error out on the following code, which used to 
compile fine:

$ cat f2.c

  typedef __attribute__((__ext_vector_type__(8))) unsigned short vector_ushort8;

  vector_ushort8 foo1(void) {
return 1 << (vector_ushort8){7,6,5,4,3,2,1,0};
  }

$ clang f2.c -c

clang used to transform the scaler operand to a vector operand (similar to the 
way gcc's vector is handled) when compiling for normal c/c++ (but printed an 
error message when compiling for opencl), but this patch dropped the check for 
LangOpts added in r230464 and changed that behavior. I don't think this was 
intentional?


Repository:
  rL LLVM

https://reviews.llvm.org/D21678

Repository:
  rL LLVM

https://reviews.llvm.org/D24467

Files:
  llvm/tools/clang/include/clang/Basic/DiagnosticSemaKinds.td
  llvm/tools/clang/lib/Sema/SemaExpr.cpp
  llvm/tools/clang/test/Sema/vecshift.c

Index: llvm/tools/clang/lib/Sema/SemaExpr.cpp
===
--- llvm/tools/clang/lib/Sema/SemaExpr.cpp
+++ llvm/tools/clang/lib/Sema/SemaExpr.cpp
@@ -8721,24 +8721,28 @@
 static QualType checkVectorShift(Sema &S, ExprResult &LHS, ExprResult &RHS,
  SourceLocation Loc, bool IsCompAssign) {
   // OpenCL v1.1 s6.3.j says RHS can be a vector only if LHS is a vector.
-  if (!LHS.get()->getType()->isVectorType()) {
+  if ((S.LangOpts.OpenCL || S.LangOpts.ZVector) &&
+  !LHS.get()->getType()->isVectorType()) {
 S.Diag(Loc, diag::err_shift_rhs_only_vector)
   << RHS.get()->getType() << LHS.get()->getType()
   << LHS.get()->getSourceRange() << RHS.get()->getSourceRange();
 return QualType();
   }
 
   if (!IsCompAssign) {
-LHS = S.UsualUnaryConversions(LHS.get());
+if (S.LangOpts.OpenCL || S.LangOpts.ZVector)
+  LHS = S.UsualUnaryConversions(LHS.get());
+else
+  LHS = S.DefaultFunctionArrayLvalueConversion(LHS.get());
 if (LHS.isInvalid()) return QualType();
   }
 
   RHS = S.UsualUnaryConversions(RHS.get());
   if (RHS.isInvalid()) return QualType();
 
   QualType LHSType = LHS.get()->getType();
-  const VectorType *LHSVecTy = LHSType->castAs();
-  QualType LHSEleType = LHSVecTy->getElementType();
+  const VectorType *LHSVecTy = LHSType->getAs();
+  QualType LHSEleType = LHSVecTy ? LHSVecTy->getElementType() : LHSType;
 
   // Note that RHS might not be a vector.
   QualType RHSType = RHS.get()->getType();
@@ -8758,7 +8762,19 @@
 return QualType();
   }
 
-  if (RHSVecTy) {
+  if (!LHSVecTy) {
+assert(RHSVecTy);
+if (IsCompAssign)
+  return RHSType;
+if (LHSEleType != RHSEleType) {
+  LHS = S.ImpCastExprToType(LHS.get(),RHSEleType, CK_IntegralCast);
+  LHSEleType = RHSEleType;
+}
+QualType VecTy =
+S.Context.getExtVectorType(LHSEleType, RHSVecTy->getNumElements());
+LHS = S.ImpCastExprToType(LHS.get(), VecTy, CK_VectorSplat);
+LHSType = VecTy;
+  } else if (RHSVecTy) {
 // OpenCL v1.1 s6.3.j says that for vector types, the operators
 // are applied component-wise. So if RHS is a vector, then ensure
 // that the number of elements is the same as LHS...
@@ -8768,6 +8784,62 @@
 << LHS.get()->getSourceRange() << RHS.get()->getSourceRange();
   return QualType();
 }
+if (!S.LangOpts.OpenCL && !S.LangOpts.ZVector) {
+  const BuiltinType *LHSBT = LHSEleType->getAs();
+  const BuiltinType *RHSBT = RHSEleType->getAs();
+  if (LHSBT != RHSBT) {
+BuiltinType::Kind LHSKind = LHSBT->getKind();
+BuiltinType::Kind RHSKind = RHSBT->getKind();
+bool DiffSizes = true;
+switch (LHSKind) {
+case BuiltinType::Char_S:
+  DiffSizes =
+  RHSKind != BuiltinType::Char_U && RHSKind != BuiltinType::UChar;
+  break;
+case BuiltinType::Char_U:
+  DiffSizes =
+  RHSKind != BuiltinType::Char_S && RHSKind != BuiltinType::UChar;
+  break;
+case BuiltinType::UChar:
+  Diff

Re: [PATCH] D24467: Fix an error after D21678

2016-09-12 Thread Akira Hatanaka via cfe-commits 
ahatanak added inline comments.


Comment at: llvm/tools/clang/lib/Sema/SemaExpr.cpp:8733
@@ -8731,2 +8732,3 @@
   if (!IsCompAssign) {
-LHS = S.UsualUnaryConversions(LHS.get());
+if (S.LangOpts.OpenCL || S.LangOpts.ZVector)
+  LHS = S.UsualUnaryConversions(LHS.get());

It wasn't clear to me why we have to call DefaultFunctionArrayLvalueConversion 
instead of UsualUnaryConversions. Is it possible to come up with a test case 
that would fail without this change, and if it is, can you add it to vecshift.c?


Comment at: llvm/tools/clang/lib/Sema/SemaExpr.cpp:8747
@@ -8742,3 +8746,3 @@
 
   // Note that RHS might not be a vector.
   QualType RHSType = RHS.get()->getType();

We no longer error out when LHS is not a vector, so it should mention that 
either the LHS or the RHS might not be a vector.


Comment at: llvm/tools/clang/lib/Sema/SemaExpr.cpp:8752
@@ -8747,3 +8751,3 @@
 
   // OpenCL v1.1 s6.3.j says that the operands need to be integers.
   if (!LHSEleType->isIntegerType()) {

This comment should be updated since other vector types (e.g., gcc's vector 
type) are handled by this function too.


Comment at: llvm/tools/clang/lib/Sema/SemaExpr.cpp:8774
@@ +8773,3 @@
+QualType VecTy =
+S.Context.getExtVectorType(LHSEleType, RHSVecTy->getNumElements());
+LHS = S.ImpCastExprToType(LHS.get(), VecTy, CK_VectorSplat);

Is it correct to always create an ExtVectorType here? What if the RHS is a 
GenericVector?


Comment at: llvm/tools/clang/lib/Sema/SemaExpr.cpp:8787
@@ -8770,1 +8786,3 @@
 }
+if (!S.LangOpts.OpenCL && !S.LangOpts.ZVector) {
+  const BuiltinType *LHSBT = LHSEleType->getAs();

Is it possible to split this out to another patch? The first patch would fix 
handling of (scalar << vector) and the second patch would make clang reject 
vector shifts if element sizes of the LHS and RHS are different. It's not clear 
whether it's correct to reject the latter case, so perhaps you can discuss it 
in a separate patch? 


Comment at: llvm/tools/clang/lib/Sema/SemaExpr.cpp:8847
@@ -8774,3 +8846,3 @@
   S.Context.getExtVectorType(RHSEleType, LHSVecTy->getNumElements());
 RHS = S.ImpCastExprToType(RHS.get(), VecTy, CK_VectorSplat);
   }

When the LHS is a scalar, we check the type of the LHS and the element type of 
the RHS, and if necessary, cast the LHS to the element type of the RHS. What's 
the reason we don't do the same here?


Repository:
  rL LLVM

https://reviews.llvm.org/D24467



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

Re: [PATCH] D24467: Fix an error after D21678

2016-09-13 Thread Vladimir Yakovlev via cfe-commits 
vbyakovlcl removed rL LLVM as the repository for this revision.
vbyakovlcl changed the visibility of this Differential Revision from "All 
Users" to "Public (No Login Required)".
vbyakovlcl updated this revision to Diff 71135.

https://reviews.llvm.org/D24467

Files:
  llvm/tools/clang/lib/Sema/SemaExpr.cpp
  llvm/tools/clang/test/Sema/vecshift.c

Index: llvm/tools/clang/lib/Sema/SemaExpr.cpp
===
--- llvm/tools/clang/lib/Sema/SemaExpr.cpp
+++ llvm/tools/clang/lib/Sema/SemaExpr.cpp
@@ -8721,7 +8721,8 @@
 static QualType checkVectorShift(Sema &S, ExprResult &LHS, ExprResult &RHS,
  SourceLocation Loc, bool IsCompAssign) {
   // OpenCL v1.1 s6.3.j says RHS can be a vector only if LHS is a vector.
-  if (!LHS.get()->getType()->isVectorType()) {
+  if ((S.LangOpts.OpenCL || S.LangOpts.ZVector) &&
+  !LHS.get()->getType()->isVectorType()) {
 S.Diag(Loc, diag::err_shift_rhs_only_vector)
   << RHS.get()->getType() << LHS.get()->getType()
   << LHS.get()->getSourceRange() << RHS.get()->getSourceRange();
@@ -8737,8 +8738,10 @@
   if (RHS.isInvalid()) return QualType();
 
   QualType LHSType = LHS.get()->getType();
-  const VectorType *LHSVecTy = LHSType->castAs();
-  QualType LHSEleType = LHSVecTy->getElementType();
+  // Note that LHS might be a scalar because the routine calls not only in
+  // OpenCL case.
+  const VectorType *LHSVecTy = LHSType->getAs();
+  QualType LHSEleType = LHSVecTy ? LHSVecTy->getElementType() : LHSType;
 
   // Note that RHS might not be a vector.
   QualType RHSType = RHS.get()->getType();
@@ -8758,7 +8761,19 @@
 return QualType();
   }
 
-  if (RHSVecTy) {
+  if (!LHSVecTy) {
+assert(RHSVecTy);
+if (IsCompAssign)
+  return RHSType;
+if (LHSEleType != RHSEleType) {
+  LHS = S.ImpCastExprToType(LHS.get(),RHSEleType, CK_IntegralCast);
+  LHSEleType = RHSEleType;
+}
+QualType VecTy =
+S.Context.getExtVectorType(LHSEleType, RHSVecTy->getNumElements());
+LHS = S.ImpCastExprToType(LHS.get(), VecTy, CK_VectorSplat);
+LHSType = VecTy;
+  } else if (RHSVecTy) {
 // OpenCL v1.1 s6.3.j says that for vector types, the operators
 // are applied component-wise. So if RHS is a vector, then ensure
 // that the number of elements is the same as LHS...
Index: llvm/tools/clang/test/Sema/vecshift.c
===
--- llvm/tools/clang/test/Sema/vecshift.c
+++ llvm/tools/clang/test/Sema/vecshift.c
@@ -0,0 +1,80 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+typedef __attribute__((__ext_vector_type__(8))) char vector_char8;
+typedef __attribute__((__ext_vector_type__(8))) short vector_short8;
+typedef __attribute__((__ext_vector_type__(8))) int vector_int8;
+typedef __attribute__((__ext_vector_type__(8))) unsigned char vector_uchar8;
+typedef __attribute__((__ext_vector_type__(8))) unsigned short vector_ushort8;
+typedef __attribute__((__ext_vector_type__(8))) unsigned int vector_uint8;
+typedef __attribute__((__ext_vector_type__(4))) char vector_char4;
+typedef __attribute__((__ext_vector_type__(4))) short vector_short4;
+typedef __attribute__((__ext_vector_type__(4))) int vector_int4;
+typedef __attribute__((__ext_vector_type__(4))) unsigned char vector_uchar4;
+typedef __attribute__((__ext_vector_type__(4))) unsigned short vector_ushort4;
+typedef __attribute__((__ext_vector_type__(4))) unsigned int vector_uint4;
+
+char c;
+short s;
+int i;
+unsigned char uc;
+unsigned short us;
+unsigned int ui;
+vector_char8 vc8;
+vector_short8 vs8;
+vector_int8 vi8;
+vector_uchar8 vuc8;
+vector_ushort8 vus8;
+vector_uint8 vui8;
+vector_char4 vc4;
+vector_short4 vs4;
+vector_int4 vi4;
+vector_uchar4 vuc4;
+vector_ushort4 vus4;
+vector_uint4 vui4;
+
+void foo() {
+  vc8 = 1 << vc8;
+  vuc8 = 1 << vuc8;
+  vi8 = 1 << vi8;
+  vui8 = 1 << vui8;
+  vs8 = 1 << vs8;
+  vus8 = 1 << vus8;
+
+  vc8 = c << vc8;
+  vuc8 = i << vuc8;
+  vi8 = uc << vi8;
+  vui8 = us << vui8;
+  vs8 = ui << vs8;
+  vus8 = 1 << vus8;
+
+  vc8 = vc8 << vc8;
+  vi8 = vi8 << vuc8;
+  vuc8 = vuc8 << vi8;
+  vus8 = vus8 << vui8;
+  vui8 = vui8 << vs8;
+
+  vc4 = vc4 << vc8; // expected-error {{vector operands do not have the same number of elements}}
+  vi4 = vi4 << vuc8; // expected-error {{vector operands do not have the same number of elements}}
+  vuc4 = vuc4 << vi8; // expected-error {{vector operands do not have the same number of elements}}
+  vus4 = vus4 << vui8; // expected-error {{vector operands do not have the same number of elements}}
+  vui4 = vui4 << vs8; // expected-error {{vector operands do not have the same number of elements}}
+
+
+  vc8 = vc8 << vc4; // expected-error {{vector operands do not have the same number of elements}}
+  vi8 = vi8 << vuc4; // expected-error {{vector operands do not have the same number of elements}}
+  vuc8 = vuc8 << vi4; // expected-error {{vector operands do not have the same numb

Re: [PATCH] D24467: Fix an error after D21678

2016-09-14 Thread Vladimir Yakovlev via cfe-commits 
vbyakovlcl added inline comments.


Comment at: llvm/tools/clang/lib/Sema/SemaExpr.cpp:8733
@@ -8731,2 +8732,3 @@
   if (!IsCompAssign) {
-LHS = S.UsualUnaryConversions(LHS.get());
+if (S.LangOpts.OpenCL || S.LangOpts.ZVector)
+  LHS = S.UsualUnaryConversions(LHS.get());

ahatanak wrote:
> It wasn't clear to me why we have to call 
> DefaultFunctionArrayLvalueConversion instead of UsualUnaryConversions. Is it 
> possible to come up with a test case that would fail without this change, and 
> if it is, can you add it to vecshift.c?
I cannot remember why I did this change. Removing it doesn't cause any new 
fails. 


Comment at: llvm/tools/clang/lib/Sema/SemaExpr.cpp:8747
@@ -8742,3 +8746,3 @@
 
   // Note that RHS might not be a vector.
   QualType RHSType = RHS.get()->getType();

ahatanak wrote:
> We no longer error out when LHS is not a vector, so it should mention that 
> either the LHS or the RHS might not be a vector.
I added a comment.


Comment at: llvm/tools/clang/lib/Sema/SemaExpr.cpp:8787
@@ -8770,1 +8786,3 @@
 }
+if (!S.LangOpts.OpenCL && !S.LangOpts.ZVector) {
+  const BuiltinType *LHSBT = LHSEleType->getAs();

ahatanak wrote:
> Is it possible to split this out to another patch? The first patch would fix 
> handling of (scalar << vector) and the second patch would make clang reject 
> vector shifts if element sizes of the LHS and RHS are different. It's not 
> clear whether it's correct to reject the latter case, so perhaps you can 
> discuss it in a separate patch? 
Ok. I removed this and will fail a new review after committing of this review.


Comment at: llvm/tools/clang/lib/Sema/SemaExpr.cpp:8847
@@ -8774,3 +8846,3 @@
   S.Context.getExtVectorType(RHSEleType, LHSVecTy->getNumElements());
 RHS = S.ImpCastExprToType(RHS.get(), VecTy, CK_VectorSplat);
   }

ahatanak wrote:
> When the LHS is a scalar, we check the type of the LHS and the element type 
> of the RHS, and if necessary, cast the LHS to the element type of the RHS. 
> What's the reason we don't do the same here?
No need, because CodeGen inserts a conversion.

define <8 x i16> @foo1(<8 x i16> %n, i32 %m) #0 {
entry:
  %n.addr = alloca <8 x i16>, align 16
  %m.addr = alloca i32, align 4
  store <8 x i16> %n, <8 x i16>* %n.addr, align 16
  store i32 %m, i32* %m.addr, align 4
  %0 = load <8 x i16>, <8 x i16>* %n.addr, align 16
  %1 = load i32, i32* %m.addr, align 4
  %splat.splatinsert = insertelement <8 x i32> undef, i32 %1, i32 0
  %splat.splat = shufflevector <8 x i32> %splat.splatinsert, <8 x i32> undef, 
<8 x i32> zeroinitializer
  %sh_prom = trunc <8 x i32> %splat.splat to <8 x i16>
  %shl = shl <8 x i16> %0, %sh_prom
  ret <8 x i16> %shl

We need insert a conversion of scalar LHS to the RHS element type because we 
must return a vector of the RHS type.


https://reviews.llvm.org/D24467



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

Re: [PATCH] D24467: Fix an error after D21678

2016-09-14 Thread Akira Hatanaka via cfe-commits 
ahatanak added inline comments.


Comment at: llvm/tools/clang/lib/Sema/SemaExpr.cpp:8751
@@ -8747,3 +8750,3 @@
 
   // OpenCL v1.1 s6.3.j says that the operands need to be integers.
   if (!LHSEleType->isIntegerType()) {

Should we mention that any vectors used as shift operands have to have integer 
element types? This comment gives the impression that only OpenCL vectors need 
to be integers.


Comment at: llvm/tools/clang/lib/Sema/SemaExpr.cpp:8790
@@ -8774,3 +8789,3 @@
   S.Context.getExtVectorType(RHSEleType, LHSVecTy->getNumElements());
 RHS = S.ImpCastExprToType(RHS.get(), VecTy, CK_VectorSplat);
   }

OK. So the rule is that the type of the scalar operand is normally converted to 
the vector element type of the other operand before being transformed to a 
vector, but we don't have to do this when it's used as the RHS (shift amount) 
because IRGen generates the correct IR without the conversion?


Comment at: llvm/tools/clang/test/Sema/vecshift.c:56
@@ +55,3 @@
+
+  vc4 = vc4 << vc8; // expected-error {{vector operands do not have the same 
number of elements}}
+  vi4 = vi4 << vuc8; // expected-error {{vector operands do not have the same 
number of elements}}

I don't think this patch was necessary to have clang print diagnostic "vector 
operands do not have the same number of elements"? If that's the case, I think 
it's better to add these lines in a separate commit.


https://reviews.llvm.org/D24467



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

Re: [PATCH] D24467: Fix an error after D21678

2016-09-14 Thread Vladimir Yakovlev via cfe-commits 
vbyakovlcl added inline comments.


Comment at: llvm/tools/clang/lib/Sema/SemaExpr.cpp:8790
@@ -8774,3 +8789,3 @@
   S.Context.getExtVectorType(RHSEleType, LHSVecTy->getNumElements());
 RHS = S.ImpCastExprToType(RHS.get(), VecTy, CK_VectorSplat);
   }

ahatanak wrote:
> OK. So the rule is that the type of the scalar operand is normally converted 
> to the vector element type of the other operand before being transformed to a 
> vector, but we don't have to do this when it's used as the RHS (shift amount) 
> because IRGen generates the correct IR without the conversion?
Yes. I think a CodeGen test should be added for checking of IR correctness. 


Comment at: llvm/tools/clang/test/Sema/vecshift.c:56
@@ +55,3 @@
+
+  vc4 = vc4 << vc8; // expected-error {{vector operands do not have the same 
number of elements}}
+  vi4 = vi4 << vuc8; // expected-error {{vector operands do not have the same 
number of elements}}

ahatanak wrote:
> I don't think this patch was necessary to have clang print diagnostic "vector 
> operands do not have the same number of elements"? If that's the case, I 
> think it's better to add these lines in a separate commit.
Do you propose to delete lines with this diagnostic?


https://reviews.llvm.org/D24467



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

Re: [PATCH] D24467: Fix an error after D21678

2016-09-14 Thread Akira Hatanaka via cfe-commits 
ahatanak added inline comments.


Comment at: llvm/tools/clang/test/Sema/vecshift.c:56
@@ +55,3 @@
+
+  vc4 = vc4 << vc8; // expected-error {{vector operands do not have the same 
number of elements}}
+  vi4 = vi4 << vuc8; // expected-error {{vector operands do not have the same 
number of elements}}

vbyakovlcl wrote:
> ahatanak wrote:
> > I don't think this patch was necessary to have clang print diagnostic 
> > "vector operands do not have the same number of elements"? If that's the 
> > case, I think it's better to add these lines in a separate commit.
> Do you propose to delete lines with this diagnostic?
Yes, I think you can move them to a separate patch. Doing so will help people 
who read the patch understand what problems you are trying to fix with this 
patch.


https://reviews.llvm.org/D24467



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

Re: [PATCH] D24467: Fix an error after D21678

2016-09-15 Thread Vladimir Yakovlev via cfe-commits 
vbyakovlcl updated this revision to Diff 71498.

https://reviews.llvm.org/D24467

Files:
  llvm/tools/clang/lib/Sema/SemaExpr.cpp
  llvm/tools/clang/test/CodeGen/vecshift.c
  llvm/tools/clang/test/Sema/vecshift.c

Index: llvm/tools/clang/lib/Sema/SemaExpr.cpp
===
--- llvm/tools/clang/lib/Sema/SemaExpr.cpp
+++ llvm/tools/clang/lib/Sema/SemaExpr.cpp
@@ -8721,7 +8721,8 @@
 static QualType checkVectorShift(Sema &S, ExprResult &LHS, ExprResult &RHS,
  SourceLocation Loc, bool IsCompAssign) {
   // OpenCL v1.1 s6.3.j says RHS can be a vector only if LHS is a vector.
-  if (!LHS.get()->getType()->isVectorType()) {
+  if ((S.LangOpts.OpenCL || S.LangOpts.ZVector) &&
+  !LHS.get()->getType()->isVectorType()) {
 S.Diag(Loc, diag::err_shift_rhs_only_vector)
   << RHS.get()->getType() << LHS.get()->getType()
   << LHS.get()->getSourceRange() << RHS.get()->getSourceRange();
@@ -8737,15 +8738,17 @@
   if (RHS.isInvalid()) return QualType();
 
   QualType LHSType = LHS.get()->getType();
-  const VectorType *LHSVecTy = LHSType->castAs();
-  QualType LHSEleType = LHSVecTy->getElementType();
+  // Note that LHS might be a scalar because the routine calls not only in
+  // OpenCL case.
+  const VectorType *LHSVecTy = LHSType->getAs();
+  QualType LHSEleType = LHSVecTy ? LHSVecTy->getElementType() : LHSType;
 
   // Note that RHS might not be a vector.
   QualType RHSType = RHS.get()->getType();
   const VectorType *RHSVecTy = RHSType->getAs();
   QualType RHSEleType = RHSVecTy ? RHSVecTy->getElementType() : RHSType;
 
-  // OpenCL v1.1 s6.3.j says that the operands need to be integers.
+  // The operands need to be integers.
   if (!LHSEleType->isIntegerType()) {
 S.Diag(Loc, diag::err_typecheck_expect_int)
   << LHS.get()->getType() << LHS.get()->getSourceRange();
@@ -8758,7 +8761,19 @@
 return QualType();
   }
 
-  if (RHSVecTy) {
+  if (!LHSVecTy) {
+assert(RHSVecTy);
+if (IsCompAssign)
+  return RHSType;
+if (LHSEleType != RHSEleType) {
+  LHS = S.ImpCastExprToType(LHS.get(),RHSEleType, CK_IntegralCast);
+  LHSEleType = RHSEleType;
+}
+QualType VecTy =
+S.Context.getExtVectorType(LHSEleType, RHSVecTy->getNumElements());
+LHS = S.ImpCastExprToType(LHS.get(), VecTy, CK_VectorSplat);
+LHSType = VecTy;
+  } else if (RHSVecTy) {
 // OpenCL v1.1 s6.3.j says that for vector types, the operators
 // are applied component-wise. So if RHS is a vector, then ensure
 // that the number of elements is the same as LHS...
Index: llvm/tools/clang/test/CodeGen/vecshift.c
===
--- llvm/tools/clang/test/CodeGen/vecshift.c
+++ llvm/tools/clang/test/CodeGen/vecshift.c
@@ -0,0 +1,146 @@
+// RUN: %clang_cc1 -emit-llvm %s -o - | FileCheck %s
+
+typedef __attribute__((__ext_vector_type__(8))) char vector_char8;
+typedef __attribute__((__ext_vector_type__(8))) short vector_short8;
+typedef __attribute__((__ext_vector_type__(8))) int vector_int8;
+typedef __attribute__((__ext_vector_type__(8))) unsigned char vector_uchar8;
+typedef __attribute__((__ext_vector_type__(8))) unsigned short vector_ushort8;
+typedef __attribute__((__ext_vector_type__(8))) unsigned int vector_uint8;
+typedef __attribute__((__ext_vector_type__(4))) char vector_char4;
+typedef __attribute__((__ext_vector_type__(4))) short vector_short4;
+typedef __attribute__((__ext_vector_type__(4))) int vector_int4;
+typedef __attribute__((__ext_vector_type__(4))) unsigned char vector_uchar4;
+typedef __attribute__((__ext_vector_type__(4))) unsigned short vector_ushort4;
+typedef __attribute__((__ext_vector_type__(4))) unsigned int vector_uint4;
+
+char c;
+short s;
+int i;
+unsigned char uc;
+unsigned short us;
+unsigned int ui;
+vector_char8 vc8;
+vector_short8 vs8;
+vector_int8 vi8;
+vector_uchar8 vuc8;
+vector_ushort8 vus8;
+vector_uint8 vui8;
+vector_char4 vc4;
+vector_short4 vs4;
+vector_int4 vi4;
+vector_uchar4 vuc4;
+vector_ushort4 vus4;
+vector_uint4 vui4;
+
+void foo() {
+  vc8 = 1 << vc8;
+// CHECK: [[t0:%.+]] = load <8 x i8>, <8 x i8>* {{@.+}}, align 8
+// CHECK: shl <8 x i8> , [[t0]]
+  vuc8 = 1 << vuc8;
+// CHECK: [[t1:%.+]] = load <8 x i8>, <8 x i8>* @vuc8, align 8
+// CHECK: shl <8 x i8> , [[t1]]
+  vi8 = 1 << vi8;
+// CHECK: [[t2:%.+]] = load <8 x i32>, <8 x i32>* @vi8, align 32
+// CHECK: shl <8 x i32> , [[t2]]
+  vui8 = 1 << vui8;
+// CHECK: [[t3:%.+]] = load <8 x i32>, <8 x i32>* @vui8, align 32
+// CHECK: shl <8 x i32> , [[t3]]
+  vs8 = 1 << vs8;
+// CHECK: [[t4:%.+]] = load <8 x i16>, <8 x i16>* @vs8, align 16
+// CHECK: shl <8 x i16> , [[t4]]
+  vus8 = 1 << vus8;
+// CHECK: [[t5:%.+]] = load <8 x i16>, <8 x i16>* @vus8, align 16
+// CHECK: shl <8 x i16> , [[t5]]
+
+  vc8 = c << vc8;
+// CHECK: [[t6:%.+]] = load i8, i8* @c, align 1
+// CHECK: [[splat_splatinsert:%.+]] = insertelement <8 x i8> undef, i8 [[t6]], i32 0
+// CHEC

Re: [PATCH] D24467: Fix an error after D21678

2016-09-15 Thread Akira Hatanaka via cfe-commits 
ahatanak added a comment.

LGTM with a nit in test case.



Comment at: llvm/tools/clang/test/CodeGen/vecshift.c:43
@@ +42,3 @@
+  vi8 = 1 << vi8;
+// CHECK: [[t2:%.+]] = load <8 x i32>, <8 x i32>* @vi8, align 32
+// CHECK: shl <8 x i32> , [[t2]]

This test fails on my machine because MaxVectorAlign is 128 (16B) for darwin. 
Maybe you can remove the alignment checks or add a triple to the RUN line?


https://reviews.llvm.org/D24467



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

Re: [PATCH] D24467: Fix an error after D21678

2016-09-15 Thread Vladimir Yakovlev via cfe-commits 
vbyakovlcl added inline comments.


Comment at: llvm/tools/clang/test/CodeGen/vecshift.c:43
@@ +42,3 @@
+  vi8 = 1 << vi8;
+// CHECK: [[t2:%.+]] = load <8 x i32>, <8 x i32>* @vi8, align 32
+// CHECK: shl <8 x i32> , [[t2]]

ahatanak wrote:
> This test fails on my machine because MaxVectorAlign is 128 (16B) for darwin. 
> Maybe you can remove the alignment checks or add a triple to the RUN line?
Done


https://reviews.llvm.org/D24467



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

Re: [PATCH] D24467: Fix an error after D21678

2016-09-15 Thread Vladimir Yakovlev via cfe-commits 
vbyakovlcl updated this revision to Diff 71545.

https://reviews.llvm.org/D24467

Files:
  llvm/tools/clang/lib/Sema/SemaExpr.cpp
  llvm/tools/clang/test/CodeGen/vecshift.c
  llvm/tools/clang/test/Sema/vecshift.c

Index: llvm/tools/clang/lib/Sema/SemaExpr.cpp
===
--- llvm/tools/clang/lib/Sema/SemaExpr.cpp
+++ llvm/tools/clang/lib/Sema/SemaExpr.cpp
@@ -8721,7 +8721,8 @@
 static QualType checkVectorShift(Sema &S, ExprResult &LHS, ExprResult &RHS,
  SourceLocation Loc, bool IsCompAssign) {
   // OpenCL v1.1 s6.3.j says RHS can be a vector only if LHS is a vector.
-  if (!LHS.get()->getType()->isVectorType()) {
+  if ((S.LangOpts.OpenCL || S.LangOpts.ZVector) &&
+  !LHS.get()->getType()->isVectorType()) {
 S.Diag(Loc, diag::err_shift_rhs_only_vector)
   << RHS.get()->getType() << LHS.get()->getType()
   << LHS.get()->getSourceRange() << RHS.get()->getSourceRange();
@@ -8737,15 +8738,17 @@
   if (RHS.isInvalid()) return QualType();
 
   QualType LHSType = LHS.get()->getType();
-  const VectorType *LHSVecTy = LHSType->castAs();
-  QualType LHSEleType = LHSVecTy->getElementType();
+  // Note that LHS might be a scalar because the routine calls not only in
+  // OpenCL case.
+  const VectorType *LHSVecTy = LHSType->getAs();
+  QualType LHSEleType = LHSVecTy ? LHSVecTy->getElementType() : LHSType;
 
   // Note that RHS might not be a vector.
   QualType RHSType = RHS.get()->getType();
   const VectorType *RHSVecTy = RHSType->getAs();
   QualType RHSEleType = RHSVecTy ? RHSVecTy->getElementType() : RHSType;
 
-  // OpenCL v1.1 s6.3.j says that the operands need to be integers.
+  // The operands need to be integers.
   if (!LHSEleType->isIntegerType()) {
 S.Diag(Loc, diag::err_typecheck_expect_int)
   << LHS.get()->getType() << LHS.get()->getSourceRange();
@@ -8758,7 +8761,19 @@
 return QualType();
   }
 
-  if (RHSVecTy) {
+  if (!LHSVecTy) {
+assert(RHSVecTy);
+if (IsCompAssign)
+  return RHSType;
+if (LHSEleType != RHSEleType) {
+  LHS = S.ImpCastExprToType(LHS.get(),RHSEleType, CK_IntegralCast);
+  LHSEleType = RHSEleType;
+}
+QualType VecTy =
+S.Context.getExtVectorType(LHSEleType, RHSVecTy->getNumElements());
+LHS = S.ImpCastExprToType(LHS.get(), VecTy, CK_VectorSplat);
+LHSType = VecTy;
+  } else if (RHSVecTy) {
 // OpenCL v1.1 s6.3.j says that for vector types, the operators
 // are applied component-wise. So if RHS is a vector, then ensure
 // that the number of elements is the same as LHS...
Index: llvm/tools/clang/test/CodeGen/vecshift.c
===
--- llvm/tools/clang/test/CodeGen/vecshift.c
+++ llvm/tools/clang/test/CodeGen/vecshift.c
@@ -0,0 +1,146 @@
+// RUN: %clang_cc1 -emit-llvm %s -o - | FileCheck %s
+
+typedef __attribute__((__ext_vector_type__(8))) char vector_char8;
+typedef __attribute__((__ext_vector_type__(8))) short vector_short8;
+typedef __attribute__((__ext_vector_type__(8))) int vector_int8;
+typedef __attribute__((__ext_vector_type__(8))) unsigned char vector_uchar8;
+typedef __attribute__((__ext_vector_type__(8))) unsigned short vector_ushort8;
+typedef __attribute__((__ext_vector_type__(8))) unsigned int vector_uint8;
+typedef __attribute__((__ext_vector_type__(4))) char vector_char4;
+typedef __attribute__((__ext_vector_type__(4))) short vector_short4;
+typedef __attribute__((__ext_vector_type__(4))) int vector_int4;
+typedef __attribute__((__ext_vector_type__(4))) unsigned char vector_uchar4;
+typedef __attribute__((__ext_vector_type__(4))) unsigned short vector_ushort4;
+typedef __attribute__((__ext_vector_type__(4))) unsigned int vector_uint4;
+
+char c;
+short s;
+int i;
+unsigned char uc;
+unsigned short us;
+unsigned int ui;
+vector_char8 vc8;
+vector_short8 vs8;
+vector_int8 vi8;
+vector_uchar8 vuc8;
+vector_ushort8 vus8;
+vector_uint8 vui8;
+vector_char4 vc4;
+vector_short4 vs4;
+vector_int4 vi4;
+vector_uchar4 vuc4;
+vector_ushort4 vus4;
+vector_uint4 vui4;
+
+void foo() {
+  vc8 = 1 << vc8;
+// CHECK: [[t0:%.+]] = load <8 x i8>, <8 x i8>* {{@.+}},
+// CHECK: shl <8 x i8> , [[t0]]
+  vuc8 = 1 << vuc8;
+// CHECK: [[t1:%.+]] = load <8 x i8>, <8 x i8>* {{@.+}},
+// CHECK: shl <8 x i8> , [[t1]]
+  vi8 = 1 << vi8;
+// CHECK: [[t2:%.+]] = load <8 x i32>, <8 x i32>* {{@.+}},
+// CHECK: shl <8 x i32> , [[t2]]
+  vui8 = 1 << vui8;
+// CHECK: [[t3:%.+]] = load <8 x i32>, <8 x i32>* {{@.+}},
+// CHECK: shl <8 x i32> , [[t3]]
+  vs8 = 1 << vs8;
+// CHECK: [[t4:%.+]] = load <8 x i16>, <8 x i16>* {{@.+}},
+// CHECK: shl <8 x i16> , [[t4]]
+  vus8 = 1 << vus8;
+// CHECK: [[t5:%.+]] = load <8 x i16>, <8 x i16>* {{@.+}},
+// CHECK: shl <8 x i16> , [[t5]]
+
+  vc8 = c << vc8;
+// CHECK: [[t6:%.+]] = load i8, i8* @c,
+// CHECK: [[splat_splatinsert:%.+]] = insertelement <8 x i8> undef, i8 [[t6]], i32 0
+// CHECK: [[splat_splat:%.+]] = shufflevector <8 x i8>

Re: [PATCH] D24467: Fix an error after D21678

2016-09-15 Thread Akira Hatanaka via cfe-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rL281669: [Sema] Allow shifting a scalar operand by a vector 
operand. (authored by ahatanak).

Changed prior to commit:
  https://reviews.llvm.org/D24467?vs=71545&id=71569#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D24467

Files:
  cfe/trunk/lib/Sema/SemaExpr.cpp
  cfe/trunk/test/CodeGen/vecshift.c
  cfe/trunk/test/Sema/vecshift.c

Index: cfe/trunk/test/CodeGen/vecshift.c
===
--- cfe/trunk/test/CodeGen/vecshift.c
+++ cfe/trunk/test/CodeGen/vecshift.c
@@ -0,0 +1,146 @@
+// RUN: %clang_cc1 -emit-llvm %s -o - | FileCheck %s
+
+typedef __attribute__((__ext_vector_type__(8))) char vector_char8;
+typedef __attribute__((__ext_vector_type__(8))) short vector_short8;
+typedef __attribute__((__ext_vector_type__(8))) int vector_int8;
+typedef __attribute__((__ext_vector_type__(8))) unsigned char vector_uchar8;
+typedef __attribute__((__ext_vector_type__(8))) unsigned short vector_ushort8;
+typedef __attribute__((__ext_vector_type__(8))) unsigned int vector_uint8;
+typedef __attribute__((__ext_vector_type__(4))) char vector_char4;
+typedef __attribute__((__ext_vector_type__(4))) short vector_short4;
+typedef __attribute__((__ext_vector_type__(4))) int vector_int4;
+typedef __attribute__((__ext_vector_type__(4))) unsigned char vector_uchar4;
+typedef __attribute__((__ext_vector_type__(4))) unsigned short vector_ushort4;
+typedef __attribute__((__ext_vector_type__(4))) unsigned int vector_uint4;
+
+char c;
+short s;
+int i;
+unsigned char uc;
+unsigned short us;
+unsigned int ui;
+vector_char8 vc8;
+vector_short8 vs8;
+vector_int8 vi8;
+vector_uchar8 vuc8;
+vector_ushort8 vus8;
+vector_uint8 vui8;
+vector_char4 vc4;
+vector_short4 vs4;
+vector_int4 vi4;
+vector_uchar4 vuc4;
+vector_ushort4 vus4;
+vector_uint4 vui4;
+
+void foo() {
+  vc8 = 1 << vc8;
+// CHECK: [[t0:%.+]] = load <8 x i8>, <8 x i8>* {{@.+}},
+// CHECK: shl <8 x i8> , [[t0]]
+  vuc8 = 1 << vuc8;
+// CHECK: [[t1:%.+]] = load <8 x i8>, <8 x i8>* {{@.+}},
+// CHECK: shl <8 x i8> , [[t1]]
+  vi8 = 1 << vi8;
+// CHECK: [[t2:%.+]] = load <8 x i32>, <8 x i32>* {{@.+}},
+// CHECK: shl <8 x i32> , [[t2]]
+  vui8 = 1 << vui8;
+// CHECK: [[t3:%.+]] = load <8 x i32>, <8 x i32>* {{@.+}},
+// CHECK: shl <8 x i32> , [[t3]]
+  vs8 = 1 << vs8;
+// CHECK: [[t4:%.+]] = load <8 x i16>, <8 x i16>* {{@.+}},
+// CHECK: shl <8 x i16> , [[t4]]
+  vus8 = 1 << vus8;
+// CHECK: [[t5:%.+]] = load <8 x i16>, <8 x i16>* {{@.+}},
+// CHECK: shl <8 x i16> , [[t5]]
+
+  vc8 = c << vc8;
+// CHECK: [[t6:%.+]] = load i8, i8* @c,
+// CHECK: [[splat_splatinsert:%.+]] = insertelement <8 x i8> undef, i8 [[t6]], i32 0
+// CHECK: [[splat_splat:%.+]] = shufflevector <8 x i8> [[splat_splatinsert]], <8 x i8> undef, <8 x i32> zeroinitializer
+// CHECK: [[t7:%.+]] = load <8 x i8>, <8 x i8>* {{@.+}},
+// CHECK: shl <8 x i8> [[splat_splat]], [[t7]]
+  vuc8 = i << vuc8;
+// CHECK: [[t8:%.+]] = load i32, i32* @i,
+// CHECK: [[tconv:%.+]] = trunc i32 [[t8]] to i8
+// CHECK: [[splat_splatinsert7:%.+]] = insertelement <8 x i8> undef, i8 [[tconv]], i32 0
+// CHECK: [[splat_splat8:%.+]] = shufflevector <8 x i8> [[splat_splatinsert7]], <8 x i8> undef, <8 x i32> zeroinitializer
+// CHECK: [[t9:%.+]] = load <8 x i8>, <8 x i8>* {{@.+}},
+// CHECK: shl <8 x i8> [[splat_splat8]], [[t9]]
+  vi8 = uc << vi8;
+// CHECK: [[t10:%.+]] = load i8, i8* @uc,
+// CHECK: [[conv10:%.+]] = zext i8 [[t10]] to i32
+// CHECK: [[splat_splatinsert11:%.+]] = insertelement <8 x i32> undef, i32 [[conv10]], i32 0
+// CHECK: [[splat_splat12:%.+]] = shufflevector <8 x i32> [[splat_splatinsert11]], <8 x i32> undef, <8 x i32> zeroinitializer
+// CHECK: [[t11:%.+]] = load <8 x i32>, <8 x i32>* {{@.+}},
+// CHECK: shl <8 x i32> [[splat_splat12]], [[t11]]
+  vui8 = us << vui8;
+// CHECK: [[t12:%.+]] = load i16, i16* @us,
+// CHECK: [[conv14:%.+]] = zext i16 [[t12]] to i32
+// CHECK: [[splat_splatinsert15:%.+]] = insertelement <8 x i32> undef, i32 [[conv14]], i32 0
+// CHECK: [[splat_splat16:%.+]] = shufflevector <8 x i32> [[splat_splatinsert15]], <8 x i32> undef, <8 x i32> zeroinitializer
+// CHECK: [[t13:%.+]] = load <8 x i32>, <8 x i32>* {{@.+}},
+// CHECK: shl <8 x i32> [[splat_splat16]], [[t13]]
+  vs8 = ui << vs8;
+// CHECK: [[t14:%.+]] = load i32, i32* @ui,
+// CHECK: [[conv18:%.+]] = trunc i32 [[t14]] to i16
+// CHECK: [[splat_splatinsert19:%.+]] = insertelement <8 x i16> undef, i16 [[conv18]], i32 0
+// CHECK: [[splat_splat20:%.+]] = shufflevector <8 x i16> [[splat_splatinsert19]], <8 x i16> undef, <8 x i32> zeroinitializer
+// CHECK: [[t15:%.+]] = load <8 x i16>, <8 x i16>* {{@.+}},
+// CHECK: shl <8 x i16> [[splat_splat20]], [[t15]]
+  vus8 = 1 << vus8;
+// CHECK: [[t16:%.+]] = load <8 x i16>, <8 x i16>* {{@.+}},
+// CHECK: [[shl22:%.+]] = shl <8 x i16> , [[t16]]
+
+ vc8 = vc8 << vc8;
+// CHECK: [[t17:%.+]] = load <8 x i8>, <8 x i8>* {{@.+}},
+// CHECK: [[t18:%.+]] = load <8