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

[Alexey Lapshin via llvm-commits]

Martin =?utf-8?q?Storsjö?= <martin at martin.st>
Message-ID:
In-Reply-To: <llvm.org/llvm/llvm-project/pull/138692 at github.com>


avl-llvm 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.


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