[PATCH] D69219: [SelectionDAG] Enable lowering unordered atomics loads w/LoadSDNode (and stores w/StoreSDNode) by default

Tue Oct 29 15:08:45 PDT 2019

reames closed this revision.
reames added a comment.

This landed.  For some reason the normal auto-close didn't trigger.

Author: Philip Reames
Date: 2019-10-29T12:46:24-07:00
New Revision: 2460989eabb8adca4b7973aa23997b5ec832219b <https://reviews.llvm.org/rG2460989eabb8adca4b7973aa23997b5ec832219b>

URL: https://github.com/llvm/llvm-project/commit/2460989eabb8adca4b7973aa23997b5ec832219b
DIFF: https://github.com/llvm/llvm-project/commit/2460989eabb8adca4b7973aa23997b5ec832219b.diff

LOG: [SelectionDAG] Enable lowering unordered atomics loads w/LoadSDNode (and stores w/StoreSDNode) by default

Enable the new SelectionDAG representation for unordered loads and stores introduced in r371441 by default.  As a reminder, the new lowering changes the representation of an unordered atomic load from an AtomicSDNode - which is essentially a black box which gets passed through without combines messing with it - to a LoadSDNode w/a atomic marker on the MMO. The later parallels the way we handle volatiles, and I've audited the code to ensure that every location which checks one checks the other.

This has been fairly heavily fuzzed, and I examined diffs in a reasonable large corpus of assembly by hand, so I'm reasonable sure this is correct for the common case.  Late in the review for this, it was discovered that I hadn't correctly handled cases which could be legalized into CAS operations.  This points out that there's a strong bias in the IR of the frontend I'm working with towards only legal atomics.  If there are problems with this patch, the most likely area will be legalization.

Differential Revision: https://reviews.llvm.org/D69219

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69219/new/

https://reviews.llvm.org/D69219