Sub-vector selection and CLang v5.0

classic Classic list List threaded Threaded
4 messages Options
Reply | Threaded
Open this post in threaded view
|

Sub-vector selection and CLang v5.0

Hans Wennborg via cfe-dev

Hi CFE-Devs,

 

I received a bug report for a breaking change between CLang v4.0 and v5.0 regarding sub-vectors that has been caused by:

 

http://llvm.org/viewvc/llvm-project?view=revision&revision=298369

 

The example is as follows:

 

  char16 v0, v1;

  ...

  v1.s0123456 = v0.s0123456; // Replace the 1st 7-elements of ‘v1’ with the 1st 7-elements of ‘v0’

 

which used to generate valid code, but which is now diagnosed with the following error:

 

  error: vector component access has invalid length 7. Supported: 1,2,3,4,8,16.

 

However, I don’t see why it should not be possible to replace any range of elements of a vector with a corresponding set of elements.  The “fix” in this case is to rewrite it as:

 

  v1.lo = (char8){v0.s0, v0.s1, v0.s2, v0.s3,

                  v0.s4, v0.s5, v0.s6, v1.s7};

 

but this also generates slightly more code (on our platform) than the alternative rewrite:

 

  v1 = (char16){v0.s0, v0.s1, v0.s2, v0.s3,

                v0.s4, v0.s5, v0.s6, v1.s7,

                v1.s8, v1.s9, v1.sa, v1.sb,

                v1.sc, v1.sd, v1.se, v1.sf};

 

In either case, the rewrite seems unnecessarily verbose compared to the original form.

 

Thanks,

 

            MartinO

 

--------------------------------------------------------------
Intel Research and Development Ireland Limited
Registered in Ireland
Registered Office: Collinstown Industrial Park, Leixlip, County Kildare
Registered Number: 308263

This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). Any review or distribution by others is strictly prohibited. If you are not the intended recipient, please contact the sender and delete all copies.


_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: Sub-vector selection and CLang v5.0

Hans Wennborg via cfe-dev
On 2/16/2018 1:11 AM, ORiordan, Martin via cfe-dev wrote:

Hi CFE-Devs,

 

I received a bug report for a breaking change between CLang v4.0 and v5.0 regarding sub-vectors that has been caused by:

 

http://llvm.org/viewvc/llvm-project?view=revision&revision=298369

 

The example is as follows:

 

  char16 v0, v1;

  ...

  v1.s0123456 = v0.s0123456; // Replace the 1st 7-elements of ‘v1’ with the 1st 7-elements of ‘v0’

 

which used to generate valid code, but which is now diagnosed with the following error:

 

  error: vector component access has invalid length 7. Supported: 1,2,3,4,8,16.

 

However, I don’t see why it should not be possible to replace any range of elements of a vector with a corresponding set of elements.


Vector swizzles are defined in the OpenCL standard, and https://www.khronos.org/registry/OpenCL/specs/opencl-2.2-cplusplus.html#builtin-vector-data-types says "vector-swizzle-selector shall have swizzle size of 1, 2, 3, 4, 8 or 16."

-Eli

-- 
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project

_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: Sub-vector selection and CLang v5.0

Hans Wennborg via cfe-dev

Thanks for the explanation Eli.

 

In this case the source language is C using the vector extensions – I chose ‘char16’ because it is familiar, but it could have been the ARM Neon form:

 

typedef char int8x16_t __attribute__((ext_vector_type(16)));

 

int8x16_t v0, v1;

...

v1.s0123456 = v0.s0123456; // Replace the 1st 7-elements of ‘v1’ with the 1st 7-elements of ‘v0’

 

However, though OpenCL has a hard semantic requirement for swizzle vector lengths, there is no corresponding restriction on extensions for other languages that do not formally specify such a semantic constraint.

 

In the machine vision [MV] space we use a lot of vectors that are 3, 5, 7, 11 and 13 elements in length (NxN matrices heavily used in convolutions).

 

LLVM itself has no difficulty handling swizzles with arbitrary sub-vector lengths, so I would like to suggest that this test is only enabled if the source language is OpenCL.  Our Myriad:SHAVE target implementation also has very carefully optimised code generation for these odd-element length swizzles because algorithms using them are so common in the MV space.

 

Replace:

