Merge lp:~linaro-toolchain-dev/gcc-linaro/neon-shifts-4.7 into lp:gcc-linaro/4.7

Proposed by Andrew Stubbs
Status: Rejected
Rejected by: Andrew Stubbs
Proposed branch: lp:~linaro-toolchain-dev/gcc-linaro/neon-shifts-4.7
Merge into: lp:gcc-linaro/4.7
Prerequisite: lp:~ams-codesourcery/gcc-linaro/arm-64-bit-shifts-4.7
To merge this branch: bzr merge lp:~linaro-toolchain-dev/gcc-linaro/neon-shifts-4.7
Reviewer Review Type Date Requested Status
Michael Hope Needs Fixing
Review via email: mp+99530@code.launchpad.net

This proposal supersedes a proposal from 2012-03-09.

Description of the change

Implement DImode shifts in NEON.

Patch posted upstream here:
http://gcc.gnu.org/ml/gcc-patches/2012-02/msg01207.html

UPDATE 20120309: Fixed bug with negative shifts. No update upstream yet.
UPDATE 20120327: Fixed Bootstrap failure. No update upstream yet.

To post a comment you must log in.
Revision history for this message
Michael Hope (michaelh1) wrote : Posted in a previous version of this proposal

cbuild has taken a snapshot of this branch at r114969 and queued it for build.

The diff against the ancestor r114965 is available at:
 http://builds.linaro.org/toolchain/snapshots/gcc-linaro-4.7+bzr114969~ams-codesourcery~neon-shifts-4.7.diff

and will be built on the following builders:
 a9-builder armv5-builder i686 x86_64

You can track the build queue at:
 http://ex.seabright.co.nz/helpers/scheduler

cbuild-snapshot: gcc-linaro-4.7+bzr114969~ams-codesourcery~neon-shifts-4.7
cbuild-ancestor: lp:gcc-linaro/4.7+bzr114965
cbuild-state: check

Revision history for this message
Michael Hope (michaelh1) wrote : Posted in a previous version of this proposal

cbuild had trouble building this on armv7l-natty-cbuild259-tcpanda02-cortexa9r1.

See the *failed.txt logs under the build results at:
 http://ex.seabright.co.nz/build/gcc-linaro-4.7+bzr114969~ams-codesourcery~neon-shifts-4.7/logs/armv7l-natty-cbuild259-tcpanda02-cortexa9r1

The test suite results were not checked.

cbuild-checked: armv7l-natty-cbuild259-tcpanda02-cortexa9r1

review: Needs Fixing
Revision history for this message
Michael Hope (michaelh1) wrote : Posted in a previous version of this proposal
Revision history for this message
Michael Hope (michaelh1) wrote : Posted in a previous version of this proposal

cbuild successfully built this on armv7l-natty-cbuild259-tcpanda04-armv5r2.

The build results are available at:
 http://ex.seabright.co.nz/build/gcc-linaro-4.7+bzr114969~ams-codesourcery~neon-shifts-4.7/logs/armv7l-natty-cbuild259-tcpanda04-armv5r2

+UNSUPPORTED: gcc.target/arm/thumb-16bit-ops.c
+UNSUPPORTED: gcc.target/arm/thumb-ifcvt.c

The full diff is at:
 http://ex.seabright.co.nz/build/gcc-linaro-4.7+bzr114969~ams-codesourcery~neon-shifts-4.7/logs/armv7l-natty-cbuild259-tcpanda04-armv5r2/testsuite-diff.txt

The full testsuite results are at:
 http://ex.seabright.co.nz/build/gcc-linaro-4.7+bzr114969~ams-codesourcery~neon-shifts-4.7/logs/armv7l-natty-cbuild259-tcpanda04-armv5r2/gcc-testsuite.txt

cbuild-checked: armv7l-natty-cbuild259-tcpanda04-armv5r2

Revision history for this message
Michael Hope (michaelh1) wrote : Posted in a previous version of this proposal
Revision history for this message
Michael Hope (michaelh1) wrote : Posted in a previous version of this proposal

