Re: r298369 - [OpenCL] Added diagnostic for checking length of vector

2017-10-12 Thread Bruno Cardoso Lopes via cfe-commits 
On Thu, Oct 12, 2017 at 8:39 AM, Anastasia Stulova
 wrote:
>
> I think this bit is a bit confusing to us. Some of our original OpenCL
> checks were removed in some places because in some cases OpenCL semantic was
> adopted elsewhere.

It's confusing indeed. We should probably enhance the docs for
scenarios like this.

> But I think this should indeed go under OpenCL check.

https://reviews.llvm.org/D38868

Thanks,

-- 
Bruno Cardoso Lopes
http://www.brunocardoso.cc
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: r298369 - [OpenCL] Added diagnostic for checking length of vector

2017-10-12 Thread Anastasia Stulova via cfe-commits 

I think this bit is a bit confusing to us. Some of our original OpenCL checks 
were removed in some places because in some cases OpenCL semantic was adopted 
elsewhere. But I think this should indeed go under OpenCL check.

Thanks,
Anastasia


From: Bruno Cardoso Lopes 
Sent: 11 October 2017 00:27
To: Egor Churaev
Cc: cfe-commits; Anastasia Stulova
Subject: Re: r298369 - [OpenCL] Added diagnostic for checking length of vector

Hi Egor,

On Tue, Mar 21, 2017 at 6:20 AM, Egor Churaev via cfe-commits
 wrote:
> Author: echuraev
> Date: Tue Mar 21 08:20:57 2017
> New Revision: 298369
>
> URL: http://llvm.org/viewvc/llvm-project?rev=298369&view=rev
> Log:
> [OpenCL] Added diagnostic for checking length of vector
>
> Reviewers: Anastasia, cfe-commits
>
> Reviewed By: Anastasia
>
> Subscribers: bader, yaxunl
>
> Differential Revision: https://reviews.llvm.org/D30937
>
> Added:
> cfe/trunk/test/SemaOpenCL/vector_swizzle_length.cl
> Modified:
> cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
> cfe/trunk/lib/Sema/SemaExprMember.cpp
>
> Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=298369&r1=298368&r2=298369&view=diff
> ==
> --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
> +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Tue Mar 21 08:20:57 
> 2017
> @@ -8236,6 +8236,8 @@ def err_opencl_ptrptr_kernel_param : Err
>  def err_kernel_arg_address_space : Error<
>"pointer arguments to kernel functions must reside in '__global', "
>"'__constant' or '__local' address space">;
> +def err_opencl_ext_vector_component_invalid_length : Error<
> +  "vector component access has invalid length %0.  Supported: 
> 1,2,3,4,8,16.">;
>  def err_opencl_function_variable : Error<
>"%select{non-kernel function|function scope}0 variable cannot be declared 
> in %1 address space">;
>  def err_static_function_scope : Error<
>
> Modified: cfe/trunk/lib/Sema/SemaExprMember.cpp
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExprMember.cpp?rev=298369&r1=298368&r2=298369&view=diff
> ==
> --- cfe/trunk/lib/Sema/SemaExprMember.cpp (original)
> +++ cfe/trunk/lib/Sema/SemaExprMember.cpp Tue Mar 21 08:20:57 2017
> @@ -284,6 +284,14 @@ IsRGBA(char c) {
>}
>  }
>
> +// OpenCL v1.1, s6.1.7
> +// The component swizzle length must be in accordance with the acceptable
> +// vector sizes.
> +static bool IsValidOpenCLComponentSwizzleLength(unsigned len)
> +{
> +  return (len >= 1 && len <= 4) || len == 8 || len == 16;
> +}
> +
>  /// Check an ext-vector component access expression.
>  ///
>  /// VK should be set in advance to the value kind of the base
> @@ -376,6 +384,19 @@ CheckExtVectorComponent(Sema &S, QualTyp
>  }
>}
>
> +  if (!HalvingSwizzle) {
> +unsigned SwizzleLength = CompName->getLength();
> +
> +if (HexSwizzle)
> +  SwizzleLength--;
> +
> +if (IsValidOpenCLComponentSwizzleLength(SwizzleLength) == false) {
> +  S.Diag(OpLoc, diag::err_opencl_ext_vector_component_invalid_length)
> +<< SwizzleLength << SourceRange(CompLoc);
> +  return QualType();
> +}
> +  }
> +
>// The component accessor looks fine - now we need to compute the actual 
> type.
>// The vector type is implied by the component accessor. For example,
>// vec4.b is a float, vec4.xy is a vec2, vec4.rgb is a vec3, etc.
>
> Added: cfe/trunk/test/SemaOpenCL/vector_swizzle_length.cl
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaOpenCL/vector_swizzle_length.cl?rev=298369&view=auto
> ==
> --- cfe/trunk/test/SemaOpenCL/vector_swizzle_length.cl (added)
> +++ cfe/trunk/test/SemaOpenCL/vector_swizzle_length.cl Tue Mar 21 08:20:57 
> 2017
> @@ -0,0 +1,10 @@
> +// RUN: %clang_cc1 %s -verify -pedantic -fsyntax-only
> +
> +typedef float float8 __attribute__((ext_vector_type(8)));
> +
> +void foo() {
> +float8 f2 = (float8)(0, 0, 0, 0, 0, 0, 0, 0);
> +
> +f2.s01234; // expected-error {{vector component access has invalid 
> length 5.  Supported: 1,2,3,4,8,16}}
> +f2.xyzxy; // expected-error {{vector component access has invalid length 
> 5.  Supported: 1,2,3,4,8,16}}
> +}