if (IsValidOpenCLComponentSwizzleLength(SwizzleLength) == false) {

with:

if (S.getLangOpts().OpenCL && IsValidOpenCLComponentSwizzleLength(SwizzleLength) == false) {

 

The rest of the ‘Sema’ checking and the LLVR-IR produced are valid for arbitrary sub-vector lengths.

 

Thanks,

 

            MartinO

 

From: Friedman, Eli [mailto:[hidden email]]
Sent: Friday, February 16, 2018 9:36 PM
To: ORiordan, Martin <[hidden email]>; Clang Dev <[hidden email]>
Subject: Re: [cfe-dev] Sub-vector selection and CLang v5.0

 

On 2/16/2018 1:11 AM, ORiordan, Martin via cfe-dev wrote:

Hi CFE-Devs,

 

I received a bug report for a breaking change between CLang v4.0 and v5.0 regarding sub-vectors that has been caused by:

 

http://llvm.org/viewvc/llvm-project?view=revision&revision=298369

 

The example is as follows:

 

  char16 v0, v1;

  ...

  v1.s0123456 = v0.s0123456; // Replace the 1st 7-elements of ‘v1’ with the 1st 7-elements of ‘v0’

 

which used to generate valid code, but which is now diagnosed with the following error:

 

  error: vector component access has invalid length 7. Supported: 1,2,3,4,8,16.

 

However, I don’t see why it should not be possible to replace any range of elements of a vector with a corresponding set of elements.


Vector swizzles are defined in the OpenCL standard, and https://www.khronos.org/registry/OpenCL/specs/opencl-2.2-cplusplus.html#builtin-vector-data-types says "vector-swizzle-selector shall have swizzle size of 1, 2, 3, 4, 8 or 16."

-Eli


-- 
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project

--------------------------------------------------------------
Intel Research and Development Ireland Limited
Registered in Ireland
Registered Office: Collinstown Industrial Park, Leixlip, County Kildare
Registered Number: 308263

This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). Any review or distribution by others is strictly prohibited. If you are not the intended recipient, please contact the sender and delete all copies.


_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: Sub-vector selection and CLang v5.0

Hans Wennborg via cfe-dev

Hi again.

 

Sorry for the noise.  I have just realised that this has been resolved more or less as I suggested in the v6.0 sources – the diagnostic is now only generated if the source language is OpenCL.  My apologies for distracting people on something that has already been resolved – I should have checked v6 or head first, and thanks again Eli for your careful response.

 

            MartinO

 

From: cfe-dev [mailto:[hidden email]] On Behalf Of ORiordan, Martin via cfe-dev
Sent: Saturday, February 17, 2018 11:28 AM
To: Friedman, Eli <[hidden email]>; Clang Dev <[hidden email]>
Subject: Re: [cfe-dev] Sub-vector selection and CLang v5.0

 

Thanks for the explanation Eli.

 

In this case the source language is C using the vector extensions – I chose ‘char16’ because it is familiar, but it could have been the ARM Neon form:

 

typedef char int8x16_t __attribute__((ext_vector_type(16)));

 

int8x16_t v0, v1;

...

v1.s0123456 = v0.s0123456; // Replace the 1st 7-elements of ‘v1’ with the 1st 7-elements of ‘v0’

 

However, though OpenCL has a hard semantic requirement for swizzle vector lengths, there is no corresponding restriction on extensions for other languages that do not formally specify such a semantic constraint.

 

In the machine vision [MV] space we use a lot of vectors that are 3, 5, 7, 11 and 13 elements in length (NxN matrices heavily used in convolutions).

 

LLVM itself has no difficulty handling swizzles with arbitrary sub-vector lengths, so I would like to suggest that this test is only enabled if the source language is OpenCL.  Our Myriad:SHAVE target implementation also has very carefully optimised code generation for these odd-element length swizzles because algorithms using them are so common in the MV space.

 

Replace:

if (IsValidOpenCLComponentSwizzleLength(SwizzleLength) == false) {

with:

if (S.getLangOpts().OpenCL && IsValidOpenCLComponentSwizzleLength(SwizzleLength) == false) {

 

The rest of the ‘Sema’ checking and the LLVR-IR produced are valid for arbitrary sub-vector lengths.

 

Thanks,

 

            MartinO

 

From: Friedman, Eli [[hidden email]]
Sent: Friday, February 16, 2018 9:36 PM
To: ORiordan, Martin <[hidden email]>; Clang Dev <[hidden email]>
Subject: Re: [cfe-dev] Sub-vector selection and CLang v5.0

 

On 2/16/2018 1:11 AM, ORiordan, Martin via cfe-dev wrote:

Hi CFE-Devs,

 

I received a bug report for a breaking change between CLang v4.0 and v5.0 regarding sub-vectors that has been caused by:

 

http://llvm.org/viewvc/llvm-project?view=revision&revision=298369

 

The example is as follows:

 

  char16 v0, v1;

  ...

  v1.s0123456 = v0.s0123456; // Replace the 1st 7-elements of ‘v1’ with the 1st 7-elements of ‘v0’

 

which used to generate valid code, but which is now diagnosed with the following error:

 

  error: vector component access has invalid length 7. Supported: 1,2,3,4,8,16.

 

However, I don’t see why it should not be possible to replace any range of elements of a vector with a corresponding set of elements.


Vector swizzles are defined in the OpenCL standard, and https://www.khronos.org/registry/OpenCL/specs/opencl-2.2-cplusplus.html#builtin-vector-data-types says "vector-swizzle-selector shall have swizzle size of 1, 2, 3, 4, 8 or 16."

-Eli

-- 
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project

--------------------------------------------------------------
Intel Research and Development Ireland Limited
Registered in Ireland
Registered Office: Collinstown Industrial Park, Leixlip, County Kildare
Registered Number: 308263

This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). Any review or distribution by others is strictly prohibited. If you are not the intended recipient, please contact the sender and delete all copies.

--------------------------------------------------------------
Intel Research and Development Ireland Limited
Registered in Ireland
Registered Office: Collinstown Industrial Park, Leixlip, County Kildare
Registered Number: 308263

This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). Any review or distribution by others is strictly prohibited. If you are not the intended recipient, please contact the sender and delete all copies.


_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev