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, so whatever later reloads or
splits
it picks the order of the two volatile reads, and the sequence point
between them is lost. can_combine_p checks insns between INSN
and I3
for volatile references, but not I3 itself.
The x86 compare patterns now reject such operands (dc57b45e7a79),
but
other patterns accept them too: for "h = hi; l = lo;" combined into
((dw) h << N) | l, the x86 doubleword concat patterns take both
halves
as memory operands and their split reads lo first. Rather than
fixing
each pattern, refuse in combine to combine an insn that reads
volatile
memory directly into an I3 that already reads volatile memory.
The new volatile_reads_p walks I3's pattern: every SET in a PARALLEL
counts, and so does anything read to form a stored location (a MEM's
address, a ZERO_EXTRACT's position and size), but a volatile MEM
that is
only stored to does not, since an insn does all its reads before its
writes. That does not hold for an asm, whose template may access
its
operands in any order, so for an asm I3 any volatile reference
refuses.
Bootstrapped and regression tested on x86_64-linux-gnu and
aarch64-linux-gnu.
PR rtl-optimization/127851
gcc/ChangeLog:
* combine.cc (volatile_reads_p): New function.
(can_combine_p): Use it to refuse combining an insn that
reads
volatile memory directly into an I3 that already reads
volatile
memory; for an asm I3, refuse on any volatile reference.
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-4.c: New test.
* gcc.target/i386/volatile-fuse-order-5.c: New test.
* gcc.target/i386/volatile-fuse-order-6.c: New test.
Signed-off-by: Vineet Gupta <[email protected]>
---
v4: - volatile_reads_p walks I3's pattern: every SET in a
PARALLEL counts,
and so do reads in a SET's destination (a MEM's address, a
ZERO_EXTRACT's position/size); a volatile store alone
does not
(Jeff).
- For an asm I3 any volatile reference refuses: its
template may write
an output before reading an input.
- x86 volatile-fuse-order-1/2/3 dropped, they went in with
dc57b45e7a79. New x86 tests: -4/-5 (asm, including
multi-output
PARALLELs) and -6 (doubleword concat), which still fail
on trunk.
- With the i386.md part of dc57b45e7a79 reverted, -1..-6
still pass:
combine refuses the merge itself.
v3: the x86 check-function-bodies now match only the load and the
compare;
use ia32 / ! ia32 selectors, so -mx32 is covered too (Uros).
v2: the x86 check-function-bodies tests now also run and check
the bodies
on ia32 (Uros); added -fno-pic for the global-variable ones.
gcc/combine.cc | 58
+++++++++++++++++++
.../aarch64/volatile-fuse-order-1.c | 24 ++++++++
.../aarch64/volatile-fuse-order-2.c | 23 ++++++++
.../gcc.target/i386/volatile-fuse-order-4.c | 49
++++++++++++++++
.../gcc.target/i386/volatile-fuse-order-5.c | 45
++++++++++++++
.../gcc.target/i386/volatile-fuse-order-6.c | 35 +++++++++++
6 files changed, 234 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-4.c
create mode 100644
gcc/testsuite/gcc.target/i386/volatile-fuse-order-5.c
create mode 100644
gcc/testsuite/gcc.target/i386/volatile-fuse-order-6.c
diff --git a/gcc/combine.cc b/gcc/combine.cc
index c9daefc905c3..b369914e49e4 100644
--- a/gcc/combine.cc
+++ b/gcc/combine.cc
@@ -1723,6 +1723,53 @@ set_nonzero_bits_and_sign_copies (rtx x,
const_rtx set, void *data)
}
}
+/* Return true if insn pattern X reads volatile memory. */
+
+static bool
+volatile_reads_p (const_rtx x)
+{
+ switch (GET_CODE (x))
+ {
+ case PARALLEL:
+ /* Every element counts, not just the one single_set would
pick. */
+ for (int i = 0; i < XVECLEN (x, 0); i++)
+ if (volatile_reads_p (XVECEXP (x, 0, i)))
+ return true;
+ return false;
+
+ case SET:
+ {
+ /* Strip down to the stored location. The position and
size of a
+ bit-field are read. */
+ rtx dest = SET_DEST (x);
+ while (GET_CODE (dest) == SUBREG
+ || GET_CODE (dest) == STRICT_LOW_PART
+ || GET_CODE (dest) == ZERO_EXTRACT)
+ {
+ if (GET_CODE (dest) == ZERO_EXTRACT
+ && (volatile_refs_p (XEXP (dest, 1))
+ || volatile_refs_p (XEXP (dest, 2))))
+ return true;
+ dest = XEXP (dest, 0);
+ }
+ /* A stored MEM is not read, and since an insn does all its
reads
+ before its writes, a volatile store cannot move ahead of a
+ volatile read in the same insn. Its address is read,
though,
+ e.g. (mem (mem/v ...)). */
+ if (MEM_P (dest) && volatile_refs_p (XEXP (dest, 0)))
+ return true;
+ return volatile_refs_p (SET_SRC (x));
+ }
+
+ case CLOBBER:
+ /* A clobbered MEM is not read, but its address is. */
+ return MEM_P (XEXP (x, 0)) && volatile_refs_p (XEXP (XEXP
(x, 0), 0));
+
+ default:
+ return volatile_refs_p (x);
+ }
+}
+
/* See if INSN can be combined into I3. PRED, PRED2, SUCC
and SUCC2 are
optionally insns that were previously combined into I3 or
that will be
combined into the merger of INSN and I3. The order is
PRED, PRED2,
@@ -2004,6 +2051,17 @@ 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, since all of an insn's
reads happen
+ before its writes, but not in an asm, whose template may
access its
+ operands in any order. (When INSN goes through SUCC,
combine may
+ split the result back into two insns that keep the reads
ordered.) */
+ if (succ == 0
+ && (asm_noperands (PATTERN (i3)) >= 0
+ ? volatile_refs_p (PATTERN (i3))
+ : volatile_reads_p (PATTERN (i3))))
+ return false;