cbuild has taken a snapshot of this branch at r114970 and queued it for build.

The diff against the ancestor r114965 is available at:
 http://builds.linaro.org/toolchain/snapshots/gcc-linaro-4.7+bzr114970~ams-codesourcery~neon-shifts-4.7.diff

and will be built on the following builders:
 a9-builder armv5-builder i686 x86_64

You can track the build queue at:
 http://ex.seabright.co.nz/helpers/scheduler

cbuild-snapshot: gcc-linaro-4.7+bzr114970~ams-codesourcery~neon-shifts-4.7
cbuild-ancestor: lp:gcc-linaro/4.7+bzr114965
cbuild-state: check

Revision history for this message
Michael Hope (michaelh1) wrote : Posted in a previous version of this proposal
Revision history for this message
Michael Hope (michaelh1) wrote : Posted in a previous version of this proposal
Revision history for this message
Michael Hope (michaelh1) wrote : Posted in a previous version of this proposal

cbuild had trouble building this on armv7l-natty-cbuild264-tcpanda02-cortexa9r1.

See the *failed.txt logs under the build results at:
 http://ex.seabright.co.nz/build/gcc-linaro-4.7+bzr114970~ams-codesourcery~neon-shifts-4.7/logs/armv7l-natty-cbuild264-tcpanda02-cortexa9r1

The test suite results were not checked.

cbuild-checked: armv7l-natty-cbuild264-tcpanda02-cortexa9r1

review: Needs Fixing
Revision history for this message
Michael Hope (michaelh1) wrote : Posted in a previous version of this proposal

cbuild successfully built this on armv7l-natty-cbuild264-tcpanda04-armv5r2.

The build results are available at:
 http://ex.seabright.co.nz/build/gcc-linaro-4.7+bzr114970~ams-codesourcery~neon-shifts-4.7/logs/armv7l-natty-cbuild264-tcpanda04-armv5r2

+UNSUPPORTED: gcc.target/arm/thumb-16bit-ops.c
+UNSUPPORTED: gcc.target/arm/thumb-ifcvt.c

The full diff is at:
 http://ex.seabright.co.nz/build/gcc-linaro-4.7+bzr114970~ams-codesourcery~neon-shifts-4.7/logs/armv7l-natty-cbuild264-tcpanda04-armv5r2/testsuite-diff.txt

The full testsuite results are at:
 http://ex.seabright.co.nz/build/gcc-linaro-4.7+bzr114970~ams-codesourcery~neon-shifts-4.7/logs/armv7l-natty-cbuild264-tcpanda04-armv5r2/gcc-testsuite.txt

cbuild-checked: armv7l-natty-cbuild264-tcpanda04-armv5r2

Revision history for this message
Michael Hope (michaelh1) wrote :

cbuild has taken a snapshot of this branch at r114971 and queued it for build.

The diff against the ancestor r114965 is available at:
 http://builds.linaro.org/toolchain/snapshots/gcc-linaro-4.7+bzr114971~ams-codesourcery~neon-shifts-4.7.diff

and will be built on the following builders:
 a9-builder armv5-builder i686 x86_64

You can track the build queue at:
 http://ex.seabright.co.nz/helpers/scheduler

cbuild-snapshot: gcc-linaro-4.7+bzr114971~ams-codesourcery~neon-shifts-4.7
cbuild-ancestor: lp:gcc-linaro/4.7+bzr114965
cbuild-state: check

Revision history for this message
Michael Hope (michaelh1) wrote :
Revision history for this message
Michael Hope (michaelh1) wrote :
Revision history for this message
Michael Hope (michaelh1) wrote :
Revision history for this message
Michael Hope (michaelh1) wrote :
Revision history for this message
Michael Hope (michaelh1) wrote :

cbuild successfully built this on armv7l-natty-cbuild280-ursa1-armv5r2.

The build results are available at:
 http://ex.seabright.co.nz/build/gcc-linaro-4.7+bzr114971~ams-codesourcery~neon-shifts-4.7/logs/armv7l-natty-cbuild280-ursa1-armv5r2