Sorry for the necromancy her

Re: r298369 - [OpenCL] Added diagnostic for checking length of vector

2017-10-10 Thread Bruno Cardoso Lopes via cfe-commits 
Hi Egor,

On Tue, Mar 21, 2017 at 6:20 AM, Egor Churaev via cfe-commits
 wrote:
> Author: echuraev
> Date: Tue Mar 21 08:20:57 2017
> New Revision: 298369
>
> URL: http://llvm.org/viewvc/llvm-project?rev=298369&view=rev
> Log:
> [OpenCL] Added diagnostic for checking length of vector
>
> Reviewers: Anastasia, cfe-commits
>
> Reviewed By: Anastasia
>
> Subscribers: bader, yaxunl
>
> Differential Revision: https://reviews.llvm.org/D30937
>
> Added:
> cfe/trunk/test/SemaOpenCL/vector_swizzle_length.cl
> Modified:
> cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
> cfe/trunk/lib/Sema/SemaExprMember.cpp
>
> Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=298369&r1=298368&r2=298369&view=diff
> ==
> --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
> +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Tue Mar 21 08:20:57 
> 2017
> @@ -8236,6 +8236,8 @@ def err_opencl_ptrptr_kernel_param : Err
>  def err_kernel_arg_address_space : Error<
>"pointer arguments to kernel functions must reside in '__global', "
>"'__constant' or '__local' address space">;
> +def err_opencl_ext_vector_component_invalid_length : Error<
> +  "vector component access has invalid length %0.  Supported: 
> 1,2,3,4,8,16.">;
>  def err_opencl_function_variable : Error<
>"%select{non-kernel function|function scope}0 variable cannot be declared 
> in %1 address space">;
>  def err_static_function_scope : Error<
>
> Modified: cfe/trunk/lib/Sema/SemaExprMember.cpp
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExprMember.cpp?rev=298369&r1=298368&r2=298369&view=diff
> ==
> --- cfe/trunk/lib/Sema/SemaExprMember.cpp (original)
> +++ cfe/trunk/lib/Sema/SemaExprMember.cpp Tue Mar 21 08:20:57 2017
> @@ -284,6 +284,14 @@ IsRGBA(char c) {
>}
>  }
>
> +// OpenCL v1.1, s6.1.7
> +// The component swizzle length must be in accordance with the acceptable
> +// vector sizes.
> +static bool IsValidOpenCLComponentSwizzleLength(unsigned len)
> +{
> +  return (len >= 1 && len <= 4) || len == 8 || len == 16;
> +}
> +
>  /// Check an ext-vector component access expression.
>  ///
>  /// VK should be set in advance to the value kind of the base
> @@ -376,6 +384,19 @@ CheckExtVectorComponent(Sema &S, QualTyp
>  }
>}
>
> +  if (!HalvingSwizzle) {
> +unsigned SwizzleLength = CompName->getLength();
> +
> +if (HexSwizzle)
> +  SwizzleLength--;
> +
> +if (IsValidOpenCLComponentSwizzleLength(SwizzleLength) == false) {
> +  S.Diag(OpLoc, diag::err_opencl_ext_vector_component_invalid_length)
> +<< SwizzleLength << SourceRange(CompLoc);
> +  return QualType();
> +}
> +  }
> +
>// The component accessor looks fine - now we need to compute the actual 
> type.
>// The vector type is implied by the component accessor. For example,
>// vec4.b is a float, vec4.xy is a vec2, vec4.rgb is a vec3, etc.
>
> Added: cfe/trunk/test/SemaOpenCL/vector_swizzle_length.cl
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaOpenCL/vector_swizzle_length.cl?rev=298369&view=auto
> ==
> --- cfe/trunk/test/SemaOpenCL/vector_swizzle_length.cl (added)
> +++ cfe/trunk/test/SemaOpenCL/vector_swizzle_length.cl Tue Mar 21 08:20:57 
> 2017
> @@ -0,0 +1,10 @@
> +// RUN: %clang_cc1 %s -verify -pedantic -fsyntax-only
> +
> +typedef float float8 __attribute__((ext_vector_type(8)));
> +
> +void foo() {
> +float8 f2 = (float8)(0, 0, 0, 0, 0, 0, 0, 0);
> +
> +f2.s01234; // expected-error {{vector component access has invalid 
> length 5.  Supported: 1,2,3,4,8,16}}
> +f2.xyzxy; // expected-error {{vector component access has invalid length 
> 5.  Supported: 1,2,3,4,8,16}}
> +}

