[llvm] [DWARFLinkerParallel] Change more cases of compare_exchange_weak to compare_exchange_strong (PR #138692)

[Martin Storsjö via llvm-commits]

mstorsjo wrote:

> > > > > The remaining uses of compare_exchange_weak() in
> > > > > DWARFLinkerCompileUnit.h also seem very suspicious; if there would be a true concurrent write, leading to compare_exchange_weak() returning false, it seems like we would get stuck in an endless loop - but changing them to compare_exchange_strong() wouldn't make any difference with respect to that.
> > > > 
> > > > 
> > > > Can you have a look at these cases? (I haven't run into issues with them, but the code looks like it could lock up.)
> > > 
> > > 
> > > yes, you are right those places look incorrect. Thank you for pointing to this.
> > > Instead of
> > > ```
> > >       auto InputData = Flags.load();
> > >       while (!Flags.compare_exchange_weak(InputData,
> > >                                           ((InputData & ~0x7) | Placement))) {
> > >       }
> > > ```
> > > 
> > > 
> > >     
> > >       
> > >     
> > > 
> > >       
> > >     
> > > 
> > >     
> > >   
> > > there should be something like
> > > ```
> > >       do {
> > >         auto InputData = Flags.load();
> > >       while (!Flags.compare_exchange_weak(InputData,
> > >                                           ((InputData & ~0x7) | Placement)));
> > > ```
> > > 
> > > 
> > >     
> > >       
> > >     
> > > 
> > >       
> > >     
> > > 
> > >     
> > >   
> > > Would you prefer me to do these changes or would you prefer to do these changes youself?
> > 
> > 
> > Yes, that looks correct to me. As you're more familiar with the overall intent here (I'm just trying to debug things that show up when running `ninja check` ;-) ) I'd prefer if you'd open such a PR, but I can review it!
> 
> My answer was too hurry. This code looks OK:
> 
> ```
> auto InputData = Flags.load();
> while (!Flags.compare_exchange_weak(InputData,
>                                     ((InputData & ~0x7) | Placement))) {
> }
> ```
> 
> There should not be endless loop here. if there would be a true concurrent write, leading to compare_exchange_weak() returning false, then InputData would be updated with current value of "Flags". Then it would - either match on next iteration and loop will finish, either(in case new concurrent write) it would be updated again.
> 
> So, after this MR was integrated, it looks like no more other issues inside dwarflinker parallel with usages of compare_exchange_weak.

Oh, indeed - you're right, I forgot that it updates the `InputData` variable with the new value on change. Yes then those things should indeed be ok.

https://github.com/llvm/llvm-project/pull/138692