| Message ID | 1403045084.3788.20.camel@gnopaine |
|---|---|
| State | New |
| Headers | show |
On Tue, Jun 17, 2014 at 6:44 PM, BIll Schmidt <wschmidt@linux.vnet.ibm.com> wrote: > Hi, > > As described in https://gcc.gnu.org/bugzilla/show_bug.cgi?id=61542, a > new test case (gcc.dg/vect/vect-nop-move.c) was added in 4.9. This > exposes a bug on PowerPC little endian for extracting an element from a > V4SF value that goes back to 4.8. The following patch fixes the > problem. > > Tested on powerpc64le-unknown-linux-gnu with no regressions. Ok to > commit to trunk? I would also like to commit to 4.8 and 4.9 as soon as > possible to be picked up by the distros. This is okay everywhere. > I would also like to backport gcc.dg/vect/vect-nop-move.c to 4.8 to > provide regression coverage. You should ask Bernd and the RMs. Was the bug fix that prompted the new testcase backported to all targets? Thanks, David
Hi, On Wed, 18 Jun 2014 09:56:15, David Edelsohn wrote: > > On Tue, Jun 17, 2014 at 6:44 PM, BIll Schmidt > <wschmidt@linux.vnet.ibm.com> wrote: >> Hi, >> >> As described in https://gcc.gnu.org/bugzilla/show_bug.cgi?id=61542, a >> new test case (gcc.dg/vect/vect-nop-move.c) was added in 4.9. This >> exposes a bug on PowerPC little endian for extracting an element from a >> V4SF value that goes back to 4.8. The following patch fixes the >> problem. >> >> Tested on powerpc64le-unknown-linux-gnu with no regressions. Ok to >> commit to trunk? I would also like to commit to 4.8 and 4.9 as soon as >> possible to be picked up by the distros. > > This is okay everywhere. > >> I would also like to backport gcc.dg/vect/vect-nop-move.c to 4.8 to >> provide regression coverage. > > You should ask Bernd and the RMs. Was the bug fix that prompted the > new testcase backported to all targets? > > Thanks, David actually I only added the check_vect to that test case, but that exposed a bug on Solaris-9. See https://gcc.gnu.org/viewcvs/gcc?view=revision&revision=207668. That was in the -fdump-rtl-combine-details handling, where fprintf got a NULL value passed for %s, which ICEs on Solaris9. So if you backport that test case, be sure to check that one too. Originally the test case seems to check something for the aarch64-target. See https://gcc.gnu.org/viewcvs/gcc?view=revision&revision=205712. Obviously the patch in rtlanal.c (set_noop_p) was never backported to the 4.8 branch. Maybe Tejas who originally wrote that test case, can explain, if it makes sense to backport this fix too. Thanks Bernd.
Bernd, thanks. At this point I think I will avoid opening this can of worms and not worry about backporting the test case. Thanks, Bill On Wed, 2014-06-18 at 19:18 +0200, Bernd Edlinger wrote: > Hi, > > On Wed, 18 Jun 2014 09:56:15, David Edelsohn wrote: > > > > On Tue, Jun 17, 2014 at 6:44 PM, BIll Schmidt > > <wschmidt@linux.vnet.ibm.com> wrote: > >> Hi, > >> > >> As described in https://gcc.gnu.org/bugzilla/show_bug.cgi?id=61542, a > >> new test case (gcc.dg/vect/vect-nop-move.c) was added in 4.9. This > >> exposes a bug on PowerPC little endian for extracting an element from a > >> V4SF value that goes back to 4.8. The following patch fixes the > >> problem. > >> > >> Tested on powerpc64le-unknown-linux-gnu with no regressions. Ok to > >> commit to trunk? I would also like to commit to 4.8 and 4.9 as soon as > >> possible to be picked up by the distros. > > > > This is okay everywhere. > > > >> I would also like to backport gcc.dg/vect/vect-nop-move.c to 4.8 to > >> provide regression coverage. > > > > You should ask Bernd and the RMs. Was the bug fix that prompted the > > new testcase backported to all targets? > > > > Thanks, David > > actually I only added the check_vect to that test case, but that > exposed a bug on Solaris-9. > > See https://gcc.gnu.org/viewcvs/gcc?view=revision&revision=207668. > > That was in the -fdump-rtl-combine-details handling, where > fprintf got a NULL value passed for %s, which ICEs on Solaris9. > > So if you backport that test case, be sure to check that one too. > > Originally the test case seems to check something for the aarch64-target. > See https://gcc.gnu.org/viewcvs/gcc?view=revision&revision=205712. > > Obviously the patch in rtlanal.c (set_noop_p) was never backported to the 4.8 branch. > Maybe Tejas who originally wrote that test case, can explain, if it makes > sense to backport this fix too. > > > Thanks > Bernd. >
Bill Schmidt <wschmidt@linux.vnet.ibm.com> writes: > Bernd, thanks. At this point I think I will avoid opening this can of > worms and not worry about backporting the test case. Sorry for the late answer (catching up on a big backlog), but the testcase was originally added for an optimisation rather than a bug fix. It sounds like it tripped an ICE, so it should be safe to backport as a compile-only test with no dg-additional-options and no dg-finals. Thanks, Richard > > Thanks, > Bill > > On Wed, 2014-06-18 at 19:18 +0200, Bernd Edlinger wrote: >> Hi, >> >> On Wed, 18 Jun 2014 09:56:15, David Edelsohn wrote: >> > >> > On Tue, Jun 17, 2014 at 6:44 PM, BIll Schmidt >> > <wschmidt@linux.vnet.ibm.com> wrote: >> >> Hi, >> >> >> >> As described in https://gcc.gnu.org/bugzilla/show_bug.cgi?id=61542, a >> >> new test case (gcc.dg/vect/vect-nop-move.c) was added in 4.9. This >> >> exposes a bug on PowerPC little endian for extracting an element from a >> >> V4SF value that goes back to 4.8. The following patch fixes the >> >> problem. >> >> >> >> Tested on powerpc64le-unknown-linux-gnu with no regressions. Ok to >> >> commit to trunk? I would also like to commit to 4.8 and 4.9 as soon as >> >> possible to be picked up by the distros. >> > >> > This is okay everywhere. >> > >> >> I would also like to backport gcc.dg/vect/vect-nop-move.c to 4.8 to >> >> provide regression coverage. >> > >> > You should ask Bernd and the RMs. Was the bug fix that prompted the >> > new testcase backported to all targets? >> > >> > Thanks, David >> >> actually I only added the check_vect to that test case, but that >> exposed a bug on Solaris-9. >> >> See https://gcc.gnu.org/viewcvs/gcc?view=revision&revision=207668. >> >> That was in the -fdump-rtl-combine-details handling, where >> fprintf got a NULL value passed for %s, which ICEs on Solaris9. >> >> So if you backport that test case, be sure to check that one too. >> >> Originally the test case seems to check something for the aarch64-target. >> See https://gcc.gnu.org/viewcvs/gcc?view=revision&revision=205712. >> >> Obviously the patch in rtlanal.c (set_noop_p) was never backported to >> the 4.8 branch. >> Maybe Tejas who originally wrote that test case, can explain, if it makes >> sense to backport this fix too. >> >> >> Thanks >> Bernd. >>
Index: gcc/config/rs6000/vsx.md =================================================================== --- gcc/config/rs6000/vsx.md (revision 211741) +++ gcc/config/rs6000/vsx.md (working copy) @@ -1667,7 +1667,7 @@ { if (GET_CODE (op3) == SCRATCH) op3 = gen_reg_rtx (V4SFmode); - emit_insn (gen_vsx_xxsldwi_v4sf (op3, op1, op1, op2)); + emit_insn (gen_vsx_xxsldwi_v4sf (op3, op1, op1, GEN_INT (ele))); tmp = op3; } emit_insn (gen_vsx_xscvspdp_scalar2 (op0, tmp));