This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
[PATCH][AArch64] Don't split 64-bit constant stores to volatile location
- From: Kyrill Tkachov <kyrylo dot tkachov at foss dot arm dot com>
- To: "gcc-patches at gcc dot gnu dot org" <gcc-patches at gcc dot gnu dot org>
- Cc: Richard Earnshaw <Richard dot Earnshaw at arm dot com>, James Greenhalgh <James dot Greenhalgh at arm dot com>, Marcus Shawcroft <Marcus dot Shawcroft at arm dot com>
- Date: Thu, 22 Aug 2019 10:16:28 +0100
- Subject: [PATCH][AArch64] Don't split 64-bit constant stores to volatile location
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)?
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.
diff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/aarch64.md
index 753c7978700d441c71cf97d5ae1e11f160b270af..cb91da796b3e9bfd6f19e714ccee7791b9cf3714 100644
--- a/gcc/config/aarch64/aarch64.md
+++ b/gcc/config/aarch64/aarch64.md
@@ -1108,8 +1108,8 @@
(match_operand:GPI 1 "general_operand"))]
""
"
- if (MEM_P (operands[0]) && CONST_INT_P (operands[1])
- && <MODE>mode == DImode
+ if (MEM_P (operands[0]) && !MEM_VOLATILE_P (operands[0])
+ && CONST_INT_P (operands[1]) && <MODE>mode == DImode
&& aarch64_split_dimode_const_store (operands[0], operands[1]))
DONE;
diff --git a/gcc/testsuite/gcc.target/aarch64/nosplit-di-const-volatile_1.c b/gcc/testsuite/gcc.target/aarch64/nosplit-di-const-volatile_1.c
new file mode 100644
index 0000000000000000000000000000000000000000..da5975ad1652c00a3b69b3dc19e5c09c77526de7
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/nosplit-di-const-volatile_1.c
@@ -0,0 +1,15 @@
+/* Check that storing the 64-bit immediate to a volatile location is done
+ with a single store. */
+
+/* { dg-do compile } */
+/* { dg-options "-O2" } */
+
+typedef unsigned long long u64;
+
+void bar (u64 *x)
+{
+ *(volatile u64 *)x = 0xabcdef10abcdef10ULL;
+}
+
+/* { dg-final { scan-assembler-times "str\tx..?, .*" 1 } } */
+/* { dg-final { scan-assembler-not "str\tw..?, .*" } } */