<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=iso-8859-1">
<style type="text/css" style="display:none;"><!-- P {margin-top:0;margin-bottom:0;} --></style>
</head>
<body dir="ltr">
<div id="divtagdefaultwrapper" style="font-size: 12pt; color: rgb(0, 0, 0); font-family: Calibri,Helvetica,sans-serif,"EmojiFont","Apple Color Emoji","Segoe UI Emoji",NotoColorEmoji,"Segoe UI Symbol","Android Emoji",EmojiSymbols;" dir="ltr">
<p><br>
</p>
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.<br>
<br>
Thanks,<br>
Anastasia<br>
<br>
<div style="color: rgb(0, 0, 0);">
<div>
<hr tabindex="-1" style="display:inline-block; width:98%">
<div id="x_divRplyFwdMsg" dir="ltr"><font style="font-size:11pt" color="#000000" face="Calibri, sans-serif"><b>From:</b> Bruno Cardoso Lopes <bruno.cardoso@gmail.com><br>
<b>Sent:</b> 11 October 2017 00:27<br>
<b>To:</b> Egor Churaev<br>
<b>Cc:</b> cfe-commits; Anastasia Stulova<br>
<b>Subject:</b> Re: r298369 - [OpenCL] Added diagnostic for checking length of vector</font>
<div> </div>
</div>
</div>
<font size="2"><span style="font-size:10pt;">
<div class="PlainText">Hi Egor,<br>
<br>
On Tue, Mar 21, 2017 at 6:20 AM, Egor Churaev via cfe-commits<br>
<cfe-commits@lists.llvm.org> wrote:<br>
> Author: echuraev<br>
> Date: Tue Mar 21 08:20:57 2017<br>
> New Revision: 298369<br>
><br>
> URL: <a href="http://llvm.org/viewvc/llvm-project?rev=298369&view=rev">http://llvm.org/viewvc/llvm-project?rev=298369&view=rev</a><br>
> Log:<br>
> [OpenCL] Added diagnostic for checking length of vector<br>
><br>
> Reviewers: Anastasia, cfe-commits<br>
><br>
> Reviewed By: Anastasia<br>
><br>
> Subscribers: bader, yaxunl<br>
><br>
> Differential Revision: <a href="https://reviews.llvm.org/D30937">https://reviews.llvm.org/D30937</a><br>
><br>
> Added:<br>
> cfe/trunk/test/SemaOpenCL/vector_swizzle_length.cl<br>
> Modified:<br>
> cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td<br>
> cfe/trunk/lib/Sema/SemaExprMember.cpp<br>
><br>
> Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=298369&r1=298368&r2=298369&view=diff">
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=298369&r1=298368&r2=298369&view=diff</a><br>
> ==============================================================================<br>
> --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)<br>
> +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Tue Mar 21 08:20:57 2017<br>
> @@ -8236,6 +8236,8 @@ def err_opencl_ptrptr_kernel_param : Err<br>
> def err_kernel_arg_address_space : Error<<br>
> "pointer arguments to kernel functions must reside in '__global', "<br>
> "'__constant' or '__local' address space">;<br>
> +def err_opencl_ext_vector_component_invalid_length : Error<<br>
> + "vector component access has invalid length %0. Supported: 1,2,3,4,8,16.">;<br>
> def err_opencl_function_variable : Error<<br>
> "%select{non-kernel function|function scope}0 variable cannot be declared in %1 address space">;<br>
> def err_static_function_scope : Error<<br>
><br>
> Modified: cfe/trunk/lib/Sema/SemaExprMember.cpp<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExprMember.cpp?rev=298369&r1=298368&r2=298369&view=diff">
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExprMember.cpp?rev=298369&r1=298368&r2=298369&view=diff</a><br>
> ==============================================================================<br>
> --- cfe/trunk/lib/Sema/SemaExprMember.cpp (original)<br>
> +++ cfe/trunk/lib/Sema/SemaExprMember.cpp Tue Mar 21 08:20:57 2017<br>
> @@ -284,6 +284,14 @@ IsRGBA(char c) {<br>
> }<br>
> }<br>
><br>
> +// OpenCL v1.1, s6.1.7<br>
> +// The component swizzle length must be in accordance with the acceptable<br>
> +// vector sizes.<br>
> +static bool IsValidOpenCLComponentSwizzleLength(unsigned len)<br>
> +{<br>
> + return (len >= 1 && len <= 4) || len == 8 || len == 16;<br>
> +}<br>
> +<br>
> /// Check an ext-vector component access expression.<br>
> ///<br>
> /// VK should be set in advance to the value kind of the base<br>
> @@ -376,6 +384,19 @@ CheckExtVectorComponent(Sema &S, QualTyp<br>
> }<br>
> }<br>
><br>
> + if (!HalvingSwizzle) {<br>
> + unsigned SwizzleLength = CompName->getLength();<br>
> +<br>
> + if (HexSwizzle)<br>
> + SwizzleLength--;<br>
> +<br>
> + if (IsValidOpenCLComponentSwizzleLength(SwizzleLength) == false) {<br>
> + S.Diag(OpLoc, diag::err_opencl_ext_vector_component_invalid_length)<br>
> + << SwizzleLength << SourceRange(CompLoc);<br>
> + return QualType();<br>
> + }<br>
> + }<br>
> +<br>
> // The component accessor looks fine - now we need to compute the actual type.<br>
> // The vector type is implied by the component accessor. For example,<br>
> // vec4.b is a float, vec4.xy is a vec2, vec4.rgb is a vec3, etc.<br>
><br>
> Added: cfe/trunk/test/SemaOpenCL/vector_swizzle_length.cl<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaOpenCL/vector_swizzle_length.cl?rev=298369&view=auto">
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaOpenCL/vector_swizzle_length.cl?rev=298369&view=auto</a><br>
> ==============================================================================<br>
> --- cfe/trunk/test/SemaOpenCL/vector_swizzle_length.cl (added)<br>
> +++ cfe/trunk/test/SemaOpenCL/vector_swizzle_length.cl Tue Mar 21 08:20:57 2017<br>
> @@ -0,0 +1,10 @@<br>
> +// RUN: %clang_cc1 %s -verify -pedantic -fsyntax-only<br>
> +<br>
> +typedef float float8 __attribute__((ext_vector_type(8)));<br>
> +<br>
> +void foo() {<br>
> + float8 f2 = (float8)(0, 0, 0, 0, 0, 0, 0, 0);<br>
> +<br>
> + f2.s01234; // expected-error {{vector component access has invalid length 5. Supported: 1,2,3,4,8,16}}<br>
> + f2.xyzxy; // expected-error {{vector component access has invalid length 5. Supported: 1,2,3,4,8,16}}<br>
> +}<br>
<br>
Sorry for the necromancy here, but I wonder if we should only make<br>
this if LangOpts.OpenCL in on. Is there an initial intent for not to?<br>
<br>
The rationale is that we have given users support for ext_vector_type<br>
without OpenCL mode with arbitrary vectors lengths not defined in<br>
"6.1.2 Built-In Vector Data Types", that said it makes sense to<br>
support the component notation for those. I'm happy to fix it, I just<br>
need to know if this covers some background I'm not aware of.<br>
<br>
Thanks,<br>
<br>
-- <br>
Bruno Cardoso Lopes<br>
<a href="http://www.brunocardoso.cc">http://www.brunocardoso.cc</a><br>
</div>
</span></font></div>
</div>
</body>
</html>