AMDGPU: Cleanup/refactor SIMemoryLegalizer [3]:

[kzhuravl]

Summary

• Refactor SIMemOpInfo's constructors
• Allow construction of NotAtomic SIMemOpInfo

[t-tye]

LGTM except for suggested added comments.

lib/Target/AMDGPU/SIMemoryLegalizer.cpp

195

Suggest comment along the lines:

// Since machine memory operands are optional, if not present have to conservatively assume the worst case of a sequentially consistent system scope atomic.

Same comment for other factory methods.

[kzhuravl]

lib/Target/AMDGPU/SIMemoryLegalizer.cpp

195

Similar comment is in D37397.

lib/Target/AMDGPU/SIMemoryLegalizer.cpp

 #define DEBUG_TYPE "si-memory-legalizer"
 #define PASS_NAME "SI Memory Legalizer"
 
 namespace {
 
 class SIMemOpInfo final {
 private:
   SyncScope::ID SSID = SyncScope::System;
-  AtomicOrdering Ordering = AtomicOrdering::SequentiallyConsistent;
-  AtomicOrdering FailureOrdering = AtomicOrdering::SequentiallyConsistent;
+  AtomicOrdering Ordering = AtomicOrdering::NotAtomic;
+  AtomicOrdering FailureOrdering = AtomicOrdering::NotAtomic;
 
-  SIMemOpInfo() = default;
+  SIMemOpInfo(SyncScope::ID SSID, AtomicOrdering Ordering)
+      : SSID(SSID), Ordering(Ordering) {}
 
-  SIMemOpInfo(SyncScope::ID SSID,
-              AtomicOrdering Ordering,
+  SIMemOpInfo(SyncScope::ID SSID, AtomicOrdering Ordering,
               AtomicOrdering FailureOrdering)
-      : SSID(SSID),
-        Ordering(Ordering),
-        FailureOrdering(FailureOrdering) {}
-
-  SIMemOpInfo(const MachineMemOperand *MMO)
-      : SSID(MMO->getSyncScopeID()),
-        Ordering(MMO->getOrdering()),
-        FailureOrdering(MMO->getFailureOrdering()) {}
+      : SSID(SSID), Ordering(Ordering), FailureOrdering(FailureOrdering) {}
 
 public:
   /// \returns Synchronization scope ID of the machine instruction used to
   /// create this SIMemOpInfo.
   SyncScope::ID getSSID() const {
     return SSID;
   }
   /// \returns Ordering constraint of the machine instruction used to
   /// create this SIMemOpInfo.
   AtomicOrdering getOrdering() const {
     return Ordering;
   }
   /// \returns Failure ordering constraint of the machine instruction used to
   /// create this SIMemOpInfo.
   AtomicOrdering getFailureOrdering() const {
     return FailureOrdering;
   }
 
+  /// \returns True if ordering constraint of the machine instruction used to
+  /// create this SIMemOpInfo is unordered or higher, false otherwise.
+  bool isAtomic() const {
+    return Ordering != AtomicOrdering::NotAtomic;
+  }
+
   /// \returns Load info if \p MI is a load operation, "None" otherwise.
   static Optional<SIMemOpInfo> getLoadInfo(
       const MachineBasicBlock::iterator &MI);
   /// \returns Store info if \p MI is a store operation, "None" otherwise.
   static Optional<SIMemOpInfo> getStoreInfo(
       const MachineBasicBlock::iterator &MI);
   /// \returns Atomic fence info if \p MI is an atomic fence operation,
   /// "None" otherwise.

 /* static */
 Optional<SIMemOpInfo> SIMemOpInfo::getLoadInfo(
     const MachineBasicBlock::iterator &MI) {
   assert(MI->getDesc().TSFlags & SIInstrFlags::maybeAtomic);
 
   if (!(MI->mayLoad() && !MI->mayStore()))
     return None;
   if (!MI->hasOneMemOperand())
-    return SIMemOpInfo();
+    return SIMemOpInfo(SyncScope::System,
+                       AtomicOrdering::SequentiallyConsistent);
 
   const MachineMemOperand *MMO = *MI->memoperands_begin();
-  if (!MMO->isAtomic())
-    return None;
-
-  return SIMemOpInfo(MMO);
+  return SIMemOpInfo(MMO->getSyncScopeID(), MMO->getOrdering());
 }
 
 /* static */
 Optional<SIMemOpInfo> SIMemOpInfo::getStoreInfo(
     const MachineBasicBlock::iterator &MI) {
   assert(MI->getDesc().TSFlags & SIInstrFlags::maybeAtomic);
 
   if (!(!MI->mayLoad() && MI->mayStore()))
     return None;
   if (!MI->hasOneMemOperand())
-    return SIMemOpInfo();
+    return SIMemOpInfo(SyncScope::System,
+                       AtomicOrdering::SequentiallyConsistent);
 
   const MachineMemOperand *MMO = *MI->memoperands_begin();
-  if (!MMO->isAtomic())
-    return None;
-
-  return SIMemOpInfo(MMO);
+  return SIMemOpInfo(MMO->getSyncScopeID(), MMO->getOrdering());
 }
 
 /* static */
 Optional<SIMemOpInfo> SIMemOpInfo::getAtomicFenceInfo(
     const MachineBasicBlock::iterator &MI) {
   assert(MI->getDesc().TSFlags & SIInstrFlags::maybeAtomic);
 
   if (MI->getOpcode() != AMDGPU::ATOMIC_FENCE)
     return None;
 
   SyncScope::ID SSID =
       static_cast<SyncScope::ID>(MI->getOperand(1).getImm());
   AtomicOrdering Ordering =
       static_cast<AtomicOrdering>(MI->getOperand(0).getImm());
-  return SIMemOpInfo(SSID, Ordering, AtomicOrdering::NotAtomic);
+  return SIMemOpInfo(SSID, Ordering);
 }
 
 /* static */
 Optional<SIMemOpInfo> SIMemOpInfo::getAtomicCmpxchgInfo(
     const MachineBasicBlock::iterator &MI) {
   assert(MI->getDesc().TSFlags & SIInstrFlags::maybeAtomic);
 
   if (!(MI->mayLoad() && MI->mayStore()))
     return None;
   if (!MI->hasOneMemOperand())
-    return SIMemOpInfo();
+    return SIMemOpInfo(SyncScope::System,
+                       AtomicOrdering::SequentiallyConsistent,
+                       AtomicOrdering::SequentiallyConsistent);
 
   const MachineMemOperand *MMO = *MI->memoperands_begin();
-  if (!MMO->isAtomic())
-    return None;
-  if (MMO->getFailureOrdering() == AtomicOrdering::NotAtomic)
-    return None;
-
-  return SIMemOpInfo(MMO);
+  return SIMemOpInfo(MMO->getSyncScopeID(), MMO->getOrdering(),
+                     MMO->getFailureOrdering());
 }
 
 /* static */
 Optional<SIMemOpInfo> SIMemOpInfo::getAtomicRmwInfo(
     const MachineBasicBlock::iterator &MI) {
   assert(MI->getDesc().TSFlags & SIInstrFlags::maybeAtomic);
 
   if (!(MI->mayLoad() && MI->mayStore()))
     return None;
   if (!MI->hasOneMemOperand())
-    return SIMemOpInfo();
+    return SIMemOpInfo(SyncScope::System,
+                       AtomicOrdering::SequentiallyConsistent);
 
   const MachineMemOperand *MMO = *MI->memoperands_begin();
-  if (!MMO->isAtomic())
-    return None;
-  if (MMO->getFailureOrdering() != AtomicOrdering::NotAtomic)
-    return None;
-
-  return SIMemOpInfo(MMO);
+  return SIMemOpInfo(MMO->getSyncScopeID(), MMO->getOrdering());
 }
 
 bool SIMemoryLegalizer::insertBufferWbinvl1Vol(MachineBasicBlock::iterator &MI,
                                                bool Before) const {
   MachineBasicBlock &MBB = *MI->getParent();
   DebugLoc DL = MI->getDebugLoc();
 
   if (!Before)
void SIMemoryLegalizer::reportUnknownSynchScope(
   CTX->diagnose(Diag);
 }
 
 bool SIMemoryLegalizer::expandLoad(const SIMemOpInfo &MOI,
                                    MachineBasicBlock::iterator &MI) {
   assert(MI->mayLoad() && !MI->mayStore());
 
   bool Changed = false;
+
+  if (MOI.isAtomic()) {
     if (MOI.getSSID() == SyncScope::System ||
         MOI.getSSID() == MMI->getAgentSSID()) {
       if (MOI.getOrdering() == AtomicOrdering::Acquire ||
           MOI.getOrdering() == AtomicOrdering::SequentiallyConsistent)
         Changed |= setGLC(MI);
 
       if (MOI.getOrdering() == AtomicOrdering::SequentiallyConsistent)
         Changed |= insertWaitcntVmcnt0(MI);
 
       if (MOI.getOrdering() == AtomicOrdering::Acquire ||
           MOI.getOrdering() == AtomicOrdering::SequentiallyConsistent) {
         Changed |= insertWaitcntVmcnt0(MI, false);
         Changed |= insertBufferWbinvl1Vol(MI, false);
       }
 
       return Changed;
     } else if (MOI.getSSID() == SyncScope::SingleThread ||
                MOI.getSSID() == MMI->getWorkgroupSSID() ||
                MOI.getSSID() == MMI->getWavefrontSSID()) {
       return Changed;
     } else {
       reportUnknownSynchScope(MI);
       return Changed;
     }
   }
 
+  return Changed;
+}
+
 bool SIMemoryLegalizer::expandStore(const SIMemOpInfo &MOI,
                                     MachineBasicBlock::iterator &MI) {
   assert(!MI->mayLoad() && MI->mayStore());
 
   bool Changed = false;
+
+  if (MOI.isAtomic()) {
     if (MOI.getSSID() == SyncScope::System ||
         MOI.getSSID() == MMI->getAgentSSID()) {
       if (MOI.getOrdering() == AtomicOrdering::Release ||
           MOI.getOrdering() == AtomicOrdering::SequentiallyConsistent)
         Changed |= insertWaitcntVmcnt0(MI);
 
       return Changed;
     } else if (MOI.getSSID() == SyncScope::SingleThread ||
                MOI.getSSID() == MMI->getWorkgroupSSID() ||
                MOI.getSSID() == MMI->getWavefrontSSID()) {
       return Changed;
     } else {
       reportUnknownSynchScope(MI);
       return Changed;
     }
   }
 
+  return Changed;
+}
+
 bool SIMemoryLegalizer::expandAtomicFence(const SIMemOpInfo &MOI,
                                           MachineBasicBlock::iterator &MI) {
   assert(MI->getOpcode() == AMDGPU::ATOMIC_FENCE);
 
   bool Changed = false;
+
+  if (MOI.isAtomic()) {
     if (MOI.getSSID() == SyncScope::System ||
         MOI.getSSID() == MMI->getAgentSSID()) {
       if (MOI.getOrdering() == AtomicOrdering::Acquire ||
           MOI.getOrdering() == AtomicOrdering::Release ||
           MOI.getOrdering() == AtomicOrdering::AcquireRelease ||
           MOI.getOrdering() == AtomicOrdering::SequentiallyConsistent)
         Changed |= insertWaitcntVmcnt0(MI);
 
       if (MOI.getOrdering() == AtomicOrdering::Acquire ||
           MOI.getOrdering() == AtomicOrdering::AcquireRelease ||
           MOI.getOrdering() == AtomicOrdering::SequentiallyConsistent)
         Changed |= insertBufferWbinvl1Vol(MI);
 
       AtomicPseudoMIs.push_back(MI);
       return Changed;
     } else if (MOI.getSSID() == SyncScope::SingleThread ||
                MOI.getSSID() == MMI->getWorkgroupSSID() ||
                MOI.getSSID() == MMI->getWavefrontSSID()) {
       AtomicPseudoMIs.push_back(MI);
       return Changed;
     } else {
       reportUnknownSynchScope(MI);
       return Changed;
     }
   }
 
+  return Changed;
+}
+
 bool SIMemoryLegalizer::expandAtomicCmpxchg(const SIMemOpInfo &MOI,
                                             MachineBasicBlock::iterator &MI) {
   assert(MI->mayLoad() && MI->mayStore());
 
   bool Changed = false;
+
+  if (MOI.isAtomic()) {
     if (MOI.getSSID() == SyncScope::System ||
         MOI.getSSID() == MMI->getAgentSSID()) {
       if (MOI.getOrdering() == AtomicOrdering::Release ||
           MOI.getOrdering() == AtomicOrdering::AcquireRelease ||
           MOI.getOrdering() == AtomicOrdering::SequentiallyConsistent ||
           MOI.getFailureOrdering() == AtomicOrdering::SequentiallyConsistent)
         Changed |= insertWaitcntVmcnt0(MI);
 
       if (MOI.getOrdering() == AtomicOrdering::Acquire ||
           MOI.getOrdering() == AtomicOrdering::AcquireRelease ||
           MOI.getOrdering() == AtomicOrdering::SequentiallyConsistent ||
           MOI.getFailureOrdering() == AtomicOrdering::Acquire ||
           MOI.getFailureOrdering() == AtomicOrdering::SequentiallyConsistent) {
         Changed |= insertWaitcntVmcnt0(MI, false);
         Changed |= insertBufferWbinvl1Vol(MI, false);
       }
 
       return Changed;
     } else if (MOI.getSSID() == SyncScope::SingleThread ||
                MOI.getSSID() == MMI->getWorkgroupSSID() ||
                MOI.getSSID() == MMI->getWavefrontSSID()) {
       Changed |= setGLC(MI);
       return Changed;
     } else {
       reportUnknownSynchScope(MI);
       return Changed;
     }
   }
 
+  return Changed;
+}
+
 bool SIMemoryLegalizer::expandAtomicRmw(const SIMemOpInfo &MOI,
                                         MachineBasicBlock::iterator &MI) {
   assert(MI->mayLoad() && MI->mayStore());
 
   bool Changed = false;
+
+  if (MOI.isAtomic()) {
     if (MOI.getSSID() == SyncScope::System ||
         MOI.getSSID() == MMI->getAgentSSID()) {
       if (MOI.getOrdering() == AtomicOrdering::Release ||
           MOI.getOrdering() == AtomicOrdering::AcquireRelease ||
           MOI.getOrdering() == AtomicOrdering::SequentiallyConsistent)
         Changed |= insertWaitcntVmcnt0(MI);
 
       if (MOI.getOrdering() == AtomicOrdering::Acquire ||
           MOI.getOrdering() == AtomicOrdering::AcquireRelease ||
           MOI.getOrdering() == AtomicOrdering::SequentiallyConsistent) {
         Changed |= insertWaitcntVmcnt0(MI, false);
         Changed |= insertBufferWbinvl1Vol(MI, false);
       }
 
       return Changed;
     } else if (MOI.getSSID() == SyncScope::SingleThread ||
                MOI.getSSID() == MMI->getWorkgroupSSID() ||
                MOI.getSSID() == MMI->getWavefrontSSID()) {
       Changed |= setGLC(MI);
       return Changed;
     } else {
       reportUnknownSynchScope(MI);
       return Changed;
     }
   }
 
+  return Changed;
+}
+
 bool SIMemoryLegalizer::runOnMachineFunction(MachineFunction &MF) {
   bool Changed = false;
   const SISubtarget &ST = MF.getSubtarget<SISubtarget>();
   const IsaInfo::IsaVersion IV = IsaInfo::getIsaVersion(ST.getFeatureBits());
 
   CTX = &MF.getFunction()->getContext();
   MMI = &MF.getMMI().getObjFileInfo<AMDGPUMachineModuleInfo>();
   TII = ST.getInstrInfo();