Merge lp:~rob-welcomehome/gcc-linaro/4.8-merge-199814 into lp:gcc-linaro

Proposed by rsavoye
Status: Rejected
Rejected by: Yvan Roux
Proposed branch: lp:~rob-welcomehome/gcc-linaro/4.8-merge-199814
Merge into: lp:gcc-linaro
To merge this branch: bzr merge lp:~rob-welcomehome/gcc-linaro/4.8-merge-199814
Reviewer Review Type Date Requested Status
Yvan Roux Disapprove
Review via email: mp+171110@code.launchpad.net

Description of the change

Merge of 199814.

To post a comment you must log in.
Revision history for this message
Linaro Toolchain Builder (cbuild) wrote :

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

The diff against the ancestor r122056 is available at:
 http://cbuild.validation.linaro.org/snapshots/gcc-linaro-4.8+bzr122057~rob-welcomehome~4.8-merge-199814.diff

and will be built on the following builders:
 a9-builder a9hf-builder armv5-builder i686 x86_64 xaarch64 xaarch64_bare xcortexa15hf

You can track the build queue at:
 http://cbuild.validation.linaro.org/helpers/scheduler

cbuild-snapshot: gcc-linaro-4.8+bzr122057~rob-welcomehome~4.8-merge-199814
cbuild-ancestor: lp:gcc-linaro+bzr122056
cbuild-state: check

Revision history for this message
Yvan Roux (yvan-roux) wrote :

* Changelog.linaro entries are duplicated
* Diff matches upstream
* one regression : gcc.target/arm/neon-for-64bits-2.c on armv5, a9, a9hf and a15
* one regression : gcc.dg/torture/va-arg-25.c on i686
* aarch64 targets are clean
* x86_64 results are missing

review: Needs Fixing
Revision history for this message
Yvan Roux (yvan-roux) wrote :

* x86_64 is clean
* Need to understand regressions for approval

review: Needs Information
Revision history for this message
Christophe Lyon (christophe-lyon) wrote :

The patch being merged here is discussed at:
http://gcc.gnu.org/ml/gcc-patches/2013-06/msg00395.html

This thread mentions an earlier patch (for OR, instead of XOR)
http://gcc.gnu.org/ml/gcc-patches/2013-05/msg01838.html
which was accepted although the author explains that neon-for-64bits-2.c has a regression which the patch does not even set to XFAIL.

I am wondering whether the case wouldn't be similar in this case.

Anyway, for the testcase in question, here is the generated code before, and after patch 199814:
Before:
binary_xor:
 fldd d17, [r1] @ int
 fldd d16, [r2] @ int
 veor d16, d17, d16
 fstd d16, [r0] @ int
 bx lr

After:
binary_xor:
 ldrd r2, [r2]
 stmfd sp!, {r4, r5}
 ldrd r4, [r1]
 eor r4, r4, r2
 eor r5, r5, r3
 strd r4, [r0]
 ldmfd sp!, {r4, r5}
 bx lr

Note that this test makes uses of the -mneon-for-64bits option, recently introduced and probably not widely used.

I guess the patch improves code otherwise (ie when this option is not in use), but I can't be completely sure since a previous and similar one already introduced a regression with no attempt to fix the testcase at least.

Revision history for this message
Yvan Roux (yvan-roux) wrote :

* the i686 re-spawned build is clean...

Revision history for this message
Yvan Roux (yvan-roux) :
review: Needs Fixing
Revision history for this message
Yvan Roux (yvan-roux) wrote :

deprecated

review: Disapprove

Subscribers

People subscribed via source and target branches

to all changes: