[llvm-dev] FYI: proposed changes to atomic load/store in SelectionDAG
Philip Reames via llvm-dev
llvm-dev at lists.llvm.org
Wed Aug 28 13:40:11 PDT 2019
I have a set of changes out for review which are possibly note worthy,
and backend contributors may wish to be aware of.
TLDR: atomic loads as normal LoadSDNodes w/an "isAtomic" flag.
Background
At the moment, we lower all atomic loads and stores as instances of
AtomicSDNode (along with cmpxchg, and atomicrmw). This requires us to
duplicate any isel rules we wish to apply for atomic loads or stores,
but does have the nice property that it's harder to introduce a silent
miscompile by adding an transform which forgets about atomicity.
Proposed End Result
Represent atomic loads and stores as normal LoadSDNode or StoreSDNodes.
Analogously to volatility, provide a flag on the node (stored in the
MMO) which indicates whether the operation is atomic. All transforms
updated to check isAtomic if needed.
The advantages of this representations are:
1) Once the audit has been done, it makes it easier to keep atomic and
non-atomic rules in sync.
2) It makes GlobalISEL easier (by eliminating the need for the special
case).
3) Unify patterns flowing through other backend passes (i.e. unordered
atomics and non-atomics shouldn't generate radically different MI
structures)
One open question is whether we do this just for unordered atomics, or
for all atomics. I'd be open to either, but would start with just
unordered to start with either way.
Migration Plan
This would be done on a per-backend basis, and to start with, I'm only
proposing to port X86.
The basic strategy I plan on taking is:
1. introduce infrastructure and a flag for testing
(https://reviews.llvm.org/D66309)
2. Audit uses of isVolatile, and apply isAtomic conservatively*
3. piecemeal conservative* update generic code and x86 backedge code in
individual reviews w/tests for cases which didn't check volatile,
but can be found with inspection
4. flip the (x86) flag at the end (with minimal diffs)
5. Work through todo list identified in (2) and (3) exposing
performance ops
(*) The "conservative" bit here is aimed at minimizing the number of
diffs involved in (4). Ideally, there'd be none. In practice, getting it
down to something reviewable by a human is the actual goal. Note that
there are (currently) no paths which produce LoadSDNode or StoreSDNode
with atomic MMOs, so we don't need to worry about preserving any
behavior there.
We've taken a very similar strategy twice before with success - once at
IR level, and once at the MI level (post ISEL). I'll probably need some
help with some of the ISEL patterns since that's the part I'm not
familiar with.
Thoughts?
Philip
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20190828/600e5448/attachment.html>
More information about the llvm-dev
mailing list