On Wed, Oct 14, 2026 at 7:21 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
> 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]>
> ---
> 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 | 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 | 37 +++++++++++++++++++
> .../gcc.target/i386/volatile-fuse-order-3.c | 32 ++++++++++++++++
> 6 files changed, 149 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 429530816846..49288e950787 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..2d917846851a
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/i386/volatile-fuse-order-1.c
> @@ -0,0 +1,26 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -fomit-frame-pointer -fno-pic -masm=att" } */
> +/* { dg-final { check-function-bodies "**" "" "" { target { ! *-*-darwin* }
> } {^\t?\.} } } */
You probably want to use:
f:
...
movl x(|\(%rip\)), %eax
cmpl y(|\(%rip\)), %eax
...
to skip function prologue and .cfi directives and leave only two lines
that are important.
> +
> +/* 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..85804d7f2e5b
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/i386/volatile-fuse-order-2.c
> @@ -0,0 +1,37 @@
> +/* { dg-do compile { target { ia32 || lp64 } } } */
Just:
/* { dg-do compile } */
> +/* { dg-options "-O2 -fomit-frame-pointer -masm=att" } */
> +/* { dg-final { check-function-bodies "ia32" "*ia32" "" { target { ia32 && {
> ! *-*-darwin* } } } {^\t?\.} } } */
> +/* { dg-final { check-function-bodies "x64" "*x64" "" { target { lp64 && { !
> *-*-darwin* } } } {^\t?\.} } } */
/* { dg-final { check-function-bodies "x64" "*x64" "" { target { { !
ia32 } && { ! *-*-darwin* } } } {^\t?\.} } } */
Use ia32 and { ! ia32 }, please see gcc.target/i386/truncsfbf-1.c for example.
> +
> +/* 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. */
> +
> +/*
> +ia32f:
> +ia32.LFB0:
> +ia32 .cfi_startproc
> +ia32 movl 4\(%esp\), %eax
> +ia32 movl \(%eax\), %edx
> +ia32 cmpl \(%eax\), %edx
> +ia32 setl %al
> +ia32...
> +*ia32
> +
> +x64f:
> +x64.LFB0:
> +x64 .cfi_startproc
> +x64 movl \(%rdi\), %eax
> +x64 cmpl \(%rdi\), %eax
> +x64 setl %al
> +x64...
> +*x64
> +*/
Again, you should use only two lines that are important.
> +
> +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..46d588d58864
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/i386/volatile-fuse-order-3.c
> @@ -0,0 +1,32 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -fomit-frame-pointer -fno-pic -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
And here.
Uros.
> +**...
> +*/
> +
> +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
>