Sorry for the necromancy here, but I wonder if we should only make
this if LangOpts.OpenCL in on. Is there an initial intent for not to?

The rationale is that we have given users support for ext_vector_type
without OpenCL mode with arbitrary vectors lengths not defined in
"6.1.2 Built-In Vector Data Types", that said it makes sense to
support the component notation for those. I'm happy to fix it, I just
need to know if this covers some background I'm not aware of.

Thanks,

-- 
Bruno Cardoso Lopes
http://www.brunocardoso.cc
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r298369 - [OpenCL] Added diagnostic for checking length of vector

2017-03-21 Thread Egor Churaev via cfe-commits 
Author: echuraev
Date: Tue Mar 21 08:20:57 2017
New Revision: 298369

URL: http://llvm.org/viewvc/llvm-project?rev=298369&view=rev
Log:
[OpenCL] Added diagnostic for checking length of vector

Reviewers: Anastasia, cfe-commits

Reviewed By: Anastasia

Subscribers: bader, yaxunl

Differential Revision: https://reviews.llvm.org/D30937

Added:
cfe/trunk/test/SemaOpenCL/vector_swizzle_length.cl
Modified:
cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
cfe/trunk/lib/Sema/SemaExprMember.cpp

Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=298369&r1=298368&r2=298369&view=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Tue Mar 21 08:20:57 
2017
@@ -8236,6 +8236,8 @@ def err_opencl_ptrptr_kernel_param : Err
 def err_kernel_arg_address_space : Error<
   "pointer arguments to kernel functions must reside in '__global', "
   "'__constant' or '__local' address space">;
+def err_opencl_ext_vector_component_invalid_length : Error<
+  "vector component access has invalid length %0.  Supported: 1,2,3,4,8,16.">;
 def err_opencl_function_variable : Error<
   "%select{non-kernel function|function scope}0 variable cannot be declared in 
%1 address space">;
 def err_static_function_scope : Error<

Modified: cfe/trunk/lib/Sema/SemaExprMember.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExprMember.cpp?rev=298369&r1=298368&r2=298369&view=diff
==
--- cfe/trunk/lib/Sema/SemaExprMember.cpp (original)
+++ cfe/trunk/lib/Sema/SemaExprMember.cpp Tue Mar 21 08:20:57 2017
@@ -284,6 +284,14 @@ IsRGBA(char c) {
   }
 }
 
+// OpenCL v1.1, s6.1.7
+// The component swizzle length must be in accordance with the acceptable
+// vector sizes.
+static bool IsValidOpenCLComponentSwizzleLength(unsigned len)
+{
+  return (len >= 1 && len <= 4) || len == 8 || len == 16;
+}
+
 /// Check an ext-vector component access expression.
 ///
 /// VK should be set in advance to the value kind of the base
@@ -376,6 +384,19 @@ CheckExtVectorComponent(Sema &S, QualTyp
 }
   }
 
+  if (!HalvingSwizzle) {
+unsigned SwizzleLength = CompName->getLength();
+
+if (HexSwizzle)
+  SwizzleLength--;
+
+if (IsValidOpenCLComponentSwizzleLength(SwizzleLength) == false) {
+  S.Diag(OpLoc, diag::err_opencl_ext_vector_component_invalid_length)
+<< SwizzleLength << SourceRange(CompLoc);
+  return QualType();
+}
+  }
+
   // The component accessor looks fine - now we need to compute the actual 
type.
   // The vector type is implied by the component accessor. For example,
   // vec4.b is a float, vec4.xy is a vec2, vec4.rgb is a vec3, etc.

Added: cfe/trunk/test/SemaOpenCL/vector_swizzle_length.cl
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaOpenCL/vector_swizzle_length.cl?rev=298369&view=auto
==
--- cfe/trunk/test/SemaOpenCL/vector_swizzle_length.cl (added)
+++ cfe/trunk/test/SemaOpenCL/vector_swizzle_length.cl Tue Mar 21 08:20:57 2017
@@ -0,0 +1,10 @@
+// RUN: %clang_cc1 %s -verify -pedantic -fsyntax-only
+
+typedef float float8 __attribute__((ext_vector_type(8)));
+
+void foo() {
+float8 f2 = (float8)(0, 0, 0, 0, 0, 0, 0, 0);
+
+f2.s01234; // expected-error {{vector component access has invalid length 
5.  Supported: 1,2,3,4,8,16}}
+f2.xyzxy; // expected-error {{vector component access has invalid length 
5.  Supported: 1,2,3,4,8,16}}
+}


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