On 04/02/16 19:35, H.J. Lu wrote:
On Thu, Feb 4, 2016 at 1:34 AM, Kyrill Tkachov
<kyrylo.tkac...@foss.arm.com> wrote:
On 04/02/16 09:13, Ramana Radhakrishnan wrote:
On Fri, Jan 22, 2016 at 9:52 AM, Kyrill Tkachov
<kyrylo.tkac...@foss.arm.com> wrote:
Hi all,
PR target/65932 is a wrong-code bug affecting arm and has manifested
itself
when compiling the Linux kernel, so it's something that we really
ought to fix. The problem stems from the fact that PROMOTE_MODE and
TARGET_PROMOTE_FUNCTION_MODE don't match up on arm.
PROMOTE_MODE also marks the promotion as unsigned, whereas the
TARGET_PROMOTE_FUNCTION_MODE doesn't. This can lead to short variables
being wrongly zero-extended instead of sign-extended. This also occurs
in PR target/67714.
Jim Wilson tried a few approaches and from the discussion
on the PR and on the ML
(https://gcc.gnu.org/ml/gcc-patches/2016-01/msg00814.html)
the preferred approach is to make PROMOTE_MODE and
TARGET_PROMOTE_FUNCTION_MODE
match up. Changing TARGET_PROMOTE_FUNCTION_MODE to zero-extend would be
an
ABI
change so we don't want to do that. Changing PROMOTE_MODE to not
zero-extend
fixes both PR 65932 and 67714. So Jim's patch is the first patch in this
series.
It has been posted at
https://gcc.gnu.org/ml/gcc-patches/2015-06/msg02132.html
and this series is based on top of the arm.h hunk of that patch.
There have been some concerns about the codegen quality fallout from
Jim's
patch, which is what the remaining patches in this series address:
* 3 new arm testsuite failures: gcc.target/arm/wmul-[123].c.
wmul-1.c and wmul-2.c are cases where we no longer generate
sign-extend+multiply (and accumulate) instructions but instead generate
the normal full-width multiplies (the operands are sign-extended from
preceeding sign-extending loads). This is a regression for some targets
on which the sign-extending form is faster. Patches 2 and 3 address this.
gcc.target/arm/wmul-3.c is a test where we actually end up generating
better code and so the testcase just needs to be adjusted.
Patch 4 deals with that.
* Sign-extending rather than zero-extending short values means we make
more
use of the ldrsb and ldrsh arm instructions rather than the
zero-extending
ldrb and ldrh. On Thumb1 targets ldrsb and ldrsh have more limited
addressing
modes (only REG + REG), which could hurt code size. However, the change
also
means that we can now merge sequences of load-zero-extend followed by a
sign-extend
into a single load-sign-extend.
So we'd turn a (ldrh ; sxth) into an (ldrsh).
I've built a few popular embedded benchmarks for a Cortex-M0 target
(Thumb1)
and looked
at the generated assembly for -Os and -O2. I did see both effects. That
is,
ldrh instructions with an immediate being turned into two instructions:
ldrh r4, [r4, #12]
->
movs r5, #12
ldrsh r4, [r4, r5]
But I also observed the beneficial effect. Various register allocation
perturbations also featured in the changes
Overall, for every codebase I've looked at both -Os and -O2 the new code
was
slightly smaller. Probably not enough to call this an outright win, but
definitely not a regression overall.
As for ARM and Thumb2 states, this series doesn't have a major impact.
SPEC2006 bencharks are slightly reduced in size (but nothing to write
home
about) and there are no code quality regressions. And even the
regressions
caught by the testsuite in the wmul-[12].c tests don't actually manifest
in practice in SPEC, but they are fixed anyway.
The series contains one target-independent change to CSE in patch 3 that
I'll explain there.
The series has been bootstrapped and tested on arm, aarch64 and x86_64.
Is this ok for trunk together with Jim's arm.h hunk from
g ?
The whole series is OK provided the middle-end hunk has been approved.
Thanks.
Richi approved the midend hunk at:
https://gcc.gnu.org/ml/gcc-patches/2016-01/msg01733.html
This caused:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69676
Yeah, we already have https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69671.
Seems to be something with the x86 patterns. Kirill has assigned it to himself.
Thanks,
Kyrill