+UNSUPPORTED: gcc.target/arm/thumb-16bit-ops.c
+UNSUPPORTED: gcc.target/arm/thumb-ifcvt.c

The full diff is at:
 http://ex.seabright.co.nz/build/gcc-linaro-4.7+bzr114971~ams-codesourcery~neon-shifts-4.7/logs/armv7l-natty-cbuild280-ursa1-armv5r2/testsuite-diff.txt

The full testsuite results are at:
 http://ex.seabright.co.nz/build/gcc-linaro-4.7+bzr114971~ams-codesourcery~neon-shifts-4.7/logs/armv7l-natty-cbuild280-ursa1-armv5r2/gcc-testsuite.txt

cbuild-checked: armv7l-natty-cbuild280-ursa1-armv5r2

Revision history for this message
Michael Hope (michaelh1) wrote :

cbuild successfully built this on armv7l-natty-cbuild280-tcpanda06-cortexa9r1.

The build results are available at:
 http://ex.seabright.co.nz/build/gcc-linaro-4.7+bzr114971~ams-codesourcery~neon-shifts-4.7/logs/armv7l-natty-cbuild280-tcpanda06-cortexa9r1

-PASS: gcc.dg/di-sync-multithread.c (test for excess errors)
-PASS: gcc.dg/di-sync-multithread.c execution test
+FAIL: gcc.dg/di-sync-multithread.c (test for excess errors)
+UNRESOLVED: gcc.dg/di-sync-multithread.c compilation failed to produce executable
-FAIL: gcc.target/arm/combine-movs.c scan-assembler movs\tr[0-9]
+PASS: gcc.target/arm/combine-movs.c scan-assembler movs\tr[0-9]
+PASS: gcc.target/arm/thumb-16bit-ops.c (test for excess errors)
+PASS: gcc.target/arm/thumb-16bit-ops.c scan-assembler add\tr0, r0, #256
+PASS: gcc.target/arm/thumb-16bit-ops.c scan-assembler add\tr0, r1, #8
+PASS: gcc.target/arm/thumb-16bit-ops.c scan-assembler adds\tr0, r0, #255
+PASS: gcc.target/arm/thumb-16bit-ops.c scan-assembler adds\tr0, r0, r1
+PASS: gcc.target/arm/thumb-16bit-ops.c scan-assembler adds\tr0, r1, #7
+PASS: gcc.target/arm/thumb-16bit-ops.c scan-assembler asr\tr0, r1, r2
+PASS: gcc.target/arm/thumb-16bit-ops.c scan-assembler asrs\tr0, r0, r1
+PASS: gcc.target/arm/thumb-16bit-ops.c scan-assembler asrs\tr0, r1, #15
+PASS: gcc.target/arm/thumb-16bit-ops.c scan-assembler lsl\tr0, r1, r2
+PASS: gcc.target/arm/thumb-16bit-ops.c scan-assembler lsls\tr0, r0, r1
+PASS: gcc.target/arm/thumb-16bit-ops.c scan-assembler lsls\tr0, r1, #15
+PASS: gcc.target/arm/thumb-16bit-ops.c scan-assembler lsr\tr0, r1, r2
+PASS: gcc.target/arm/thumb-16bit-ops.c scan-assembler lsrs\tr0, r0, r1
+PASS: gcc.target/arm/thumb-16bit-ops.c scan-assembler lsrs\tr0, r1, #15
+PASS: gcc.target/arm/thumb-16bit-ops.c scan-assembler mov\tr0, #256
+PASS: gcc.target/arm/thumb-16bit-ops.c scan-assembler mov\tr0, r1
+PASS: gcc.target/arm/thumb-16bit-ops.c scan-assembler movs\tr0, #255
+PASS: gcc.target/arm/thumb-16bit-ops.c scan-assembler muls\tr0, r1, r0
+PASS: gcc.target/arm/thumb-16bit-ops.c scan-assembler mvns\tr0, r1
+PASS: gcc.target/arm/thumb-16bit-ops.c scan-assembler negs\tr0, r1
+PASS: gcc.target/arm/thumb-ifcvt.c (test for excess errors)
+PASS: gcc.target/arm/thumb-ifcvt.c scan-assembler asrne
+PASS: gcc.target/arm/thumb-ifcvt.c scan-assembler lslne

