On Tue, Oct 28, 2025 at 9:29 AM Richard Biener
<[email protected]> wrote:
>
> On Tue, Oct 28, 2025 at 7:03 AM H.J. Lu <[email protected]> wrote:
> >
> > On Tue, Oct 28, 2025 at 1:26 PM Uros Bizjak <[email protected]> wrote:
> > >
> > > On Mon, Oct 27, 2025 at 11:56 PM H.J. Lu <[email protected]> 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.

Richard.

>
> Richard.
>
> >
> > --
> > H.J.

Reply via email to