This is an archive of the discontinued LLVM Phabricator instance.

[lldb][AArch64] Use atomics to sync threads in SVE threading test
ClosedPublic

Authored by DavidSpickett on Aug 15 2023, 5:03 AM.

Details

Summary

Previously we would "process continue" then wait for the number of
threads to be 3 before proceeding with the test.

Testing this on QEMU I saw it would sometimes get stuck at this check,
with one of the threads on a breakpoint before the other had started.
We do want it to be on a breakpoint, but we need the other thread to have
at least started so lldb can interact with both.

I've also seen it timeout on the Graviton buildbot, likely the same
cause.

To fix this add 2 variables to stall either thread until the other
has started up. Then it doesn't matter which one hits its breakpoint
first, the test will just continue the one that didn't, until both
are on the expected breakpoint.

Diff Detail

Event Timeline

DavidSpickett created this revision.Aug 15 2023, 5:03 AM
Herald added a project: Restricted Project. · View Herald TranscriptAug 15 2023, 5:03 AM
DavidSpickett requested review of this revision.Aug 15 2023, 5:03 AM
Herald added a project: Restricted Project. · View Herald TranscriptAug 15 2023, 5:03 AM

One recent example of a timeout on the Graviton bot: https://lab.llvm.org/buildbot/#/builders/96/builds/43649

I was not able to reproduce the bot's timeout situation with this or the previous implementation, but I did run this on a loop with stress in the background and didn't get any failures. I think doing it with atomics is at least in theory the proper way to do it regardless, even if it has its own corner cases that I haven't thought of.

Matt added a subscriber: Matt.Aug 15 2023, 8:15 PM

Actually these don't need to be atomic. They aren't writing to the same locations
we just have one writer and one reader, and if the reader is delayed a bit it's
not a problem.

The volatile isn't needed at -O0 but might as well on the off chance something tries
to optimise it later.

This revision was not accepted when it landed; it landed in state Needs Review.Aug 31 2023, 2:06 AM
This revision was automatically updated to reflect the committed changes.

The only way to know this truly works is to observe it on the bot for a while and I'm already piling too many reviews on Omair as it is. So I've gone ahead and landed this.