[PATCH] D24669: {Sema] Gcc compatibility of vector shift.

[Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL284579: [Sema] Gcc compatibility of vector shift (authored 
by asbokhan).

Changed prior to commit:
  https://reviews.llvm.org/D24669?vs=74269=75130#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D24669

Files:
  cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
  cfe/trunk/lib/Sema/SemaExpr.cpp
  cfe/trunk/test/CodeGen/vecshift.c
  cfe/trunk/test/Sema/vecshift.c

Index: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
===
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
@@ -2305,6 +2305,9 @@
   "cannot convert between vector and non-scalar values (%0 and %1)">;
 def err_typecheck_vector_lengths_not_equal : Error<
   "vector operands do not have the same number of elements (%0 and %1)">;
+def warn_typecheck_vector_element_sizes_not_equal : Warning<
+  "vector operands do not have the same elements sizes (%0 and %1)">,
+  InGroup>, DefaultError;
 def err_ext_vector_component_exceeds_length : Error<
   "vector component access exceeds type %0">;
 def err_ext_vector_component_name_illegal : Error<
Index: cfe/trunk/test/Sema/vecshift.c
===
--- cfe/trunk/test/Sema/vecshift.c
+++ cfe/trunk/test/Sema/vecshift.c
@@ -1,5 +1,9 @@
-// RUN: %clang_cc1 -fsyntax-only -verify %s
+// RUN: %clang_cc1 -fsyntax-only -DERR -verify %s
+// RUN: %clang_cc1 -fsyntax-only -Wno-error-vec-elem-size -verify %s
+// RUN: %clang_cc1 -fsyntax-only -DEXT -DERR -verify %s
+// RUN: %clang_cc1 -fsyntax-only -DEXT -Wno-error-vec-elem-size -verify %s
 
+#ifdef EXT
 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;
@@ -12,6 +16,20 @@
 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;
+#else
+typedef __attribute__((vector_size(8))) char vector_char8;
+typedef __attribute__((vector_size(16))) short vector_short8;
+typedef __attribute__((vector_size(32))) int vector_int8;
+typedef __attribute__((vector_size(8))) unsigned char vector_uchar8;
+typedef __attribute__((vector_size(16))) unsigned short vector_ushort8;
+typedef __attribute__((vector_size(32))) unsigned int vector_uint8;
+typedef __attribute__((vector_size(4))) char vector_char4;
+typedef __attribute__((vector_size(4))) short vector_short4;
+typedef __attribute__((vector_size(16))) int vector_int4;
+typedef __attribute__((vector_size(4))) unsigned char vector_uchar4;
+typedef __attribute__((vector_size(8))) unsigned short vector_ushort4;
+typedef __attribute__((vector_size(16))) unsigned int vector_uint4;
+#endif
 
 char c;
 short s;
@@ -48,16 +66,30 @@
   vus8 = 1 << vus8;
 
   vc8 = vc8 << vc8;
-  vi8 = vi8 << vuc8;
-  vuc8 = vuc8 << vi8;
-  vus8 = vus8 << vui8;
-  vui8 = vui8 << vs8;
+#ifdef ERR
+  vi8 = vi8 << vuc8; // expected-error {{vector operands do not have the same elements sizes}}
+  vuc8 = vuc8 << vi8; // expected-error {{vector operands do not have the same elements sizes}}
+  vus8 = vus8 << vui8; // expected-error {{vector operands do not have the same elements sizes}}
+  vui8 = vui8 << vs8; // expected-error {{vector operands do not have the same elements sizes}}
+#else
+  vi8 = vi8 << vuc8; // expected-warning {{vector operands do not have the same elements sizes}}
+  vuc8 = vuc8 << vi8; // expected-warning {{vector operands do not have the same elements sizes}}
+  vus8 = vus8 << vui8; // expected-warning {{vector operands do not have the same elements sizes}}
+  vui8 = vui8 << vs8; // expected-warning {{vector operands do not have the same elements sizes}}
+#endif
 
   vc8 <<= vc8;
-  vi8 <<= vuc8;
-  vuc8 <<= vi8;
-  vus8 <<= vui8;
-  vui8 <<= vs8;
+#ifdef ERR
+  vi8 <<= vuc8; // expected-error {{vector operands do not have the same elements sizes}}
+  vuc8 <<= vi8; // expected-error {{vector operands do not have the same elements sizes}}
+  vus8 <<= vui8; // expected-error {{vector operands do not have the same elements sizes}}
+  vui8 <<= vs8; // expected-error {{vector operands do not have the same elements sizes}}
+#else
+  vi8 <<= vuc8; // expected-warning {{vector operands do not have the same elements sizes}}
+  vuc8 <<= vi8; // expected-warning {{vector operands do not have the same elements sizes}}
+  vus8 <<= vui8; // expected-warning {{vector operands do not have the same elements sizes}}
+  vui8 <<= vs8; // expected-warning {{vector operands do not have the same elements sizes}}
+#endif
 
   c <<= vc8; // expected-error {{assigning to 'char' from incompatible type}}
   i <<= vuc8; // expected-error 

[PATCH] D24669: {Sema] Gcc compatibility of vector shift.

[Bruno Cardoso Lopes via cfe-commits]

bruno accepted this revision.
bruno added a comment.
This revision is now accepted and ready to land.

Ok, great!

LGTM


https://reviews.llvm.org/D24669



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

[PATCH] D24669: {Sema] Gcc compatibility of vector shift.

[Vladimir Yakovlev via cfe-commits]

vbyakovlcl added inline comments.



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

bruno wrote:
> vbyakovlcl wrote:
> > bruno wrote:
> > > vbyakovlcl wrote:
> > > > bruno wrote:
> > > > > Besides `__ext_vector_type__`, would this also trigger for 
> > > > > `vector_size`? Right now this is an error for `vector_size` primarily 
> > > > > because the number of elements is different, can you confirm this 
> > > > > won't change?
> > > > I compare vector element sizes, so there must not be any differencies 
> > > > caused by different triggers. I added additional type definitions to 
> > > > the tests. All compiles fain.
> > > 
> > > I don't think this is right. When I try to compile similar code for 
> > > `vector_size` without your patch, I get the errors:
> > > 
> > > /tmp/x.c:80:15: error: vector operands do not have the same number of 
> > > elements ('vector_int8n' (vector of 2 'int' values) and 'vector_uchar8n' 
> > > (vector of 8 'unsigned char' values))
> > >   vi8n = vi8n << vuc8n; // expected-warning {{vector operands do not have 
> > > the same elements sizes}}
> > >   ^  ~
> > > /tmp/x.c:81:17: error: vector operands do not have the same number of 
> > > elements ('vector_uchar8n' (vector of 8 'unsigned char' values) and 
> > > 'vector_int8n' (vector of 2 'int' values))
> > >   vuc8n = vuc8n << vi8n; // expected-warning {{vector operands do not 
> > > have the same elements sizes}}
> > >   ~ ^  
> > > /tmp/x.c:82:17: error: vector operands do not have the same number of 
> > > elements ('vector_ushort8n' (vector of 4 'unsigned short' values) and 
> > > 'vector_uint8n' (vector of 2 'unsigned int' values))
> > >   vus8n = vus8n << vui8n; // expected-warning {{vector operands do not 
> > > have the same elements sizes}}
> > >   ~ ^  ~
> > > /tmp/x.c:83:17: error: vector operands do not have the same number of 
> > > elements ('vector_uint8n' (vector of 2 'unsigned int' values) and 
> > > 'vector_short8n' (vector of 4 'short' values))
> > >   vui8n = vui8n << vs8n; // expected-warning {{vector operands do not 
> > > have the same elements sizes}}
> > >   ~ ^   
> > > 
> > > Given your test changes, it seems that now, instead of "vector operands 
> > > do not have the same number of elements" we would get "vector operands do 
> > > not have the same elements sizes". I rather we stay with the first. 
> > > Additionally, even if we had "vector operands do not have the same 
> > > elements sizes" for `vector_size`, this should never be demoted to a 
> > > warning.
> > Argument of a GNU vector size attribute specifies vector size measured in 
> > bytes(see https://gcc.gnu.org/onlinedocs/gcc/Vector-Extensions.html). So 
> > you got right diagnostics. Both compilers with and without my changes print 
> > the same diagnostics  for yours case. Here is a small testcase used both 
> > GNU and clang extensions
> > 
> > $ cat bruno.c   
> > 
> >  
> > typedef __attribute__((__ext_vector_type__(8))) int vector_int8;
> > typedef __attribute__((__ext_vector_type__(8))) unsigned char vector_uchar8;
> >  
> > typedef __attribute__((vector_size(8))) int vector_int8n;
> > typedef __attribute__((vector_size(8))) unsigned char vector_uchar8n;
> >  
> > vector_int8  vi8;
> > vector_uchar8 vuc8;
> > vector_int8n  vi8n;
> > vector_uchar8n vuc8n;
> >  
> > int foo() {
> >   vi8 = vi8 << vuc8;
> >   vi8n = vi8n << vuc8n;
> > 
> > $ clang -c bruno.c -Wno-error-vec-elem-size 
> > 
> >  
> > bruno.c:13:13: warning: vector operands do not have the same elements sizes 
> > ('vector_int8' (vector of 8 'int' values) and 'vector_uchar8' (vector of 8 
> > 'unsigned char' values)) [-Wvec-elem-size]
> >   vi8 = vi8 << vuc8;
> > ~~~ ^  
> > bruno.c:14:15: error: vector operands do not have the same number of 
> > elements ('vector_int8n' (vector of 2 'int' values) and 'vector_uchar8n' 
> > (vector of 8 'unsigned char' values))
> >   vi8n = vi8n << vuc8n;
> > 
> > The compiler without the changes prints the second error only 
> > (bruno.c:14:15).
> What actually concerns me here is the following: if you invoke `clang -c 
> bruno.c -Wvec-elem-size`, will that override the `error: vector operands do 
> not have the same number of elements ` message for `vector_size` typed 
> vectors? If so, I don't think this is right.
No, this will not override the error because these diagnostics use independent 
conditions. The option  vec-elem-size is used only for 

[PATCH] D24669: {Sema] Gcc compatibility of vector shift.

[Bruno Cardoso Lopes via cfe-commits]

bruno added inline comments.



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

vbyakovlcl wrote:
> bruno wrote:
> > vbyakovlcl wrote:
> > > bruno wrote:
> > > > Besides `__ext_vector_type__`, would this also trigger for 
> > > > `vector_size`? Right now this is an error for `vector_size` primarily 
> > > > because the number of elements is different, can you confirm this won't 
> > > > change?
> > > I compare vector element sizes, so there must not be any differencies 
> > > caused by different triggers. I added additional type definitions to the 
> > > tests. All compiles fain.
> > 
> > I don't think this is right. When I try to compile similar code for 
> > `vector_size` without your patch, I get the errors:
> > 
> > /tmp/x.c:80:15: error: vector operands do not have the same number of 
> > elements ('vector_int8n' (vector of 2 'int' values) and 'vector_uchar8n' 
> > (vector of 8 'unsigned char' values))
> >   vi8n = vi8n << vuc8n; // expected-warning {{vector operands do not have 
> > the same elements sizes}}
> >   ^  ~
> > /tmp/x.c:81:17: error: vector operands do not have the same number of 
> > elements ('vector_uchar8n' (vector of 8 'unsigned char' values) and 
> > 'vector_int8n' (vector of 2 'int' values))
> >   vuc8n = vuc8n << vi8n; // expected-warning {{vector operands do not have 
> > the same elements sizes}}
> >   ~ ^  
> > /tmp/x.c:82:17: error: vector operands do not have the same number of 
> > elements ('vector_ushort8n' (vector of 4 'unsigned short' values) and 
> > 'vector_uint8n' (vector of 2 'unsigned int' values))
> >   vus8n = vus8n << vui8n; // expected-warning {{vector operands do not have 
> > the same elements sizes}}
> >   ~ ^  ~
> > /tmp/x.c:83:17: error: vector operands do not have the same number of 
> > elements ('vector_uint8n' (vector of 2 'unsigned int' values) and 
> > 'vector_short8n' (vector of 4 'short' values))
> >   vui8n = vui8n << vs8n; // expected-warning {{vector operands do not have 
> > the same elements sizes}}
> >   ~ ^   
> > 
> > Given your test changes, it seems that now, instead of "vector operands do 
> > not have the same number of elements" we would get "vector operands do not 
> > have the same elements sizes". I rather we stay with the first. 
> > Additionally, even if we had "vector operands do not have the same elements 
> > sizes" for `vector_size`, this should never be demoted to a warning.
> Argument of a GNU vector size attribute specifies vector size measured in 
> bytes(see https://gcc.gnu.org/onlinedocs/gcc/Vector-Extensions.html). So you 
> got right diagnostics. Both compilers with and without my changes print the 
> same diagnostics  for yours case. Here is a small testcase used both GNU and 
> clang extensions
> 
> $ cat bruno.c 
>   
>  
> typedef __attribute__((__ext_vector_type__(8))) int vector_int8;
> typedef __attribute__((__ext_vector_type__(8))) unsigned char vector_uchar8;
>  
> typedef __attribute__((vector_size(8))) int vector_int8n;
> typedef __attribute__((vector_size(8))) unsigned char vector_uchar8n;
>  
> vector_int8  vi8;
> vector_uchar8 vuc8;
> vector_int8n  vi8n;
> vector_uchar8n vuc8n;
>  
> int foo() {
>   vi8 = vi8 << vuc8;
>   vi8n = vi8n << vuc8n;
> 
> $ clang -c bruno.c -Wno-error-vec-elem-size   
>   
>  
> bruno.c:13:13: warning: vector operands do not have the same elements sizes 
> ('vector_int8' (vector of 8 'int' values) and 'vector_uchar8' (vector of 8 
> 'unsigned char' values)) [-Wvec-elem-size]
>   vi8 = vi8 << vuc8;
> ~~~ ^  
> bruno.c:14:15: error: vector operands do not have the same number of elements 
> ('vector_int8n' (vector of 2 'int' values) and 'vector_uchar8n' (vector of 8 
> 'unsigned char' values))
>   vi8n = vi8n << vuc8n;
> 
> The compiler without the changes prints the second error only (bruno.c:14:15).
What actually concerns me here is the following: if you invoke `clang -c 
bruno.c -Wvec-elem-size`, will that override the `error: vector operands do not 
have the same number of elements ` message for `vector_size` typed vectors? If 
so, I don't think this is right.


https://reviews.llvm.org/D24669



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

[PATCH] D24669: {Sema] Gcc compatibility of vector shift.

[Vladimir Yakovlev via cfe-commits]

vbyakovlcl added inline comments.



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

bruno wrote:
> vbyakovlcl wrote:
> > bruno wrote:
> > > Besides `__ext_vector_type__`, would this also trigger for `vector_size`? 
> > > Right now this is an error for `vector_size` primarily because the number 
> > > of elements is different, can you confirm this won't change?
> > I compare vector element sizes, so there must not be any differencies 
> > caused by different triggers. I added additional type definitions to the 
> > tests. All compiles fain.
> 
> I don't think this is right. When I try to compile similar code for 
> `vector_size` without your patch, I get the errors:
> 
> /tmp/x.c:80:15: error: vector operands do not have the same number of 
> elements ('vector_int8n' (vector of 2 'int' values) and 'vector_uchar8n' 
> (vector of 8 'unsigned char' values))
>   vi8n = vi8n << vuc8n; // expected-warning {{vector operands do not have the 
> same elements sizes}}
>   ^  ~
> /tmp/x.c:81:17: error: vector operands do not have the same number of 
> elements ('vector_uchar8n' (vector of 8 'unsigned char' values) and 
> 'vector_int8n' (vector of 2 'int' values))
>   vuc8n = vuc8n << vi8n; // expected-warning {{vector operands do not have 
> the same elements sizes}}
>   ~ ^  
> /tmp/x.c:82:17: error: vector operands do not have the same number of 
> elements ('vector_ushort8n' (vector of 4 'unsigned short' values) and 
> 'vector_uint8n' (vector of 2 'unsigned int' values))
>   vus8n = vus8n << vui8n; // expected-warning {{vector operands do not have 
> the same elements sizes}}
>   ~ ^  ~
> /tmp/x.c:83:17: error: vector operands do not have the same number of 
> elements ('vector_uint8n' (vector of 2 'unsigned int' values) and 
> 'vector_short8n' (vector of 4 'short' values))
>   vui8n = vui8n << vs8n; // expected-warning {{vector operands do not have 
> the same elements sizes}}
>   ~ ^   
> 
> Given your test changes, it seems that now, instead of "vector operands do 
> not have the same number of elements" we would get "vector operands do not 
> have the same elements sizes". I rather we stay with the first. Additionally, 
> even if we had "vector operands do not have the same elements sizes" for 
> `vector_size`, this should never be demoted to a warning.
Argument of a GNU vector size attribute specifies vector size measured in 
bytes(see https://gcc.gnu.org/onlinedocs/gcc/Vector-Extensions.html). So you 
got right diagnostics. Both compilers with and without my changes print the 
same diagnostics  for yours case. Here is a small testcase used both GNU and 
clang extensions

$ cat bruno.c   

 
typedef __attribute__((__ext_vector_type__(8))) int vector_int8;
typedef __attribute__((__ext_vector_type__(8))) unsigned char vector_uchar8;
 
typedef __attribute__((vector_size(8))) int vector_int8n;
typedef __attribute__((vector_size(8))) unsigned char vector_uchar8n;
 
vector_int8  vi8;
vector_uchar8 vuc8;
vector_int8n  vi8n;
vector_uchar8n vuc8n;
 
int foo() {
  vi8 = vi8 << vuc8;
  vi8n = vi8n << vuc8n;

$ clang -c bruno.c -Wno-error-vec-elem-size 

 
bruno.c:13:13: warning: vector operands do not have the same elements sizes 
('vector_int8' (vector of 8 'int' values) and 'vector_uchar8' (vector of 8 
'unsigned char' values)) [-Wvec-elem-size]
  vi8 = vi8 << vuc8;
~~~ ^  
bruno.c:14:15: error: vector operands do not have the same number of elements 
('vector_int8n' (vector of 2 'int' values) and 'vector_uchar8n' (vector of 8 
'unsigned char' values))
  vi8n = vi8n << vuc8n;

The compiler without the changes prints the second error only (bruno.c:14:15).



Comment at: llvm/tools/clang/lib/Sema/SemaExpr.cpp:8795-8798
+if (S.Diags.getDiagnosticLevel(
+diag::warn_typecheck_vector_element_sizes_not_equal, Loc) ==
+DiagnosticsEngine::Level::Error)
+  return QualType();

majnemer wrote:
> Why `return QualType()` here? Would returning `LHSType` provide confusing or 
> incorrect diagnostics?
This is really excessive. Thanks.


https://reviews.llvm.org/D24669



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

[PATCH] D24669: {Sema] Gcc compatibility of vector shift.

[Vladimir Yakovlev via cfe-commits]

vbyakovlcl updated this revision to Diff 74269.

https://reviews.llvm.org/D24669

Files:
  llvm/tools/clang/include/clang/Basic/DiagnosticSemaKinds.td
  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
@@ -8784,6 +8784,16 @@
 << 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 &&
+  S.Context.getTypeSize(LHSBT) != S.Context.getTypeSize(RHSBT)) {
+S.Diag(Loc, diag::warn_typecheck_vector_element_sizes_not_equal)
+<< LHS.get()->getType() << RHS.get()->getType()
+<< LHS.get()->getSourceRange() << RHS.get()->getSourceRange();
+  }
+}
   } else {
 // ...else expand RHS to match the number of elements in LHS.
 QualType VecTy =
Index: llvm/tools/clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- llvm/tools/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ llvm/tools/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -2301,6 +2301,9 @@
   "cannot convert between vector and non-scalar values (%0 and %1)">;
 def err_typecheck_vector_lengths_not_equal : Error<
   "vector operands do not have the same number of elements (%0 and %1)">;
+def warn_typecheck_vector_element_sizes_not_equal : Warning<
+  "vector operands do not have the same elements sizes (%0 and %1)">,
+  InGroup>, DefaultError;
 def err_ext_vector_component_exceeds_length : Error<
   "vector component access exceeds type %0">;
 def err_ext_vector_component_name_illegal : Error<
Index: llvm/tools/clang/test/CodeGen/vecshift.c
===
--- llvm/tools/clang/test/CodeGen/vecshift.c
+++ llvm/tools/clang/test/CodeGen/vecshift.c
@@ -1,5 +1,7 @@
-// RUN: %clang_cc1 -emit-llvm %s -o - | FileCheck %s
+// RUN: %clang_cc1  -Wno-error-vec-elem-size -emit-llvm %s -o - | FileCheck %s
+// RUN: %clang_cc1  -Wno-error-vec-elem-size -DEXT -emit-llvm %s -o - | FileCheck %s
 
+#ifdef EXT
 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;
@@ -12,6 +14,20 @@
 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;
+#else
+typedef __attribute__((vector_size(8))) char vector_char8;
+typedef __attribute__((vector_size(16))) short vector_short8;
+typedef __attribute__((vector_size(32))) int vector_int8;
+typedef __attribute__((vector_size(8))) unsigned char vector_uchar8;
+typedef __attribute__((vector_size(16))) unsigned short vector_ushort8;
+typedef __attribute__((vector_size(32))) unsigned int vector_uint8;
+typedef __attribute__((vector_size(4))) char vector_char4;
+typedef __attribute__((vector_size(4))) short vector_short4;
+typedef __attribute__((vector_size(16))) int vector_int4;
+typedef __attribute__((vector_size(4))) unsigned char vector_uchar4;
+typedef __attribute__((vector_size(8))) unsigned short vector_ushort4;
+typedef __attribute__((vector_size(16))) unsigned int vector_uint4;
+#endif
 
 char c;
 short s;
Index: llvm/tools/clang/test/Sema/vecshift.c
===
--- llvm/tools/clang/test/Sema/vecshift.c
+++ llvm/tools/clang/test/Sema/vecshift.c
@@ -1,5 +1,9 @@
-// RUN: %clang_cc1 -fsyntax-only -verify %s
+// RUN: %clang_cc1 -fsyntax-only -DERR -verify %s
+// RUN: %clang_cc1 -fsyntax-only -Wno-error-vec-elem-size -verify %s
+// RUN: %clang_cc1 -fsyntax-only -DEXT -DERR -verify %s
+// RUN: %clang_cc1 -fsyntax-only -DEXT -Wno-error-vec-elem-size -verify %s
 
+#ifdef EXT
 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;
@@ -12,6 +16,20 @@
 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;
+#else
+typedef __attribute__((vector_size(8))) char vector_char8;
+typedef __attribute__((vector_size(16))) short vector_short8;
+typedef __attribute__((vector_size(32))) int vector_int8;
+typedef __attribute__((vector_size(8))) unsigned char 

[PATCH] D24669: {Sema] Gcc compatibility of vector shift.

[David Majnemer via cfe-commits]

majnemer added inline comments.



Comment at: llvm/tools/clang/lib/Sema/SemaExpr.cpp:8795-8798
+if (S.Diags.getDiagnosticLevel(
+diag::warn_typecheck_vector_element_sizes_not_equal, Loc) ==
+DiagnosticsEngine::Level::Error)
+  return QualType();

Why `return QualType()` here? Would returning `LHSType` provide confusing or 
incorrect diagnostics?


https://reviews.llvm.org/D24669



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

[PATCH] D24669: {Sema] Gcc compatibility of vector shift.

[Bruno Cardoso Lopes via cfe-commits]

bruno added inline comments.



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

vbyakovlcl wrote:
> bruno wrote:
> > Besides `__ext_vector_type__`, would this also trigger for `vector_size`? 
> > Right now this is an error for `vector_size` primarily because the number 
> > of elements is different, can you confirm this won't change?
> I compare vector element sizes, so there must not be any differencies caused 
> by different triggers. I added additional type definitions to the tests. All 
> compiles fain.

I don't think this is right. When I try to compile similar code for 
`vector_size` without your patch, I get the errors:

/tmp/x.c:80:15: error: vector operands do not have the same number of elements 
('vector_int8n' (vector of 2 'int' values) and 'vector_uchar8n' (vector of 8 
'unsigned char' values))
  vi8n = vi8n << vuc8n; // expected-warning {{vector operands do not have the 
same elements sizes}}
  ^  ~
