This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH][AArch64] Don't split 64-bit constant stores to volatile location
- From: James Greenhalgh <james dot greenhalgh at arm dot com>
- To: Kyrill Tkachov <kyrylo dot tkachov at foss dot arm dot com>
- Cc: "gcc-patches at gcc dot gnu dot org" <gcc-patches at gcc dot gnu dot org>, Richard Earnshaw <Richard dot Earnshaw at arm dot com>, Marcus Shawcroft <Marcus dot Shawcroft at arm dot com>, nd at arm dot com
- Date: Wed, 25 Sep 2019 22:24:39 +0100
- Subject: Re: [PATCH][AArch64] Don't split 64-bit constant stores to volatile location
- References: <e0374ecb-054b-94f3-e344-24e06a070b04@foss.arm.com> <dc9c9d84-0380-3180-d858-836b6dce3dba@foss.arm.com>
On Tue, Sep 24, 2019 at 02:40:20PM +0100, Kyrill Tkachov wrote:
> Hi all,
>
> On 8/22/19 10:16 AM, Kyrill Tkachov wrote:
> > Hi all,
> >
> > The optimisation to optimise:
> > typedef unsigned long long u64;
> >
> > void bar(u64 *x)
> > {
> > *x = 0xabcdef10abcdef10;
> > }
> >
> > from:
> > mov x1, 61200
> > movk x1, 0xabcd, lsl 16
> > movk x1, 0xef10, lsl 32
> > movk x1, 0xabcd, lsl 48
> > str x1, [x0]
> >
> > into:
> > mov w1, 61200
> > movk w1, 0xabcd, lsl 16
> > stp w1, w1, [x0]
> >
> > ends up producing two distinct stores if the destination is volatile:
> > void bar(u64 *x)
> > {
> > *(volatile u64 *)x = 0xabcdef10abcdef10;
> > }
> > mov w1, 61200
> > movk w1, 0xabcd, lsl 16
> > str w1, [x0]
> > str w1, [x0, 4]
> >
> > because we end up not merging the strs into an stp. It's questionable
> > whether the use of STP is valid for volatile in the first place.
> > To avoid unnecessary pain in a context where it's unlikely to be
> > performance critical [1] (use of volatile), this patch avoids this
> > transformation for volatile destinations, so we produce the original
> > single STR-X.
> >
> > Bootstrapped and tested on aarch64-none-linux-gnu.
> >
> > Ok for trunk (and eventual backports)?
> >
> This has been approved by James offline.
>
> Committed to trunk with r276098.
Does this need backporting?
Thanks,
James
>
> Thanks,
>
> Kyrill
>
> > Thanks,
> > Kyrill
> >
> > [1]
> > https://lore.kernel.org/lkml/20190821103200.kpufwtviqhpbuv2n@willie-the-truck/
> >
> >
> > gcc/
> > 2019-08-22 Kyrylo Tkachov <kyrylo.tkachov@arm.com>
> >
> > * config/aarch64/aarch64.md (mov<mode>): Don't call
> > aarch64_split_dimode_const_store on volatile MEM.
> >
> > gcc/testsuite/
> > 2019-08-22 Kyrylo Tkachov <kyrylo.tkachov@arm.com>
> >
> > * gcc.target/aarch64/nosplit-di-const-volatile_1.c: New test.
> >