[SelectionDAG] Fix D66309: Allow unordered atomics to have some optimizations done for them

[AtariDreams]

No description provided.

[llvmbot]

@llvm/pr-subscribers-backend-powerpc

@llvm/pr-subscribers-llvm-selectiondag

Author: AtariDreams (AtariDreams)

Changes

---

Full diff: https://github.com/llvm/llvm-project/pull/85589.diff

1 Files Affected:

• (modified) llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp (+10-17)

diff --git a/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp b/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
index 5eb53d57c9c2bf..6b6c88e5f16ba7 100644
--- a/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
@@ -8701,10 +8701,9 @@ SDValue DAGCombiner::mergeTruncStores(StoreSDNode *N) {
     return SDValue();
 
   // We only handle merging simple stores of 1-4 bytes.
-  // TODO: Allow unordered atomics when wider type is legal (see D66309)
   EVT MemVT = N->getMemoryVT();
   if (!(MemVT == MVT::i8 || MemVT == MVT::i16 || MemVT == MVT::i32) ||
-      !N->isSimple() || N->isIndexed())
+      (!N->isUnordered() && isTypeLegal(MemVT)) || N->isIndexed())
     return SDValue();
 
   // Collect all of the stores in the chain, upto the maximum store width (i64).
@@ -18651,8 +18650,7 @@ SDValue DAGCombiner::visitLOAD(SDNode *N) {
   // If load is not volatile and there are no uses of the loaded value (and
   // the updated indexed value in case of indexed loads), change uses of the
   // chain value into uses of the chain input (i.e. delete the dead load).
-  // TODO: Allow this for unordered atomics (see D66309)
-  if (LD->isSimple()) {
+  if (LD->isUnordered()) {
     if (N->getValueType(1) == MVT::Other) {
       // Unindexed loads.
       if (!N->hasAnyUseOfValue(0)) {
@@ -21105,13 +21103,12 @@ SDValue DAGCombiner::visitSTORE(SDNode *N) {
   // If this is a load followed by a store to the same location, then the store
   // is dead/noop. Peek through any truncates if canCombineTruncStore failed.
   // TODO: Add big-endian truncate support with test coverage.
-  // TODO: Can relax for unordered atomics (see D66309)
   SDValue TruncVal = DAG.getDataLayout().isLittleEndian()
                          ? peekThroughTruncates(Value)
                          : Value;
   if (auto *Ld = dyn_cast<LoadSDNode>(TruncVal)) {
     if (Ld->getBasePtr() == Ptr && ST->getMemoryVT() == Ld->getMemoryVT() &&
-        ST->isUnindexed() && ST->isSimple() &&
+        ST->isUnindexed() && ST->isUnordered() &&
         Ld->getAddressSpace() == ST->getAddressSpace() &&
         // There can't be any side effects between the load and store, such as
         // a call or store.
@@ -21125,10 +21122,9 @@ SDValue DAGCombiner::visitSTORE(SDNode *N) {
   if (SDValue NewST = replaceStoreOfInsertLoad(ST))
     return NewST;
 
-  // TODO: Can relax for unordered atomics (see D66309)
   if (StoreSDNode *ST1 = dyn_cast<StoreSDNode>(Chain)) {
-    if (ST->isUnindexed() && ST->isSimple() &&
-        ST1->isUnindexed() && ST1->isSimple()) {
+    if (ST->isUnindexed() && ST->isUnordered() && ST1->isUnindexed() &&
+        ST1->isUnordered()) {
       if (OptLevel != CodeGenOptLevel::None && ST1->getBasePtr() == Ptr &&
           ST1->getValue() == Value && ST->getMemoryVT() == ST1->getMemoryVT() &&
           ST->getAddressSpace() == ST1->getAddressSpace()) {
@@ -21246,8 +21242,7 @@ SDValue DAGCombiner::visitLIFETIME_END(SDNode *N) {
       break;
     case ISD::STORE: {
       StoreSDNode *ST = dyn_cast<StoreSDNode>(Chain);
-      // TODO: Can relax for unordered atomics (see D66309)
-      if (!ST->isSimple() || ST->isIndexed())
+      if (!ST->isUnordered() || ST->isIndexed())
         continue;
       const TypeSize StoreSize = ST->getMemoryVT().getStoreSize();
       // The bounds of a scalable store are not known until runtime, so this
@@ -27934,8 +27929,7 @@ void DAGCombiner::GatherAllAliases(SDNode *N, SDValue OriginalChain,
   SmallPtrSet<SDNode *, 16> Visited;  // Visited node set.
 
   // Get alias information for node.
-  // TODO: relax aliasing for unordered atomics (see D66309)
-  const bool IsLoad = isa<LoadSDNode>(N) && cast<LoadSDNode>(N)->isSimple();
+  const bool IsLoad = isa<LoadSDNode>(N) && cast<LoadSDNode>(N)->isUnordered();
 
   // Starting off.
   Chains.push_back(OriginalChain);
@@ -27951,9 +27945,8 @@ void DAGCombiner::GatherAllAliases(SDNode *N, SDValue OriginalChain,
     case ISD::LOAD:
     case ISD::STORE: {
       // Get alias information for C.
-      // TODO: Relax aliasing for unordered atomics (see D66309)
       bool IsOpLoad = isa<LoadSDNode>(C.getNode()) &&
-                      cast<LSBaseSDNode>(C.getNode())->isSimple();
+                      cast<LSBaseSDNode>(C.getNode())->isUnordered();
       if ((IsLoad && IsOpLoad) || !mayAlias(N, C.getNode())) {
         // Look further up the chain.
         C = C.getOperand(0);
@@ -28115,8 +28108,8 @@ bool DAGCombiner::parallelizeChainedStores(StoreSDNode *St) {
     // If the chain has more than one use, then we can't reorder the mem ops.
     if (!SDValue(Chain, 0)->hasOneUse())
       break;
-    // TODO: Relax for unordered atomics (see D66309)
-    if (!Chain->isSimple() || Chain->isIndexed())
+
+    if (!Chain->isUnordered() || Chain->isIndexed())
       break;
 
     // Find the base pointer and offset for this memory node.

[github-actions]

✅ With the latest revision this PR passed the C/C++ code formatter.

[arsenm]

This should be split into different patches for the different transforms, with appropriate tests for each. The description should also be more specific; i.e. the part that's tested is for merging unordered atomic stress.

[arsenm]

Use named values in tests

[arsenm]

This is untested

[arsenm]

loads aren't tested

[arsenm]

Also need negative tests with other orderings