/tmp/x.c:81:17: error: vector operands do not have the same number of elements 
('vector_uchar8n' (vector of 8 'unsigned char' values) and 'vector_int8n' 
(vector of 2 'int' values))
  vuc8n = vuc8n << vi8n; // expected-warning {{vector operands do not have the 
same elements sizes}}
  ~ ^  
/tmp/x.c:82:17: error: vector operands do not have the same number of elements 
('vector_ushort8n' (vector of 4 'unsigned short' values) and 'vector_uint8n' 
(vector of 2 'unsigned int' values))
  vus8n = vus8n << vui8n; // expected-warning {{vector operands do not have the 
same elements sizes}}
  ~ ^  ~
/tmp/x.c:83:17: error: vector operands do not have the same number of elements 
('vector_uint8n' (vector of 2 'unsigned int' values) and 'vector_short8n' 
(vector of 4 'short' values))
  vui8n = vui8n << vs8n; // expected-warning {{vector operands do not have the 
same elements sizes}}
  ~ ^   

Given your test changes, it seems that now, instead of "vector operands do not 
have the same number of elements" we would get "vector operands do not have the 
same elements sizes". I rather we stay with the first. Additionally, even if we 
had "vector operands do not have the same elements sizes" for `vector_size`, 
this should never be demoted to a warning.


