[PATCH] Add -fcombine-op-with-volatile-memory-load

[Uros Bizjak]

On Tue, Oct 28, 2025 at 9:31 AM Richard Biener
<richard.guenther@gmail.com> wrote:
>
> On Tue, Oct 28, 2025 at 9:29 AM Richard Biener
> <richard.guenther@gmail.com> wrote:
> >
> > On Tue, Oct 28, 2025 at 7:03 AM H.J. Lu <hjl.tools@gmail.com> wrote:
> > >
> > > On Tue, Oct 28, 2025 at 1:26 PM Uros Bizjak <ubizjak@gmail.com> wrote:
> > > >
> > > > On Mon, Oct 27, 2025 at 11:56 PM H.J. Lu <hjl.tools@gmail.com> wrote:
> > > >
> > > > > > > > IMO the -fcombine-op-with-volatile-memory-load is quite a bad name.
> > > > > > > > Options should have names that mean something to users.  As this seems to
> > > > > > > > supposed to be a target opt-in I'd suggest to not expose this to users
> > > > > > > > (or if targets are concerned, via some -m...) but instead make this a
> > > > > > >
> > > > > > > -mvolatile-memory-load?
> > > > > >
> > > > > > -mfuse-ops-with-volatile-access?
> > > > > >
> > > > > > So as to eventually also allow RMW?
> > > > >
> > > > > Yes.  We can add the LOCK prefix for RMW if it is allowed.  But will it
> > > > > improve performance?  The LOCK prefix may be expensive.
> > > >
> > > > No, you don't need the LOCK prefix for volatile RMW access. LOCK is
> > > > needed for inter-thread synchronization, which is orthogonal to RMW
> > > > access.
> > > >
> > > > What Richard mentions is something like PR3506 [1], a step further
> > > > from propagating memory input, where also memory output is propagated
> > > > to form the instruction with RMW access. As an example, maybe you want
> > > > to increase the value stored at some memory-mapped IO. The location
> > > > has to be marked volatile, otherwise optimizers can figure out that
> > > > the memory location is unused and simply remove the update of "unused"
> > > > location.
> > > >
> > > > So, in the PR it is argued that:
> > > >
> > > >         movl    y, %eax
> > > >         incl    %eax
> > > >         movl    %eax, y
> > > >
> > > > access to volatile location is the same as:
> > > >
> > > >         incl    y
> > > >
> > > > and that the latter does not violate volatile correctness.
> > > >
> > > > [1] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=3506
> > > >
> > > > Uros.
> > >
> > > This can be a followup patch.
> >
> > it should be, I was merely mentioning this to make sure the command-line option,
> > if we add one, would be one that would cover this case as well (so not
> > use 'load'
> > in the name).
>
> Oh, and I'd expect eventual fallout for define-insn-and-split or
> UNSPECs that fail
> to check volatileness on MEM operands but ends up duplicating or splitting them
> or merging them.  Basically targets that rely on volatile MEMs not appearing as
> operands to RTL ops.

Please also see attachment 55167 in PR60749 [1].

[1] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=60749#c2

Uros.