With -ffuse-ops-with-volatile-access general_operand accepts volatile
MEMs during combine. For "int a = x; int b = y; return a < b;" with
volatile x and y, combine first merges the load of y into the compare
and then the load of x, giving (compare (mem/v x) (mem/v y)). The
operands of one insn are unordered and LRA loads y first, so the two
sequenced volatile reads are reordered. can_combine_p checks insns
between INSN and I3 for volatile references, but not I3 itself.
Refuse to combine an insn that reads volatile memory directly into an
I3 whose source already reads volatile memory. A volatile store in I3
is still allowed, so read-modify-write forms are unaffected.
Bootstrapped and regression tested on x86_64-linux-gnu and aarch64-linux-gnu.
PR rtl-optimization/127851
gcc/ChangeLog:
* combine.cc (can_combine_p): Don't combine an insn that reads
volatile memory directly into an I3 whose source already reads
volatile memory.
gcc/testsuite/ChangeLog:
* gcc.target/aarch64/volatile-fuse-order-1.c: New test.
* gcc.target/aarch64/volatile-fuse-order-2.c: New test.
* gcc.target/i386/volatile-fuse-order-1.c: New test.
* gcc.target/i386/volatile-fuse-order-2.c: New test.
* gcc.target/i386/volatile-fuse-order-3.c: New test.
Signed-off-by: Vineet Gupta <[email protected]>
---
gcc/combine.cc | 7 ++++
.../aarch64/volatile-fuse-order-1.c | 24 ++++++++++++++
.../aarch64/volatile-fuse-order-2.c | 23 +++++++++++++
.../gcc.target/i386/volatile-fuse-order-1.c | 26 +++++++++++++++
.../gcc.target/i386/volatile-fuse-order-2.c | 25 +++++++++++++++
.../gcc.target/i386/volatile-fuse-order-3.c | 32 +++++++++++++++++++
6 files changed, 137 insertions(+)
create mode 100644 gcc/testsuite/gcc.target/aarch64/volatile-fuse-order-1.c
create mode 100644 gcc/testsuite/gcc.target/aarch64/volatile-fuse-order-2.c
create mode 100644 gcc/testsuite/gcc.target/i386/volatile-fuse-order-1.c
create mode 100644 gcc/testsuite/gcc.target/i386/volatile-fuse-order-2.c
create mode 100644 gcc/testsuite/gcc.target/i386/volatile-fuse-order-3.c
diff --git a/gcc/combine.cc b/gcc/combine.cc
index 11427f5a9204..8c2b84932281 100644
--- a/gcc/combine.cc
+++ b/gcc/combine.cc
@@ -2004,6 +2004,13 @@ can_combine_p (rtx_insn *insn, rtx_insn *i3, rtx_insn
*pred ATTRIBUTE_UNUSED,
return false;
if (succ != 0 && volatile_refs_p (PATTERN (succ)))
return false;
+ /* Nor may INSN go straight into an I3 that already reads volatile
+ memory: the insn would hold both reads with no order between them.
+ A volatile store in I3 is fine (a read-modify-write is ordered). */
+ rtx i3set = single_set (i3);
+ if (succ == 0
+ && volatile_refs_p (i3set ? SET_SRC (i3set) : PATTERN (i3)))
+ return false;
/* We'll check insns between INSN and I3 below. */
}
diff --git a/gcc/testsuite/gcc.target/aarch64/volatile-fuse-order-1.c
b/gcc/testsuite/gcc.target/aarch64/volatile-fuse-order-1.c
new file mode 100644
index 000000000000..25d9bf6540e4
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/volatile-fuse-order-1.c
@@ -0,0 +1,24 @@
+/* { dg-do compile } */
+/* { dg-options "-O2" } */
+/* { dg-final { check-function-bodies "**" "" "" { target lp64 } } } */
+
+/* The two volatile reads are in separate full expressions, so *x must be
+ read before *y. Combine must not put both reads into one insn, which
+ gives them no order. */
+
+/*
+** f:
+** ldr (w[0-9]+), \[x0\]
+** ldr (w[0-9]+), \[x1\]
+** cmp \1, \2
+** cset w0, lt
+** ret
+*/
+
+int
+f (volatile int *x, volatile int *y)
+{
+ int a = *x;
+ int b = *y;
+ return a < b;
+}
diff --git a/gcc/testsuite/gcc.target/aarch64/volatile-fuse-order-2.c
b/gcc/testsuite/gcc.target/aarch64/volatile-fuse-order-2.c
new file mode 100644
index 000000000000..9a391dc866a0
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/volatile-fuse-order-2.c
@@ -0,0 +1,23 @@
+/* { dg-do compile } */
+/* { dg-options "-O2" } */
+/* { dg-final { check-function-bodies "**" "" "" { target lp64 } } } */
+
+/* Both reads of *P are in separate full expressions: the first read must
+ be the first operand of the compare. */
+
+/*
+** f:
+** ldr (w[0-9]+), \[x0\]
+** ldr (w[0-9]+), \[x0\]
+** cmp \1, \2
+** cset w0, lt
+** ret
+*/
+
+int
+f (volatile int *p)
+{
+ int a = *p;
+ int b = *p;
+ return a < b;
+}
diff --git a/gcc/testsuite/gcc.target/i386/volatile-fuse-order-1.c
b/gcc/testsuite/gcc.target/i386/volatile-fuse-order-1.c
new file mode 100644
index 000000000000..f098161fe8c3
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/volatile-fuse-order-1.c
@@ -0,0 +1,26 @@
+/* { dg-do compile { target { ! ia32 } } } */
+/* { dg-options "-O2 -fomit-frame-pointer -masm=att" } */
+/* { dg-final { check-function-bodies "**" "" "" { target { ! *-*-darwin* } }
{^\t?\.} } } */
+
+/* The two volatile reads are in separate full expressions, so x must be
+ read before y. Combine must not put both reads into one compare insn,
+ which gives them no order. */
+
+/*
+**f:
+**.LFB0:
+** .cfi_startproc
+** movl x(|\(%rip\)), %eax
+** cmpl y(|\(%rip\)), %eax
+**...
+*/
+
+extern volatile int x, y;
+
+int
+f (void)
+{
+ int a = x;
+ int b = y;
+ return a < b;
+}
diff --git a/gcc/testsuite/gcc.target/i386/volatile-fuse-order-2.c
b/gcc/testsuite/gcc.target/i386/volatile-fuse-order-2.c
new file mode 100644
index 000000000000..d5b46fcf3ad5
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/volatile-fuse-order-2.c
@@ -0,0 +1,25 @@
+/* { dg-do compile { target lp64 } } */
+/* { dg-options "-O2 -fomit-frame-pointer -masm=att" } */
+/* { dg-final { check-function-bodies "**" "" "" { target { ! *-*-darwin* } }
{^\t?\.} } } */
+
+/* Both reads of *P are in separate full expressions: the first read must
+ be the one loaded into a register before the compare, which performs
+ the second read. */
+
+/*
+**f:
+**.LFB0:
+** .cfi_startproc
+** movl \(%rdi\), %eax
+** cmpl \(%rdi\), %eax
+** setl %al
+**...
+*/
+
+int
+f (volatile int *p)
+{
+ int a = *p;
+ int b = *p;
+ return a < b;
+}
diff --git a/gcc/testsuite/gcc.target/i386/volatile-fuse-order-3.c
b/gcc/testsuite/gcc.target/i386/volatile-fuse-order-3.c
new file mode 100644
index 000000000000..9c8e49522626
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/volatile-fuse-order-3.c
@@ -0,0 +1,32 @@
+/* { dg-do compile { target { ! ia32 } } } */
+/* { dg-options "-O2 -fomit-frame-pointer -masm=att" } */
+/* { dg-final { check-function-bodies "**" "" "" { target { ! *-*-darwin* } }
{^\t?\.} } } */
+
+/* As volatile-fuse-order-1.c, but only the sign of the difference is
+ used, so the two reads would end up as the operands of a MINUS inside
+ the compare, (compare (minus (mem x) (mem y)) (const_int 0)), rather
+ than directly as the compare's operands. x must still be read first. */
+
+/*
+**f:
+**.LFB0:
+** .cfi_startproc
+** movl x(|\(%rip\)), %eax
+** cmpl y(|\(%rip\)), %eax
+**...
+*/
+
+extern volatile int x, y;
+void g1 (void);
+void g2 (void);
+
+void
+f (void)
+{
+ int a = x;
+ int b = y;
+ if ((int) ((unsigned) a - (unsigned) b) < 0)
+ g1 ();
+ else
+ g2 ();
+}
--
2.53.0-Meta