https://reviews.llvm.org/D24669



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

[PATCH] D24669: {Sema] Gcc compatibility of vector shift.

[Vladimir Yakovlev via cfe-commits]

vbyakovlcl added inline comments.



Comment at: llvm/tools/clang/include/clang/Basic/DiagnosticSemaKinds.td:2306
+  "vector operands do not have the same elements sizes (%0 and %1)">,
+  InGroup>, DefaultError;
 def err_ext_vector_component_exceeds_length : Error<

bruno wrote:
> Although the motivation is to support the same warning present in GCC, I 
> think this is helpful enough anyway so that we might skip calling it 
> "gnu-vec-elem-size" and have a more generic name instead? How about plain 
> "vec-elem-size"?
I'm agree.



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

bruno wrote:
> Besides `__ext_vector_type__`, would this also trigger for `vector_size`? 
> Right now this is an error for `vector_size` primarily because the number of 
> elements is different, can you confirm this won't change?
I compare vector element sizes, so there must not be any differencies caused by 
different triggers. I added additional type definitions to the tests. All 
compiles fain.


https://reviews.llvm.org/D24669



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

[PATCH] D24669: {Sema] Gcc compatibility of vector shift.

[Vladimir Yakovlev via cfe-commits]

vbyakovlcl updated this revision to Diff 74123.

