[ARM] PR 47553: vld1q_lane_u8 & co. don't accept lanes >= 8

[ARM] PR 47553: vld1q_lane_u8 & co. don't accept lanes >= 8

[Richard Sandiford]

The v{ld,st}1q_lane_{u,s,p}8 intrinsic functions are supposed to take a range
between 0 and 15.  The function is then converted to a vld1 or vst1 of one half
of the quad value.  The problem is that the lane predicate doesn't account for
this, and only accepts the 0..7 range that are supported by the individual
vld1 and vst1.

The current code only does the "real" size-dependent range check at
output time.  That isn't ideal, but there's a separate bug (40884)
about it.

Tested on arm-linux-gnueabi (-marm and -mthumb).  I don't think this
is a regression, so: OK to install once 4.7 is open?

Richard


gcc/
	* config/arm/predicates.md (neon_lane_number): Accept 0..15.

gcc/testsuite/
	* gcc.target/arm/neon-vld-1.c: New test.

Index: gcc/config/arm/predicates.md
===================================================================
--- gcc/config/arm/predicates.md	2011-01-31 13:09:19.000000000 +0000
+++ gcc/config/arm/predicates.md	2011-01-31 13:12:41.000000000 +0000
@@ -610,7 +610,7 @@ (define_predicate "neon_inv_logic_op2"
 ;; TODO: We could check lane numbers more precisely based on the mode.
 (define_predicate "neon_lane_number"
   (and (match_code "const_int")
-       (match_test "INTVAL (op) >= 0 && INTVAL (op) <= 7")))
+       (match_test "INTVAL (op) >= 0 && INTVAL (op) <= 15")))
 ;; Predicates for named expanders that overlap multiple ISAs.
 
 (define_predicate "cmpdi_operand"
Index: gcc/testsuite/gcc.target/arm/neon-vld-1.c
===================================================================
--- /dev/null	2011-01-26 10:43:14.268819722 +0000
+++ gcc/testsuite/gcc.target/arm/neon-vld-1.c	2011-01-31 13:16:10.000000000 +0000
@@ -0,0 +1,13 @@
+/* { dg-do compile } */
+/* { dg-require-effective-target arm_neon_ok } */
+/* { dg-options "-O1" } */
+/* { dg-add-options arm_neon } */
+
+#include <arm_neon.h>
+
+uint8x16_t
+foo (uint8_t *a, uint8x16_t b)
+{
+  vst1q_lane_u8 (a, b, 14);
+  return vld1q_lane_u8 (a + 0x100, b, 15);
+}

Re: [ARM] PR 47553: vld1q_lane_u8 & co. don't accept lanes >= 8

[Ramana Radhakrishnan]

On 31/01/11 13:27, Richard Sandiford wrote:
> The v{ld,st}1q_lane_{u,s,p}8 intrinsic functions are supposed to take a range
> between 0 and 15.  The function is then converted to a vld1 or vst1 of one half
> of the quad value.  The problem is that the lane predicate doesn't account for
> this, and only accepts the 0..7 range that are supported by the individual
> vld1 and vst1.
>
> The current code only does the "real" size-dependent range check at
> output time.  That isn't ideal, but there's a separate bug (40884)
> about it.
>
> Tested on arm-linux-gnueabi (-marm and -mthumb).  I don't think this
> is a regression, so: OK to install once 4.7 is open?

OK for trunk.

I am tempted to consider this one for the other release branches since 
this is correcting a test in the interface for an intrinsic which has 
been wrong for a long time but would like the opinion of the other 
maintainers about this.

cheers
Ramana

>
> Richard
>
>
> gcc/
> 	* config/arm/predicates.md (neon_lane_number): Accept 0..15.
>
> gcc/testsuite/
> 	* gcc.target/arm/neon-vld-1.c: New test.
>
> Index: gcc/config/arm/predicates.md
> ===================================================================
> --- gcc/config/arm/predicates.md	2011-01-31 13:09:19.000000000 +0000
> +++ gcc/config/arm/predicates.md	2011-01-31 13:12:41.000000000 +0000
> @@ -610,7 +610,7 @@ (define_predicate "neon_inv_logic_op2"
>   ;; TODO: We could check lane numbers more precisely based on the mode.
>   (define_predicate "neon_lane_number"
>     (and (match_code "const_int")
> -       (match_test "INTVAL (op)>= 0&&  INTVAL (op)<= 7")))
> +       (match_test "INTVAL (op)>= 0&&  INTVAL (op)<= 15")))



>   ;; Predicates for named expanders that overlap multiple ISAs.
>
>   (define_predicate "cmpdi_operand"
> Index: gcc/testsuite/gcc.target/arm/neon-vld-1.c
> ===================================================================
> --- /dev/null	2011-01-26 10:43:14.268819722 +0000
> +++ gcc/testsuite/gcc.target/arm/neon-vld-1.c	2011-01-31 13:16:10.000000000 +0000
> @@ -0,0 +1,13 @@
> +/* { dg-do compile } */
> +/* { dg-require-effective-target arm_neon_ok } */
> +/* { dg-options "-O1" } */
> +/* { dg-add-options arm_neon } */
> +
> +#include<arm_neon.h>
> +
> +uint8x16_t
> +foo (uint8_t *a, uint8x16_t b)
> +{
> +  vst1q_lane_u8 (a, b, 14);
> +  return vld1q_lane_u8 (a + 0x100, b, 15);
> +}

Re: [ARM] PR 47553: vld1q_lane_u8 & co. don't accept lanes >= 8

[Nick Clifton]

Hi Ramana,

> I am tempted to consider this one for the other release branches since
> this is correcting a test in the interface for an intrinsic which has
> been wrong for a long time but would like the opinion of the other
> maintainers about this.

I think that I would agree with you here.  Certainly I would not object 
if you wanted to apply the patch to the 4.5 branch.

Cheers
   Nick

Re: [ARM] PR 47553: vld1q_lane_u8 & co. don't accept lanes >= 8

[Richard Sandiford]

Nick Clifton <nickc@redhat.com> writes:
> Hi Ramana,
>
>> I am tempted to consider this one for the other release branches since
>> this is correcting a test in the interface for an intrinsic which has
>> been wrong for a long time but would like the opinion of the other
>> maintainers about this.
>
> I think that I would agree with you here.  Certainly I would not object 
> if you wanted to apply the patch to the 4.5 branch.

Thanks guys.  I've now applied the patch to 4.6 and 4.5 after checking
with the RMs on IRC.

Richard