<html>
<head>
<meta http-equiv="content-type" content="text/html; charset=UTF-8">
</head>
<body text="#000000" bgcolor="#FFFFFF">
<p>I have a set of changes out for review which are possibly note
worthy, and backend contributors may wish to be aware of. <br>
</p>
<p>TLDR: atomic loads as normal LoadSDNodes w/an "isAtomic" flag.</p>
<p><br>
</p>
<p>Background</p>
<p>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.</p>
<p><br>
</p>
<p>Proposed End Result<br>
</p>
<p>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. <br>
</p>
<p>The advantages of this representations are:<br>
1) Once the audit has been done, it makes it easier to keep atomic
and non-atomic rules in sync. <br>
2) It makes GlobalISEL easier (by eliminating the need for the
special case).<br>
3) Unify patterns flowing through other backend passes (i.e.
unordered atomics and non-atomics shouldn't generate radically
different MI structures)<br>
</p>
<p>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. <br>
</p>
<p><br>
</p>
<p>Migration Plan</p>
<p>This would be done on a per-backend basis, and to start with, I'm
only proposing to port X86. <br>
</p>
<p>The basic strategy I plan on taking is:</p>
<ol class="remarkup-list">
<li class="remarkup-list-item">introduce infrastructure and a flag
for testing (<a class="moz-txt-link-freetext" href="https://reviews.llvm.org/D66309">https://reviews.llvm.org/D66309</a>)</li>
<li class="remarkup-list-item">Audit uses of isVolatile, and apply
isAtomic conservatively*</li>
<li class="remarkup-list-item">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</li>
<li class="remarkup-list-item">flip the (x86) flag at the end
(with minimal diffs)</li>
<li class="remarkup-list-item">Work through todo list identified
in (2) and (3) exposing performance ops</li>
</ol>
<p>(*) 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.</p>
<p>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.</p>
<p><br>
</p>
<p>Thoughts?</p>
<p>Philip<br>
</p>
</body>
</html>