https://reviews.llvm.org/D24669

Files:
  llvm/tools/clang/include/clang/Basic/DiagnosticSemaKinds.td
  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
@@ -8784,6 +8784,20 @@
 << 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 &&
+  S.Context.getTypeSize(LHSBT) != S.Context.getTypeSize(RHSBT)) {
+S.Diag(Loc, diag::warn_typecheck_vector_element_sizes_not_equal)
+<< LHS.get()->getType() << RHS.get()->getType()
+<< LHS.get()->getSourceRange() << RHS.get()->getSourceRange();
+if (S.Diags.getDiagnosticLevel(
+diag::warn_typecheck_vector_element_sizes_not_equal, Loc) ==
+DiagnosticsEngine::Level::Error)
+  return QualType();
+  }
+}
   } else {
 // ...else expand RHS to match the number of elements in LHS.
 QualType VecTy =
Index: llvm/tools/clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- llvm/tools/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ llvm/tools/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -2301,6 +2301,9 @@
   "cannot convert between vector and non-scalar values (%0 and %1)">;
 def err_typecheck_vector_lengths_not_equal : Error<
   "vector operands do not have the same number of elements (%0 and %1)">;
+def warn_typecheck_vector_element_sizes_not_equal : Warning<
+  "vector operands do not have the same elements sizes (%0 and %1)">,
+  InGroup>, DefaultError;
 def err_ext_vector_component_exceeds_length : Error<
   "vector component access exceeds type %0">;
 def err_ext_vector_component_name_illegal : Error<
Index: llvm/tools/clang/test/CodeGen/vecshift.c
===
--- llvm/tools/clang/test/CodeGen/vecshift.c
+++ llvm/tools/clang/test/CodeGen/vecshift.c
@@ -1,5 +1,7 @@
-// RUN: %clang_cc1 -emit-llvm %s -o - | FileCheck %s
+// RUN: %clang_cc1  -Wno-error-vec-elem-size -emit-llvm %s -o - | FileCheck %s
+// RUN: %clang_cc1  -Wno-error-vec-elem-size -DEXT -emit-llvm %s -o - | FileCheck %s
 
+#ifdef EXT
 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;
@@ -12,6 +14,20 @@
 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;
+#else
+typedef __attribute__((vector_size(8))) char vector_char8;
+typedef __attribute__((vector_size(16))) short vector_short8;
+typedef __attribute__((vector_size(32))) int vector_int8;
+typedef __attribute__((vector_size(8))) unsigned char vector_uchar8;
+typedef __attribute__((vector_size(16))) unsigned short vector_ushort8;
+typedef __attribute__((vector_size(32))) unsigned int vector_uint8;
+typedef __attribute__((vector_size(4))) char vector_char4;
+typedef __attribute__((vector_size(4))) short vector_short4;
+typedef __attribute__((vector_size(16))) int vector_int4;
+typedef __attribute__((vector_size(4))) unsigned char vector_uchar4;
+typedef __attribute__((vector_size(8))) unsigned short vector_ushort4;
+typedef __attribute__((vector_size(16))) unsigned int vector_uint4;
+#endif
 
 char c;
 short s;
Index: llvm/tools/clang/test/Sema/vecshift.c
===
--- llvm/tools/clang/test/Sema/vecshift.c
+++ llvm/tools/clang/test/Sema/vecshift.c
@@ -1,5 +1,9 @@
-// RUN: %clang_cc1 -fsyntax-only -verify %s
+// RUN: %clang_cc1 -fsyntax-only -DERR -verify %s
+// RUN: %clang_cc1 -fsyntax-only -Wno-error-vec-elem-size -verify %s
+// RUN: %clang_cc1 -fsyntax-only -DEXT -DERR -verify %s
+// RUN: %clang_cc1 -fsyntax-only -DEXT -Wno-error-vec-elem-size -verify %s
 
+#ifdef EXT
 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;
@@ -12,6 +16,20 @@
 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;
+#else
+typedef __attribute__((vector_size(8))) char 

[PATCH] D24669: {Sema] Gcc compatibility of vector shift.

[Bruno Cardoso Lopes via cfe-commits]

bruno added a reviewer: bruno.
bruno added inline comments.



Comment at: llvm/tools/clang/include/clang/Basic/DiagnosticSemaKinds.td:2306
+  "vector operands do not have the same elements sizes (%0 and %1)">,
+  InGroup>, DefaultError;
 def err_ext_vector_component_exceeds_length : Error<

Although the motivation is to support the same warning present in GCC, I think 
this is helpful enough anyway so that we might skip calling it 
"gnu-vec-elem-size" and have a more generic name instead? How about plain 
"vec-elem-size"?



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

Besides `__ext_vector_type__`, would this also trigger for `vector_size`? Right 
now this is an error for `vector_size` primarily because the number of elements 
is different, can you confirm this won't change?


https://reviews.llvm.org/D24669



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

[PATCH] D24669: {Sema] Gcc compatibility of vector shift.

[Vladimir Yakovlev via cfe-commits]

vbyakovlcl added inline comments.


> ahatanak wrote in SemaExpr.cpp:8787
> Is it possible to use ASTContext::getTypeSize here?

You are right.

https://reviews.llvm.org/D24669



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

[PATCH] D24669: {Sema] Gcc compatibility of vector shift.

[Vladimir Yakovlev via cfe-commits]

vbyakovlcl updated this revision to Diff 73642.

https://reviews.llvm.org/D24669

Files:
  llvm/tools/clang/include/clang/Basic/DiagnosticSemaKinds.td
  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
@@ -8784,6 +8784,20 @@
 << 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 &&
+  S.Context.getTypeSize(LHSBT) != S.Context.getTypeSize(RHSBT)) {
+S.Diag(Loc, diag::warn_typecheck_vector_element_sizes_not_equal)
+<< LHS.get()->getType() << RHS.get()->getType()
+<< LHS.get()->getSourceRange() << RHS.get()->getSourceRange();
+if (S.Diags.getDiagnosticLevel(
+diag::warn_typecheck_vector_element_sizes_not_equal, Loc) ==
+DiagnosticsEngine::Level::Error)
+  return QualType();
+  }
+}
   } else {
 // ...else expand RHS to match the number of elements in LHS.
 QualType VecTy =
Index: llvm/tools/clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- llvm/tools/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ llvm/tools/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -2301,6 +2301,9 @@
   "cannot convert between vector and non-scalar values (%0 and %1)">;
 def err_typecheck_vector_lengths_not_equal : Error<
   "vector operands do not have the same number of elements (%0 and %1)">;
+def warn_typecheck_vector_element_sizes_not_equal : Warning<
+  "vector operands do not have the same elements sizes (%0 and %1)">,
+  InGroup>, DefaultError;
 def err_ext_vector_component_exceeds_length : Error<
   "vector component access exceeds type %0">;
 def err_ext_vector_component_name_illegal : Error<
Index: llvm/tools/clang/test/CodeGen/vecshift.c
===
--- llvm/tools/clang/test/CodeGen/vecshift.c
+++ llvm/tools/clang/test/CodeGen/vecshift.c
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -emit-llvm %s -o - | FileCheck %s
+// RUN: %clang_cc1  -Wno-error-gnu-vec-elem-size -emit-llvm %s -o - | 
FileCheck %s
 
 typedef __attribute__((__ext_vector_type__(8))) char vector_char8;
 typedef __attribute__((__ext_vector_type__(8))) short vector_short8;
Index: llvm/tools/clang/test/Sema/vecshift.c
===
--- llvm/tools/clang/test/Sema/vecshift.c
+++ llvm/tools/clang/test/Sema/vecshift.c
@@ -1,4 +1,5 @@
-// RUN: %clang_cc1 -fsyntax-only -verify %s
+// RUN: %clang_cc1 -fsyntax-only -DERR -verify %s
+// RUN: %clang_cc1 -fsyntax-only -Wno-error-gnu-vec-elem-size -verify %s
 
 typedef __attribute__((__ext_vector_type__(8))) char vector_char8;
 typedef __attribute__((__ext_vector_type__(8))) short vector_short8;
@@ -48,16 +49,30 @@
   vus8 = 1 << vus8;
 
   vc8 = vc8 << vc8;
-  vi8 = vi8 << vuc8;
-  vuc8 = vuc8 << vi8;
-  vus8 = vus8 << vui8;
-  vui8 = vui8 << vs8;
+#ifdef ERR
+  vi8 = vi8 << vuc8; // expected-error {{vector operands do not have the same 
elements sizes}}
+  vuc8 = vuc8 << vi8; // expected-error {{vector operands do not have the same 
elements sizes}}
+  vus8 = vus8 << vui8; // expected-error {{vector operands do not have the 
same elements sizes}}
+  vui8 = vui8 << vs8; // expected-error {{vector operands do not have the same 
elements sizes}}
+#else
+  vi8 = vi8 << vuc8; // expected-warning {{vector operands do not have the 
same elements sizes}}
+  vuc8 = vuc8 << vi8; // expected-warning {{vector operands do not have the 
same elements sizes}}
+  vus8 = vus8 << vui8; // expected-warning {{vector operands do not have the 
same elements sizes}}
+  vui8 = vui8 << vs8; // expected-warning {{vector operands do not have the 
same elements sizes}}
+#endif
 
   vc8 <<= vc8;
