* [PATCH, ARM] Fix NEON vset_lane for D registers
@ 2011-05-03 12:49 Julian Brown
2011-05-03 13:59 ` Ramana Radhakrishnan
2011-05-03 14:50 ` Richard Earnshaw
0 siblings, 2 replies; 6+ messages in thread
From: Julian Brown @ 2011-05-03 12:49 UTC (permalink / raw)
To: gcc-patches
[-- Attachment #1: Type: text/plain, Size: 328 bytes --]
Hi,
This patch fixes vset_lane intrinsic variants for D-register sized
variables. A typo meant that the wrong lane would be set in many
circumstances.
Tested manually only. OK to apply?
Thanks,
Julian
ChangeLog
gcc/
* config/arm/neon.md (vec_set<mode>_internal): Fix misplaced
parenthesis in D-register case.
[-- Attachment #2: vset_lane-fix-fsf-1.diff --]
[-- Type: text/x-patch, Size: 494 bytes --]
Index: gcc/config/arm/neon.md
===================================================================
--- gcc/config/arm/neon.md (revision 173299)
+++ gcc/config/arm/neon.md (working copy)
@@ -426,7 +426,7 @@
(match_operand:SI 2 "immediate_operand" "i")))]
"TARGET_NEON"
{
- int elt = ffs ((int) INTVAL (operands[2]) - 1);
+ int elt = ffs ((int) INTVAL (operands[2])) - 1;
if (BYTES_BIG_ENDIAN)
elt = GET_MODE_NUNITS (<MODE>mode) - 1 - elt;
operands[2] = GEN_INT (elt);
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH, ARM] Fix NEON vset_lane for D registers
2011-05-03 12:49 [PATCH, ARM] Fix NEON vset_lane for D registers Julian Brown
@ 2011-05-03 13:59 ` Ramana Radhakrishnan
2011-05-03 14:50 ` Richard Earnshaw
1 sibling, 0 replies; 6+ messages in thread
From: Ramana Radhakrishnan @ 2011-05-03 13:59 UTC (permalink / raw)
To: Julian Brown; +Cc: gcc-patches
On 03/05/11 13:49, Julian Brown wrote:
> Hi,
>
> This patch fixes vset_lane intrinsic variants for D-register sized
> variables. A typo meant that the wrong lane would be set in many
> circumstances.
>
> Tested manually only. OK to apply?
Ok - yes this looks almost obvious but please do a sanity regression run.
Thanks,
Ramana
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH, ARM] Fix NEON vset_lane for D registers
2011-05-03 12:49 [PATCH, ARM] Fix NEON vset_lane for D registers Julian Brown
2011-05-03 13:59 ` Ramana Radhakrishnan
@ 2011-05-03 14:50 ` Richard Earnshaw
2011-05-03 15:19 ` Joseph S. Myers
2011-05-05 12:15 ` Julian Brown
1 sibling, 2 replies; 6+ messages in thread
From: Richard Earnshaw @ 2011-05-03 14:50 UTC (permalink / raw)
To: Julian Brown; +Cc: gcc-patches
On Tue, 2011-05-03 at 13:49 +0100, Julian Brown wrote:
> Hi,
>
> This patch fixes vset_lane intrinsic variants for D-register sized
> variables. A typo meant that the wrong lane would be set in many
> circumstances.
>
> Tested manually only. OK to apply?
>
> Thanks,
>
> Julian
>
> ChangeLog
>
> gcc/
> * config/arm/neon.md (vec_set<mode>_internal): Fix misplaced
> parenthesis in D-register case.
Presumably this is a silent 'wrong-code' bug. If so, what about
released compilers?
R.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH, ARM] Fix NEON vset_lane for D registers
2011-05-03 14:50 ` Richard Earnshaw
@ 2011-05-03 15:19 ` Joseph S. Myers
2011-05-05 12:15 ` Julian Brown
1 sibling, 0 replies; 6+ messages in thread
From: Joseph S. Myers @ 2011-05-03 15:19 UTC (permalink / raw)
To: Richard Earnshaw; +Cc: Julian Brown, gcc-patches
On Tue, 3 May 2011, Richard Earnshaw wrote:
> > gcc/
> > * config/arm/neon.md (vec_set<mode>_internal): Fix misplaced
> > parenthesis in D-register case.
>
> Presumably this is a silent 'wrong-code' bug. If so, what about
> released compilers?
And what about an execution testcase that fails before and passes after
the patch? Is it hard to add one for some reason?
--
Joseph S. Myers
joseph@codesourcery.com
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH, ARM] Fix NEON vset_lane for D registers
2011-05-03 14:50 ` Richard Earnshaw
2011-05-03 15:19 ` Joseph S. Myers
@ 2011-05-05 12:15 ` Julian Brown
2011-05-05 12:55 ` Richard Earnshaw
1 sibling, 1 reply; 6+ messages in thread
From: Julian Brown @ 2011-05-05 12:15 UTC (permalink / raw)
To: Richard Earnshaw; +Cc: gcc-patches, Ramana Radhakrishnan, Joseph S. Myers
[-- Attachment #1: Type: text/plain, Size: 1352 bytes --]
On Tue, 03 May 2011 15:49:38 +0100
Richard Earnshaw <rearnsha@arm.com> wrote:
>
> On Tue, 2011-05-03 at 13:49 +0100, Julian Brown wrote:
> > Hi,
> >
> > This patch fixes vset_lane intrinsic variants for D-register sized
> > variables. A typo meant that the wrong lane would be set in many
> > circumstances.
> >
> > Tested manually only. OK to apply?
> >
> > Thanks,
> >
> > Julian
> >
> > ChangeLog
> >
> > gcc/
> > * config/arm/neon.md (vec_set<mode>_internal): Fix misplaced
> > parenthesis in D-register case.
>
> Presumably this is a silent 'wrong-code' bug. If so, what about
> released compilers?
Yes, this is a silent wrong-code bug. It affects branches back to at
gcc-4.4-branch at least: the patch will apply trivially to those, if
deemed appropriate (I think it's obvious enough to be risk-free).
Joseph wrote:
> And what about an execution testcase that fails before and passes
> after the patch? Is it hard to add one for some reason?
I've added a testcase, and also done a regression run at Ramana's
request, which doesn't show up anything untoward.
So: OK to apply to trunk? Other branches? (Which?)
Thanks,
Julian
ChangeLog
gcc/
* config/arm/neon.md (vec_set<mode>_internal): Fix misplaced
parenthesis in D-register case.
gcc/testsuite/
* gcc.target/arm/neon-vset_lanes8.c: New test.
[-- Attachment #2: vset_lane-fix-fsf-2.diff --]
[-- Type: text/x-patch, Size: 1232 bytes --]
Index: gcc/testsuite/gcc.target/arm/neon-vset_lanes8.c
===================================================================
--- gcc/testsuite/gcc.target/arm/neon-vset_lanes8.c (revision 0)
+++ gcc/testsuite/gcc.target/arm/neon-vset_lanes8.c (revision 0)
@@ -0,0 +1,21 @@
+/* Test the `vset_lane_s8' ARM Neon intrinsic. */
+
+/* { dg-do run } */
+/* { dg-require-effective-target arm_neon_hw } */
+/* { dg-options "-O0" } */
+/* { dg-add-options arm_neon } */
+
+#include "arm_neon.h"
+#include <stdlib.h>
+#include <string.h>
+
+int8x8_t x = { 1, 2, 3, 4, 5, 6, 7, 8 };
+int8x8_t y = { 1, 2, 3, 16, 5, 6, 7, 8 };
+
+int main (void)
+{
+ x = vset_lane_s8 (16, x, 3);
+ if (memcmp (&x, &y, sizeof (x)) != 0)
+ abort();
+ return 0;
+}
Index: gcc/config/arm/neon.md
===================================================================
--- gcc/config/arm/neon.md (revision 173299)
+++ gcc/config/arm/neon.md (working copy)
@@ -426,7 +426,7 @@
(match_operand:SI 2 "immediate_operand" "i")))]
"TARGET_NEON"
{
- int elt = ffs ((int) INTVAL (operands[2]) - 1);
+ int elt = ffs ((int) INTVAL (operands[2])) - 1;
if (BYTES_BIG_ENDIAN)
elt = GET_MODE_NUNITS (<MODE>mode) - 1 - elt;
operands[2] = GEN_INT (elt);
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH, ARM] Fix NEON vset_lane for D registers
2011-05-05 12:15 ` Julian Brown
@ 2011-05-05 12:55 ` Richard Earnshaw
0 siblings, 0 replies; 6+ messages in thread
From: Richard Earnshaw @ 2011-05-05 12:55 UTC (permalink / raw)
To: Julian Brown; +Cc: gcc-patches, Ramana Radhakrishnan, Joseph S. Myers
On Thu, 2011-05-05 at 13:08 +0100, Julian Brown wrote:
> On Tue, 03 May 2011 15:49:38 +0100
> Richard Earnshaw <rearnsha@arm.com> wrote:
>
> >
> > On Tue, 2011-05-03 at 13:49 +0100, Julian Brown wrote:
> > > Hi,
> > >
> > > This patch fixes vset_lane intrinsic variants for D-register sized
> > > variables. A typo meant that the wrong lane would be set in many
> > > circumstances.
> > >
> > > Tested manually only. OK to apply?
> > >
> > > Thanks,
> > >
> > > Julian
> > >
> > > ChangeLog
> > >
> > > gcc/
> > > * config/arm/neon.md (vec_set<mode>_internal): Fix misplaced
> > > parenthesis in D-register case.
> >
> > Presumably this is a silent 'wrong-code' bug. If so, what about
> > released compilers?
>
> Yes, this is a silent wrong-code bug. It affects branches back to at
> gcc-4.4-branch at least: the patch will apply trivially to those, if
> deemed appropriate (I think it's obvious enough to be risk-free).
>
> Joseph wrote:
>
> > And what about an execution testcase that fails before and passes
> > after the patch? Is it hard to add one for some reason?
>
> I've added a testcase, and also done a regression run at Ramana's
> request, which doesn't show up anything untoward.
>
> So: OK to apply to trunk? Other branches? (Which?)
>
Yes, and all open release branches.
R.
> Thanks,
>
> Julian
>
> ChangeLog
>
> gcc/
> * config/arm/neon.md (vec_set<mode>_internal): Fix misplaced
> parenthesis in D-register case.
>
> gcc/testsuite/
> * gcc.target/arm/neon-vset_lanes8.c: New test.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2011-05-05 12:38 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-05-03 12:49 [PATCH, ARM] Fix NEON vset_lane for D registers Julian Brown
2011-05-03 13:59 ` Ramana Radhakrishnan
2011-05-03 14:50 ` Richard Earnshaw
2011-05-03 15:19 ` Joseph S. Myers
2011-05-05 12:15 ` Julian Brown
2011-05-05 12:55 ` Richard Earnshaw
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).