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

--- Comment #27 from mwahab at gcc dot gnu.org ---
(In reply to Andrew Macleod from comment #25)
> My opinion:
> 
> 1) is undesirable... even though it could possibly accelerate the conversion
> of legacy sync to atomic calls... I fear it would instead just cause
> frustration, annoyance and worse.  I don't think we need to actually
> penalize sync for no reason better than a 1 in a somethillion shot in the
> dark.
> 
> 2) Similar reasoning, I don't think we need to penalize SEQ_CST everywhere
> for a legacy sync call that probably doesn't need stronger code either.
> 
> which leaves option 3)
> 
> There are 2 primary options I think
> 
> a) introduce an additional memory model... MEMMODEL_SYNC or something which
> is even stronger than SEQ_CST.  This would involve fixing any places that
> assume SEQ_CST is the highest.  And then we use this from the places that
> expand sync calls as the memory model.  
>
> or
> b) When we are expanding sync calls, we first look for target __sync
> patterns instead of atomic patterns.  If they exist, we use those. Otherwise
> we simply expand like we do today.  
> 
> 
> (b) may be easier to implement, but puts more onus on the target.. probably
> involves more complex patterns since you need both atomic and sync patterns.
> My guess is some duplication of code will occur here.  But the impact is
> only to specific targets.
> 
> (a) Is probably cleaner... just touches a lot more code.  Since we're in
> stage 1, maybe (a) is a better long term solution...?   
> 

Adding a new barrier specifer to enum memmodel seems the simplest approach. It
would mean that all barriers used in gcc are represented in the same way and
that would make them more visible and easier to understand than splitting them
between the enum memmodel and the atomic/sync patterns.

I'm testing a patch-set for option (a). It touches several backends but all the
changes are very simple since the new barrier is the same as MEMMODEL_SEQ_CST
for most targets.

One thing though is that the aarch64 code for __sync_lock_test_and_set and
there may have a similar problem with a too-weak barrier. It's not confirmed
that there is a problem but, since it may mean more work in this area, I
thought I'd mention it.

Reply via email to