-  vi8 <<= vuc8;
-  vuc8 <<= vi8;
-  vus8 <<= vui8;
-  vui8 <<= vs8;
+#ifdef ERR
+  vi8 <<= vuc8; // expected-error {{vector operands do not have the same 
elements sizes}}
+  vuc8 <<= vi8; // expected-error {{vector operands do not have the same 
elements sizes}}
+  vus8 <<= vui8; // expected-error {{vector operands do not have the same 
elements sizes}}
+  vui8 <<= vs8; // expected-error {{vector operands do not have the same 
elements sizes}}
+#else
+  vi8 <<= vuc8; // expected-warning {{vector operands do not have the same 
elements sizes}}
+  vuc8 <<= vi8; // expected-warning {{vector operands do not have the same 
elements sizes}}
+  vus8 <<= vui8; // expected-warning {{vector operands do not have the same 
elements sizes}}
+  vui8 <<= vs8; 

[PATCH] D24669: {Sema] Gcc compatibility of vector shift.

[Akira Hatanaka via cfe-commits]

ahatanak added inline comments.


> SemaExpr.cpp:8787
>  }
> +if (!S.LangOpts.OpenCL && !S.LangOpts.ZVector) {
> +  const BuiltinType *LHSBT = LHSEleType->getAs();

Is it possible to use ASTContext::getTypeSize here?

https://reviews.llvm.org/D24669



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

[PATCH] D24669: {Sema] Gcc compatibility of vector shift.

[Vladimir Yakovlev via cfe-commits]

vbyakovlcl removed rL LLVM as the repository for this revision.
vbyakovlcl updated this revision to Diff 73468.

https://reviews.llvm.org/D24669

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
@@ -8784,6 +8784,65 @@
 << 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:
+  DiffSizes =
+  RHSKind != BuiltinType::Char_U && RHSKind != BuiltinType::Char_S;
+  break;
+case BuiltinType::Short:
+  DiffSizes = RHSKind != BuiltinType::UShort;
+  break;
+case BuiltinType::UShort:
+  DiffSizes = RHSKind != BuiltinType::Short;
+  break;
+case BuiltinType::Int:
+  DiffSizes = RHSKind != BuiltinType::UInt;
+  break;
+case BuiltinType::UInt:
+  DiffSizes = RHSKind != BuiltinType::Int;
+  break;
+case BuiltinType::Long:
+  DiffSizes = RHSKind != BuiltinType::ULong;
+  break;
+case BuiltinType::ULong:
+  DiffSizes = RHSKind != BuiltinType::Long;
+  break;
+case BuiltinType::LongLong:
+  DiffSizes = RHSKind != BuiltinType::ULongLong;
+  break;
+case BuiltinType::ULongLong:
+  DiffSizes = RHSKind != BuiltinType::LongLong;
+  break;
+default:
+  DiffSizes = true;
+  break;
+}
+if (DiffSizes) {
+  S.Diag(Loc, diag::warn_typecheck_vector_element_sizes_not_equal)
+  << LHS.get()->getType() << RHS.get()->getType()
+  << LHS.get()->getSourceRange() << RHS.get()->getSourceRange();
+  if (S.Diags.getDiagnosticLevel(
+  diag::warn_typecheck_vector_element_sizes_not_equal, Loc) ==
+  DiagnosticsEngine::Level::Error)
+return QualType();
+}
+  }
+}
   } else {
 // ...else expand RHS to match the number of elements in LHS.
 QualType VecTy =
Index: llvm/tools/clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- llvm/tools/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ llvm/tools/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -2301,6 +2301,9 @@
   "cannot convert between vector and non-scalar values (%0 and %1)">;
 def err_typecheck_vector_lengths_not_equal : Error<
   "vector operands do not have the same number of elements (%0 and %1)">;
+def warn_typecheck_vector_element_sizes_not_equal : Warning<
+  "vector operands do not have the same elements sizes (%0 and %1)">,
+  InGroup>, DefaultError;
 def err_ext_vector_component_exceeds_length : Error<
   "vector component access exceeds type %0">;
 def err_ext_vector_component_name_illegal : Error<
Index: llvm/tools/clang/test/Sema/vecshift.c
===
--- llvm/tools/clang/test/Sema/vecshift.c
+++ llvm/tools/clang/test/Sema/vecshift.c
@@ -1,4 +1,5 @@
-// RUN: %clang_cc1 -fsyntax-only -verify %s
+// RUN: %clang_cc1 -fsyntax-only -DERR -verify %s
+// RUN: %clang_cc1 -fsyntax-only -Wno-error-gnu-vec-elem-size -verify %s
 
 typedef __attribute__((__ext_vector_type__(8))) char vector_char8;
 typedef __attribute__((__ext_vector_type__(8))) short vector_short8;
@@ -48,16 +49,30 @@
   vus8 = 1 << vus8;
 
   vc8 = vc8 << vc8;
-  vi8 = vi8 << vuc8;
-  vuc8 = vuc8 << vi8;
-  vus8 = vus8 << vui8;
-  vui8 = vui8 << vs8;
+#ifdef ERR
+  vi8 = vi8 << vuc8; // expected-error {{vector operands do not have the same elements sizes}}
+  vuc8 = vuc8 << vi8; // expected-error {{vector operands do not have the same elements sizes}}
+  vus8 = vus8 << vui8; // expected-error {{vector operands do not have the same elements sizes}}
+  vui8 = vui8 << vs8; // expected-error {{vector operands do not have the same elements sizes}}
+#else
+  vi8 = vi8 << vuc8; // expected-warning {{vector operands do not have the same elements sizes}}
+  vuc8 = vuc8 << vi8; 

[PATCH] D24669: {Sema] Gcc compatibility of vector shift.

[Vladimir Yakovlev via cfe-commits]

vbyakovlcl added inline comments.


> aaron.ballman wrote in DiagnosticGroups.td:522
> I would not add this as a diagnostic group, but instead use an ad-hoc group 
> on the diagnostic itself. I don't think this is going to see very many 
> diagnostics covered by the same group, but if that turns out to be the case, 
> we can switch then.

Ok, done

> aaron.ballman wrote in DiagnosticSemaKinds.td:2306
> I'm not the best one to answer that question, but we typically avoid adding 
> new off-by-default diagnostics. Since GCC prints this as an error message and 
> this patch is for GCC compatibility, it seems weird to me to add this as an 
> off-by-default warning.

I did similar to GCC - error by default.

https://reviews.llvm.org/D24669



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

[PATCH] D24669: {Sema] Gcc compatibility of vector shift.

[Aaron Ballman via cfe-commits]

aaron.ballman added inline comments.


> vbyakovlcl wrote in DiagnosticGroups.td:522
> Gcc prints error messages
> 
> t.c:21:13: error: invalid operands to binary << (have vector_int8 and 
> vector_short8)
> 
>   vi8 = vi8 << vs8;
> 
> I could not find a flag controlling this error.

I would not add this as a diagnostic group, but instead use an ad-hoc group on 
the diagnostic itself. I don't think this is going to see very many diagnostics 
covered by the same group, but if that turns out to be the case, we can switch 
then.

> vbyakovlcl wrote in DiagnosticSemaKinds.td:2306
> The question is: would we like to have the feature as a clang extension?

I'm not the best one to answer that question, but we typically avoid adding new 
off-by-default diagnostics. Since GCC prints this as an error message and this 
patch is for GCC compatibility, it seems weird to me to add this as an 
off-by-default warning.

Repository:
  rL LLVM

https://reviews.llvm.org/D24669



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

Re: [PATCH] D24669: {Sema] Gcc compatibility of vector shift.

[Vladimir Yakovlev via cfe-commits]

vbyakovlcl added inline comments.


Comment at: llvm/tools/clang/include/clang/Basic/DiagnosticGroups.td:522
@@ -521,2 +521,3 @@
 def GNUZeroVariadicMacroArguments : 
DiagGroup<"gnu-zero-variadic-macro-arguments">;
+def GNUVecElemSize : DiagGroup<"gnu-vec-elem-size">;
 def Fallback : DiagGroup<"fallback">;

aaron.ballman wrote:
> Is this the same warning flag GCC uses?
Gcc prints error messages 

t.c:21:13: error: invalid operands to binary << (have vector_int8 and 
vector_short8)
   vi8 = vi8 << vs8;

I could not find a flag controlling this error.


Comment at: llvm/tools/clang/include/clang/Basic/DiagnosticSemaKinds.td:2306
@@ -2304,1 +2305,3 @@
+  "vector operands do not have the same elements sizes (%0 and %1)">,
+  InGroup, DefaultIgnore;
 def err_ext_vector_component_exceeds_length : Error<

aaron.ballman wrote:
> Why is this off by default?
The question is: would we like to have the feature as a clang extension? 


Repository:
  rL LLVM

https://reviews.llvm.org/D24669



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

Re: [PATCH] D24669: {Sema] Gcc compatibility of vector shift.

[Aaron Ballman via cfe-commits]

aaron.ballman added a comment.

In https://reviews.llvm.org/D24669#548984, @vbyakovlcl wrote:

> Clang 3.8 balances vector shift operand erroneous using CheckVectorOperands 
> which converts one of operand to the type of another. In 
> https://reviews.llvm.org/D21678 it was fixed by using checkVectorShift 
> instead. As result clang does not emit error if shift operands have different 
> element sizes (bat gcc does).


Ah, thank you for the explanation!



Comment at: llvm/tools/clang/include/clang/Basic/DiagnosticGroups.td:522
@@ -521,2 +521,3 @@
 def GNUZeroVariadicMacroArguments : 
DiagGroup<"gnu-zero-variadic-macro-arguments">;
+def GNUVecElemSize : DiagGroup<"gnu-vec-elem-size">;
 def Fallback : DiagGroup<"fallback">;

Is this the same warning flag GCC uses?


Comment at: llvm/tools/clang/include/clang/Basic/DiagnosticSemaKinds.td:2306
@@ -2304,1 +2305,3 @@
+  "vector operands do not have the same elements sizes (%0 and %1)">,
+  InGroup, DefaultIgnore;
 def err_ext_vector_component_exceeds_length : Error<

Why is this off by default?


Repository:
  rL LLVM

https://reviews.llvm.org/D24669



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

Re: [PATCH] D24669: {Sema] Gcc compatibility of vector shift.

[Vladimir Yakovlev via cfe-commits]

vbyakovlcl added a comment.

Clang 3.8 balances vector shift operand erroneous using CheckVectorOperands 
which converts one of operand to the type of another. In 
https://reviews.llvm.org/D21678 it was fixed by using checkVectorShift instead. 
As result clang does not emit error if shift operands have different element 
sizes (bat gcc does).


Repository:
  rL LLVM

https://reviews.llvm.org/D24669



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

Re: [PATCH] D24669: {Sema] Gcc compatibility of vector shift.

[Aaron Ballman via cfe-commits]

aaron.ballman added a comment.

In clang 3.8, your test code already produces a diagnostic for me: 
http://coliru.stacked-crooked.com/a/752a4ea34123bdae


Repository:
  rL LLVM

https://reviews.llvm.org/D24669



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

[PATCH] D24669: {Sema] Gcc compatibility of vector shift.

[Vladimir Yakovlev via cfe-commits]

vbyakovlcl created this revision.
vbyakovlcl added reviewers: aaron.ballman, ahatanak.
vbyakovlcl added a subscriber: cfe-commits.
vbyakovlcl set the repository for this revision to rL LLVM.

Gcc prints error if elements of left and right parts of a shift have different 
sizes. This patch is provided the GCC compatibility.

Repository:
  rL LLVM

https://reviews.llvm.org/D24669

Files:
  llvm/tools/clang/include/clang/Basic/DiagnosticGroups.td
  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
@@ -8784,6 +8784,65 @@
 << 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:
+  DiffSizes =
+  RHSKind != BuiltinType::Char_U && RHSKind != BuiltinType::Char_S;
+  break;
+case BuiltinType::Short:
+  DiffSizes = RHSKind != BuiltinType::UShort;
+  break;
+case BuiltinType::UShort:
+  DiffSizes = RHSKind != BuiltinType::Short;
+  break;
+case BuiltinType::Int:
+  DiffSizes = RHSKind != BuiltinType::UInt;
+  break;
+case BuiltinType::UInt:
+  DiffSizes = RHSKind != BuiltinType::Int;
+  break;
+case BuiltinType::Long:
+  DiffSizes = RHSKind != BuiltinType::ULong;
+  break;
+case BuiltinType::ULong:
+  DiffSizes = RHSKind != BuiltinType::Long;
+  break;
+case BuiltinType::LongLong:
+  DiffSizes = RHSKind != BuiltinType::ULongLong;
+  break;
+case BuiltinType::ULongLong:
+  DiffSizes = RHSKind != BuiltinType::LongLong;
+  break;
+default:
+  DiffSizes = true;
+  break;
+}
+if (DiffSizes) {
+  S.Diag(Loc, diag::warn_typecheck_vector_element_sizes_not_equal)
+  << LHS.get()->getType() << RHS.get()->getType()
+  << LHS.get()->getSourceRange() << RHS.get()->getSourceRange();
+  if (S.Diags.getDiagnosticLevel(
+  diag::warn_typecheck_vector_element_sizes_not_equal, Loc) ==
+  DiagnosticsEngine::Level::Error)
+return QualType();
+}
+  }
+}
   } else {
 // ...else expand RHS to match the number of elements in LHS.
 QualType VecTy =
Index: llvm/tools/clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- llvm/tools/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ llvm/tools/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -2301,6 +2301,9 @@
   "cannot convert between vector and non-scalar values (%0 and %1)">;
 def err_typecheck_vector_lengths_not_equal : Error<
   "vector operands do not have the same number of elements (%0 and %1)">;
+def warn_typecheck_vector_element_sizes_not_equal : Warning<
+  "vector operands do not have the same elements sizes (%0 and %1)">,
+  InGroup, DefaultIgnore;
 def err_ext_vector_component_exceeds_length : Error<
   "vector component access exceeds type %0">;
 def err_ext_vector_component_name_illegal : Error<
Index: llvm/tools/clang/include/clang/Basic/DiagnosticGroups.td
===
--- llvm/tools/clang/include/clang/Basic/DiagnosticGroups.td
+++ llvm/tools/clang/include/clang/Basic/DiagnosticGroups.td
@@ -519,6 +519,7 @@
 def ZeroLengthArray : DiagGroup<"zero-length-array">;
 def GNUZeroLineDirective : DiagGroup<"gnu-zero-line-directive">;
 def GNUZeroVariadicMacroArguments : DiagGroup<"gnu-zero-variadic-macro-arguments">;
+def GNUVecElemSize : DiagGroup<"gnu-vec-elem-size">;
 def Fallback : DiagGroup<"fallback">;
 
 // This covers both the deprecated case (in C++98)
@@ -758,7 +759,8 @@
 GNUStringLiteralOperatorTemplate,
 GNUUnionCast, GNUVariableSizedTypeNotAtEnd,
 ZeroLengthArray, GNUZeroLineDirective,
-GNUZeroVariadicMacroArguments]>;
+