On Wed, Nov 26, 2025 at 8:23 PM H.J. Lu <[email protected]> wrote:
>
> On Wed, Nov 26, 2025 at 8:00 PM Uros Bizjak <[email protected]> wrote:
> >
> > On Wed, Nov 26, 2025 at 12:43 PM H.J. Lu <[email protected]> wrote:
> > >
> > > On Wed, Nov 26, 2025 at 5:53 PM Uros Bizjak <[email protected]> wrote:
> > > >
> > > > On Wed, Nov 26, 2025 at 10:18 AM H.J. Lu <[email protected]> wrote:
> > > > >
> > > > > On Wed, Nov 26, 2025 at 3:52 PM Uros Bizjak <[email protected]> wrote:
> > > > > >
> > > > > > On Wed, Nov 26, 2025 at 8:26 AM H.J. Lu <[email protected]> wrote:
> > > > > >
> > > > > > > > > The reasons for try_combine_insn are that
> > > > > > > > >
> > > > > > > > > 1. When general_operand is called, instruction info isn't
> > > > > > > > > available.
> > > > > > > > > 2. Only recog_for_combine_1 calls init_recog_no_volatile and
> > > > > > > > > tries memory operands.
> > > > > > > > But if we make stores work sensibly, then we no longer need the
> > > > > > > > instruction, right? It's the mere need to track the
> > > > > > > > instruction that
> > > > > > > > seems rather hokey/hackish to me right now.
> > > > > > > >
> > > > > > > > jeff
> > > > > > >
> > > > > > > For
> > > > > > >
> > > > > > > volatile unsigned char u8;
> > > > > > >
> > > > > > > void test (void)
> > > > > > > {
> > > > > > > u8 = u8 + u8;
> > > > > > > u8 = u8 - u8;
> > > > > > > }
> > > > > > >
> > > > > > > When volatile store is allowed, we generate
> > > > > > >
> > > > > > > (insn 8 7 9 2 (parallel [
> > > > > > > (set (mem/v/c:QI (symbol_ref:DI ("u8") [flags 0x2]
> > > > > > > <var_decl 0x7fe9719d6e40 u8>) [0 u8+0 S1 A8])
> > > > > > > (ashift:QI (mem/v/c:QI (symbol_ref:DI ("u8")
> > > > > > > [flags
> > > > > > > 0x2] <var_decl 0x7fe9719d6e40 u8>) [0 u8+0 S1 A8])
> > > > > > > (const_int 1 [0x1])))
> > > > > > > (clobber (reg:CC 17 flags))
> > > > > > > ])
> > > > > > > "/export/gnu/import/git/gitlab/x86-gcc-test/gcc/testsuite/gcc.dg/pr86617.c":7:6
> > > > > > > 1139 {*ashlqi3_1}
> > > > > > > (expr_list:REG_UNUSED (reg:CC 17 flags)
> > > > > > > (nil)))
> > > > > > >
> > > > > > > on x86 which leads to
> > > > > > >
> > > > > > > salb u8(%rip)
> > > > > > >
> > > > > > > instead of 2 loads. Without the instruction, we don't know if a
> > > > > > > memory reference
> > > > > > > should be allowed for stores.
> > > > > >
> > > > > > combine pass doesn't handle volatiles correctly in all cases, this
> > > > > > is
> > > > > > the reason I think this propagation should be done in late-combine.
> > > > > > We
> > > > > > are interested only in propagations of memory loads/stores into
> > > > > > instructions, and late-combine does exactly that.
> > > > > >
> > > > > > Uros.
> > > > >
> > > > > late combine doesn't try to combine memory references at all.
> > > >
> > > > Try PR122343 testcase with the attached patch (gcc -O2):
> > > >
> > > > _.308r.late_combine1:
> > > >
> > > > trying to combine definition of r98 in:
> > > > 6: r98:SI=[`bar']
> > > > into:
> > > > 8: {r102:SI=r103:SI+r98:SI;clobber flags:CC;}
> > > > REG_DEAD r103:SI
> > > > REG_DEAD r98:SI
> > > > REG_UNUSED flags:CC
> > > > successfully matched this instruction to *addsi_1:
> > > > (parallel [
> > > > (set (reg:SI 102 [ _5 ])
> > > > (plus:SI (reg:SI 103 [ z_3 ])
> > > > (mem/v/c:SI (symbol_ref:DI ("bar") [flags 0x40]
> > > > <var_decl 0x7f7810bd6e40 bar>) [1 bar+0 S4 A32])))
> > > > (clobber (reg:CC 17 flags))
> > > > ])
> > > > original cost = 5 + 4 (weighted: 9.000000), replacement cost = 9
> > > > (weighted: 9.000000); keeping replacement
> > > > rescanning insn with uid = 8.
> > > > updating insn 8 in-place
> > > > verify found no changes in insn with uid = 8.
> > > > deleting insn 6
> > > > deleting insn with uid = 6.
> > > >
> > > > foo:
> > > > imull $123, %edi, %eax
> > > > addl bar(%rip), %eax
> > > > ret
> > > >
> > > > Uros.
> > >
> > > It doesn't work on:
> > >
> > > extern volatile int bar;
> > >
> > > void
> > > foo (void)
> > > {
> > > bar += 123;
> > > }
> > >
> > > Your patch generates:
> > >
> > > movl bar(%rip), %eax
> > > addl $123, %eax
> > > movl %eax, bar(%rip)
> >
> > This is expected, late-combine pass won't create RMW instructions.
> >
> > Uros.
>
> I am testing this patch. For
>
> extern volatile unsigned char u8;
>
> void
> test (void)
> {
> u8 = u8 + u8;
> u8 = u8 - u8;
> }
>
> it generates:
>
> movzbl u8(%rip), %eax
> addb %al, u8(%rip)
> movzbl u8(%rip), %eax
> subb u8(%rip), %al
> movb %al, u8(%rip)
> ret
>
> --
> H.J.
An updated patch.
--
H.J.
diff --git a/gcc/common.opt b/gcc/common.opt
index d44f713ae34..0c78b5199c6 100644
--- a/gcc/common.opt
+++ b/gcc/common.opt
@@ -1828,6 +1828,10 @@ ffunction-sections
Common Var(flag_function_sections)
Place each function into its own section.
+ffuse-ops-with-volatile-access
+Target Var(flag_fuse_ops_with_volatile_access) Init(1)
+Allow operations with volatile memory access in the combine pass.
+
fgcse
Common Var(flag_gcse) Optimization
Perform global common subexpression elimination.
diff --git a/gcc/common.opt.urls b/gcc/common.opt.urls
index d13af0a8e7c..870d2efdbed 100644
--- a/gcc/common.opt.urls
+++ b/gcc/common.opt.urls
@@ -782,6 +782,9 @@ UrlSuffix(gcc/Optimize-Options.html#index-ffunction-cse)
ffunction-sections
UrlSuffix(gcc/Optimize-Options.html#index-ffunction-sections)
+ffuse-ops-with-volatile-access
+UrlSuffix(gcc/Optimize-Options.html#index-ffuse-ops-with-volatile-access)
+
fgcse
UrlSuffix(gcc/Optimize-Options.html#index-fgcse)
diff --git a/gcc/recog.cc b/gcc/recog.cc
index 67d7fa63069..bfa10df9e4b 100644
--- a/gcc/recog.cc
+++ b/gcc/recog.cc
@@ -1596,7 +1596,11 @@ general_operand (rtx op, machine_mode mode)
{
rtx y = XEXP (op, 0);
- if (! volatile_ok && MEM_VOLATILE_P (op))
+ /* If -ffuse-ops-with-volatile-access is enabled, allow volatile
+ memory reference. */
+ if (!flag_fuse_ops_with_volatile_access
+ && !volatile_ok
+ && MEM_VOLATILE_P (op))
return false;
/* Use the mem's mode, since it will be reloaded thus. LRA can
diff --git a/gcc/simplify-rtx.cc b/gcc/simplify-rtx.cc
index 86baeb06ad4..f3601637319 100644
--- a/gcc/simplify-rtx.cc
+++ b/gcc/simplify-rtx.cc
@@ -3189,7 +3189,9 @@ simplify_context::simplify_binary_operation_1 (rtx_code code,
rhs = XEXP (rhs, 0);
}
- if (rtx_equal_p (lhs, rhs))
+ /* Keep PLUS of 2 volatile memory references. */
+ if (rtx_equal_p (lhs, rhs)
+ && (!MEM_P (lhs) || !MEM_VOLATILE_P (lhs)))
{
rtx orig = gen_rtx_PLUS (int_mode, op0, op1);
rtx coeff;
