On Tue, Oct 13, 2026 at 10:45 AM Vineet Gupta <[email protected]> wrote: > > 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
Why does this pass recog when it later needs reloading? But yes, combining multiple volatile loads is problematic. > 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 >
