Apply the same logic used to check if CMPXCHG nodes should be expanded at -O0: the register allocator may end up spilling some register in between the atomic load/store pairs, breaking the atomicity and possibly stalling the execution.
Fixes PR48017
Paths
| Differential D101163
[AArch64] Prevent spilling between ldxr/stxr pairs ClosedPublic Authored by LemonBoy on Apr 23 2021, 7:16 AM.
Details Summary Apply the same logic used to check if CMPXCHG nodes should be expanded at -O0: the register allocator may end up spilling some register in between the atomic load/store pairs, breaking the atomicity and possibly stalling the execution. Fixes PR48017
Diff Detail
Event TimelineHerald added subscribers: tmatheson, danielkiss, jfb and 2 others. · View Herald TranscriptApr 23 2021, 7:16 AM Comment Actions The same problem exists in the ARM backend, which I fixed internally by implementing new pseudos for the atomicrmw instructions, but hadn't got round to upstreaming it yet. Here it is for comparison: D101164. Comment Actions
That sounds like proper way of fixing this problem and produce tighter code. Comment Actions
I don't think that patch addresses this issue.
Comment Actions LGTM with one minor comment.
This revision is now accepted and ready to land.Apr 30 2021, 10:44 AM This revision was landed with ongoing or failed builds.May 1 2021, 8:17 AM Closed by commit rG4751cadcca45: [AArch64] Prevent spilling between ldxr/stxr pairs (authored by LemonBoy). · Explain Why This revision was automatically updated to reflect the committed changes.
Revision Contents
Diff 342154 llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
llvm/test/CodeGen/AArch64/GlobalISel/arm64-atomic.ll
llvm/test/CodeGen/AArch64/atomicrmw-O0.ll
llvm/test/Transforms/AtomicExpand/AArch64/expand-atomicrmw-xchg-fp.ll
| ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
"implement cmpxchg" doesn't seem right; should be "implement atomicrmw"? (Also "address being exchanged".)