The full diff is at:
 http://ex.seabright.co.nz/build/gcc-linaro-4.7+bzr114971~ams-codesourcery~neon-shifts-4.7/logs/armv7l-natty-cbuild280-tcpanda06-cortexa9r1/testsuite-diff.txt

The full testsuite results are at:
 http://ex.seabright.co.nz/build/gcc-linaro-4.7+bzr114971~ams-codesourcery~neon-shifts-4.7/logs/armv7l-natty-cbuild280-tcpanda06-cortexa9r1/gcc-testsuite.txt

cbuild-checked: armv7l-natty-cbuild280-tcpanda06-cortexa9r1

Revision history for this message
Michael Hope (michaelh1) wrote :

The extra failure is:

spawn /scratch/cbuild/slave/slaves/tcpanda06/gcc-linaro-4.7+bzr114971~ams-codesourcery~neon-shifts-4.7/gcc/default/build/gcc/xgcc -B/scratch/cbuild/slave/slaves/tcpanda06/gcc-linaro-4.7+bzr114971~ams-codesourcery~neon-shifts-4.7/gcc/default/build/gcc/ /scratch/cbuild/slave/slaves/tcpanda06/gcc-linaro-4.7+bzr114971~ams-codesourcery~neon-shifts-4.7/gcc/gcc-linaro-4.7/gcc/testsuite/gcc.dg/di-sync-multithread.c -pthread -std=gnu99 -lm -o ./di-sync-multithread.exe
/tmp/cc0h1XGt.s: Assembler messages:
/tmp/cc0h1XGt.s:37: Error: co-processor offset out of range
/tmp/cc0h1XGt.s:43: Error: co-processor offset out of range
/tmp/cc0h1XGt.s:49: Error: co-processor offset out of range

review: Needs Fixing
Revision history for this message
Andrew Stubbs (ams-codesourcery) wrote :

Hmmm, I've now built this on guinep.canonical.com and I can't reproduce this failure. The patch isn't applied to the exact same baseline, so I can still try that, but so far I'm mystified.

Revision history for this message
Andrew Stubbs (ams-codesourcery) wrote :

The error is caused by an instruction like this:

        fldd d16, .L12 @ int

(where .L12 is out of range for an fldd instruction.)

This is annoying because my patch doesn't meddle with any of the constant pool code: this is a latent bug, I think.

Revision history for this message
Andrew Stubbs (ams-codesourcery) wrote :

I've examined the broken testcase and the neon-shift patterns within it, but I can't see any fault there.

The testcase is built at -O0 in which only the first alternative is used; this means a straight-forward 4-byte neon shift instruction, so it's hard to see how that could be a problem. Right shifts might be problematic, as they split to 3 instructions, but this testcase doesn't use any.

I've not yet determined where the miscalculation does occur, however, it must be due to a split after the machine reorg pass, so it ought to be easy to find, if tedious.

Revision history for this message
Andrew Stubbs (ams-codesourcery) wrote :

Ok. I've checked what's going on here. This testcase uses several "atomic_*" insns from sync.md.

The insns typically split into about 7 new insns, AFAICS, and none of the patterns in sync.md have a length attribute, so they'll have been counted as only one machine instruction. This is not a problem, presumably, when optimization is enabled because the constant pools are not laid down until the machine reorg pass, which is quite late, and they're very likely to have already split by that stage. Unfortunately, when optimization is not enabled, as in this testcase, the split does not occur until the very last opportunity, and this is after reorg. Therefore, whether the test passes or not is entirely down to chance, and my patch happened to perturb things sufficiently change the result.

The problem are new and were not present in the sources I used to develop the patch, hence why I had trouble reproducing the problem initially.

Subscribers

People subscribed via source and target branches