[Bug rtl-optimization/109930] New: transform atomic exchange to unconditional store when old value is unused?

[Bug rtl-optimization/109930] New: transform atomic exchange to unconditional store when old value is unused?

[Simon.Richter at hogyros dot de]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109930

            Bug ID: 109930
           Summary: transform atomic exchange to unconditional store when
                    old value is unused?
           Product: gcc
           Version: 13.1.1
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: rtl-optimization
          Assignee: unassigned at gcc dot gnu.org
          Reporter: Simon.Richter at hogyros dot de
  Target Milestone: ---

I'm not sure if that is a valid substitution, but...

I have a state machine that has a few transitions where I already know the old
state, so I can simply do an unconditional store, but I'd also like to have an
assertion on the old state in my debug version:

    std::atomic<uint32_t> x;

    /* ... */

    auto old_value = x.exchange(5);
    assert(old_value == 3);

with NDEBUG set, the assert is omitted, and no one is interested in the old
value, so the load-with-reserve can be omitted and the store-conditional
replaced with a regular store, and this should still be semantically
equivalent.

[Bug rtl-optimization/109930] transform atomic exchange to unconditional store when old value is unused?

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109930

--- Comment #1 from Richard Biener <rguenth at gcc dot gnu.org> ---
Can you proivde a testcase that can be compiled?

[Bug rtl-optimization/109930] transform atomic exchange to unconditional store when old value is unused?

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109930

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Target|                            |!i*86-* & !x86_64-*
     Ever confirmed|0                           |1
   Last reconfirmed|                            |2023-05-22
           Keywords|                            |missed-optimization
             Status|UNCONFIRMED                 |NEW
           Severity|normal                      |enhancement

--- Comment #2 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Testcase:
```
#include <atomic>

std::atomic<uint32_t> x;
void f()
{
  auto old_value = x.exchange(5);
  (void)old_value;
}

void f0()
{
   x = 5;
}
```

Note on x86_64, these produce the same code generation anyways.
Though on aarch64, it does produce different code. 

I have a suspicion this was reported against x86_64 originally thinking these
two might produce different code generation ...

[Bug rtl-optimization/109930] transform atomic exchange to unconditional store when old value is unused?

[Simon.Richter at hogyros dot de]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109930

--- Comment #3 from Simon Richter <Simon.Richter at hogyros dot de> ---
I was looking at ARMv7 initially.

If I understood the implementation correctly, this can be a generic
optimization.

[Bug rtl-optimization/109930] transform atomic exchange to unconditional store when old value is unused?

[wilco at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109930

Wilco <wilco at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |wilco at gcc dot gnu.org

--- Comment #4 from Wilco <wilco at gcc dot gnu.org> ---
(In reply to Simon Richter from comment #3)
> I was looking at ARMv7 initially.
> 
> If I understood the implementation correctly, this can be a generic
> optimization.

This optimization is only valid for release or relaxed semantics, otherwise you
remove the acquire semantics of the exchange (without proof this is 100% safe,
this will likely allow an illegal reordering).

Btw if you know the old state then there is presumably no concurrent access
here and so you don't need atomic, let alone sequential consistency.

[Bug rtl-optimization/109930] transform atomic exchange to unconditional store when old value is unused?

[Simon.Richter at hogyros dot de]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109930

--- Comment #5 from Simon Richter <Simon.Richter at hogyros dot de> ---
> Btw if you know the old state then there is presumably no concurrent access here and so you don't need atomic, let alone sequential consistency.

I know it in some, but not all cases.

Basically, what I do is

        auto old_x = x.load();

    retry:
        switch(old_x) {
        case 1:
            if(!x.compare_exchange_weak(old_x, 2))
                 goto retry;
            stop_timer();
            old_x = x.exchange(4);
            assert(old_x == 2);
            break;
        case 2:
            // we must have preempted another instance of this function
            // do nothing
            break;
        case 3:
            // handle timeout
            ...
            break;
        case 4:
            // handle operation complete
            ...
        }

This is in code for timeout handling in a realtime system, the timer interrupt
can preempt this. State 1 is "operation in progress", state 2 is "operation
finished", state 3 is "operation timed out", and state 4 is "operation finished
and timer stopped", and the timer interrupt will try to switch from 1 to 3.

The transient state 2 then solves the race between the timer expiring and
stopping the timer (which is asynchronous because the interrupt controller has
a few cycles delay).

So the switch from state 2 to state 4 has release semantics.