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

--- Comment #12 from Hans-Peter Nilsson <hp at gcc dot gnu.org> ---
(In reply to Jonathan Wakely from comment #8)
> (In reply to Hans-Peter Nilsson from comment #0)
> > In conclusion, I think the primary bug is that
> > libstdc++-v3/include/ext/atomicity.h isn't overridden (or perhaps just
> > __gnu_cxx::__exchange_and_add_single?) and in any case that the
> > atomicity.h target-machinery appears to have rotten.
> 
> I agree that this has changed, but I think it's unrelated to this issue.

Absolutely.  FAOD: by my "primary".."tertiary" designations I did not imply a
dependency, but instead independent issues incidentally exposed by analyzing
what's exposed by the regressions from r16-3810.

> We no longer use config/cpu/cris/atomicity.h because using
> __atomic_fetch_add works fine on an _Atomic_word. Reverting to using
> config/cpu/cris/atomicity.h wouldn't help here, because
> __exchange_and_add_single would still use make_unsigned<_Atomic_word>.

Right, but presumably __exchange_and_add_single could as part of that be made
to default to __exchange_and_add as part of fixing the atomic_dir rot.  But I
believe that's moot, presuming you go for nuking all cpu/X/atomicity.h.

> Do we actually need config/cpu/cris/atomicity.h nowadays? A quick test does
> suggest the custom assembly code is smaller:

[assembly elided]

> Is that because __atomic_fetch_add works with any int, but the custom
> assembly in config/cpu/cris/atomicity.h only has to support the 4-byte
> aligned _Atomic_word?

That code looked weird so I had to look, and past-me added this comment in
gcc/config/cris/sync.md to clear up the confusion: :-)

      /* This one is for CRIS versions without load-locked-store-conditional
         machinery; assume single-core-non-shared-memory without user
         mode/supervisor mode distinction, and just disable interrupts
         while performing the operation.
         Rather than making this pattern more complex by freeing another
         register or stack position to save condition codes (the value
         of the interrupt-enabled bit), we check whether interrupts were
         enabled before we disabled them and branch to a version
         with/without afterwards re-enabling them.  */

So the "btstq 5..."-and-so-on checks the interrupt-mask bit, trading bigger
code (for this extinct base CRIS version) for keeping the same register
allocation requirements.  Current-me agrees that this is a sane decision. :-)

So, no, the generic code is just longer code and more complicated in order to
not require a scratch-register.  FAOD: cris/atomicity.h can (and should) be
deleted.

> > The secondary
> > bug is the assumption that _Atomic_word is an undecorated integer
> > type where std::make_unsigned<_Atomic_word> can be applied (and tertiary
> > that it *can't* be applied, but that's a known issue).
> 
> That bug is entirely independent of which value we use for $atomicity_dir.

Yes; see above "Absolutely..." agreement.

Reply via email to