Replace Atomic Fence with GenISA_source_value

Replace Atomic Fence with GenISA_source_value

Author: pthammin

IGC/Compiler/CISACodeGen/EmitVISAPass.cpp

@@ -8575,6 +8575,8 @@ void EmitPass::EmitGenIntrinsicMessage(llvm::GenIntrinsicInst *inst) {
   case GenISAIntrinsic::GenISA_source_value: {
     m_encoder->Copy(m_currShader->GetNULL(), GetSymbol(inst->getOperand(0)));
     m_encoder->Push();
+    m_encoder->Fence(false, false, false, false, false, false, false, true);
+    m_encoder->Push();
     break;
   }
   case GenISAIntrinsic::GenISA_movcr: {

IGC/Compiler/Optimizer/SynchronizationObjectCoalescing.cpp

@@ -17,6 +17,8 @@ SPDX-License-Identifier: MIT
 #include "Compiler/IGCPassSupport.h"
 #include "SynchronizationObjectCoalescing.hpp"
 #include "visa_igc_common_header.h"
+#include "llvm/IR/IRBuilder.h"
+#include "llvm/Analysis/CFG.h"
 #include <utility>
 #include <map>
 
@@ -285,6 +287,9 @@ class SynchronizationObjectCoalescing : public llvm::FunctionPass {
       static_cast<SynchronizationCaseMask>(WriteSyncRead | WriteSyncWrite | AtomicSyncRead | AtomicSyncWrite |
                                            WriteSyncAtomic | ReadSyncAtomic | ReadSyncWrite | AtomicSyncAtomic);
 
+  ////////////////////////////////////////////////////////////////////////
+  void CreateSourceValueInst(std::vector<llvm::Instruction *> &pAtomicInstToBeSourced, llvm::Instruction *pFenceInst);
+
   ////////////////////////////////////////////////////////////////////////
   void EraseRedundantInst(llvm::Instruction *pInst);
 
@@ -327,6 +332,7 @@ class SynchronizationObjectCoalescing : public llvm::FunctionPass {
 
   ////////////////////////////////////////////////////////////////////////
   bool IsRequiredForAtomicOperationsOrdering(const llvm::Instruction *pSourceInst,
+                                             std::vector<llvm::Instruction *> &pAtomicInstToBeSourced,
                                              bool onlyGlobalAtomics = false) const;
 
   ////////////////////////////////////////////////////////////////////////
@@ -440,6 +446,7 @@ class SynchronizationObjectCoalescing : public llvm::FunctionPass {
   std::vector<llvm::Instruction *> m_LscMemoryFences;
   std::vector<llvm::Instruction *> m_UntypedMemoryFences;
   std::vector<llvm::Instruction *> m_ThreadGroupBarriers;
+  std::unordered_set<llvm::Instruction *> m_SourcedAtomicInstructions;
 
   // this variable holds a mapping from a basic block to its memory instructions ordered by their occurrences in it
   // (the initial index of line of this basic block - the number of instructions preceding an instruction it its basic
@@ -538,6 +545,107 @@ bool SynchronizationObjectCoalescing::ProcessFunction() {
   return FindRedundancies();
 }
 
+// Referenced from MemoryModelPass
+inline PHINode *FindDominatingPhi(DominatorTree &DT, Instruction *def, BasicBlock *postDominator) {
+  IGC_ASSERT(def->getParent() != postDominator);
+  IGC_ASSERT(!DT.dominates(def, postDominator));
+  SmallPtrSet<PHINode *, 8> seen;
+  SmallVector<User *, 8> worklist(def->users());
+  while (!worklist.empty()) {
+    PHINode *phi = dyn_cast<PHINode>(worklist.pop_back_val());
+    if (phi == nullptr || seen.count(phi) > 0) {
+      continue;
+    }
+    if (phi->getParent() == postDominator || DT.dominates(phi, postDominator)) {
+      return phi;
+    }
+    seen.insert(phi);
+  }
+  return nullptr;
+}
+
+////////////////////////////////////////////////////////////////////////
+/// @brief Fence Instruction responsible for only ordering of atomic Instructions
+/// can be replaced with Source Value Intrinsic which will still maintain
+/// the order of Instructions
+void SynchronizationObjectCoalescing::CreateSourceValueInst(std::vector<llvm::Instruction *> &pAtomicInstToBeSourced,
+                                                            llvm::Instruction *pFenceInst) {
+  IGC_ASSERT(pAtomicInstToBeSourced.size() > 0);
+  // reversing the list to source the atomic instructions in the order
+  reverse(pAtomicInstToBeSourced.begin(), pAtomicInstToBeSourced.end());
+  Function *funcPtr = GenISAIntrinsic::getDeclaration(pFenceInst->getModule(), GenISAIntrinsic::GenISA_source_value);
+  BasicBlock *fenceBB = pFenceInst->getParent();
+
+  Function *F = pAtomicInstToBeSourced[0]->getFunction();
+  DominatorTree DT(*F);
+  PostDominatorTree PDT(*F);
+  LoopInfo LI(DT);
+
+  for (llvm::Instruction *atomicInst : pAtomicInstToBeSourced) {
+    // Making sure that the Fence Inst is potentially reachable from the atomic Instruction.
+    if (!isPotentiallyReachable(atomicInst, pFenceInst, nullptr, &DT, &LI)) {
+      continue;
+    }
+
+    BasicBlock *atomicBB = atomicInst->getParent();
+    BasicBlock *fenceDominator = fenceBB;
+    Instruction *insertPoint = atomicBB->getTerminator();
+    Value *sourceVal = cast<GenIntrinsicInst>(atomicInst);
+
+    // TODO: Determining Insert point can be improved which can postpone the source value intrinsic as long as possible.
+    // Similar analysis is done in FindOptimalInsertPoints() in ApplyCacheControls.cpp
+
+    // Check if fence Instruction BB post dominates atomic Instruction BB
+    // Else find the BB that is a predecessor of fence BB and post dominates atomic BB.
+    // If we don't find one, then the insert point is near the terminator of atomic BB
+    while (fenceDominator && fenceDominator != atomicBB) {
+      if (PDT.dominates(fenceDominator, atomicBB)) {
+        // If fence instruction is in same BB, then use fence as insert point
+        // Else use the terminator of fenceDominator as insert point
+        insertPoint = fenceBB == fenceDominator ? pFenceInst : fenceDominator->getTerminator();
+        // It's possible that the atomic instruction does not dominate
+        // the post-dominator, find a PHI user of the atomic instruction
+        // that dominates the post-dominator.
+        if (!DT.dominates(atomicBB, fenceDominator)) {
+          PHINode *phi = FindDominatingPhi(DT, atomicInst, fenceDominator);
+          if (phi) {
+            sourceVal = phi;
+          } else {
+            // Fallback to inserting the source value in the basic
+            // block with the atomic instruction.
+            insertPoint = atomicBB->getTerminator();
+          }
+        }
+        break;
+      }
+      fenceDominator = fenceDominator->getSinglePredecessor();
+    }
+    // If Fence is present in same BB as atomic, then insert at Fence Instruction
+    if (fenceBB == atomicBB) {
+      insertPoint = pFenceInst;
+    }
+
+    IRBuilder<> builder(insertPoint);
+    Type *sourceValType = sourceVal->getType();
+
+    // Source value intrinsic accepts only i32.
+    if (sourceValType->isIntegerTy()) {
+      sourceVal = builder.CreateZExtOrTrunc(sourceVal, builder.getInt32Ty());
+    } else if (sourceValType->isFloatingPointTy()) {
+      if (sourceValType->isFloatTy()) {
+        sourceVal = builder.CreateBitCast(sourceVal, builder.getInt32Ty());
+      } else {
+        sourceVal = builder.CreateFPToUI(sourceVal, builder.getInt32Ty());
+      }
+    } else {
+      IGC_ASSERT_MESSAGE(0, "Unexpected type");
+    }
+
+    builder.CreateCall(funcPtr, {sourceVal});
+    m_SourcedAtomicInstructions.insert(atomicInst);
+  }
+}
+
 ////////////////////////////////////////////////////////////////////////
 void SynchronizationObjectCoalescing::EraseRedundantInst(llvm::Instruction *pInst) {
   bool isFence = IsFenceOperation(pInst);
@@ -740,7 +848,13 @@ bool SynchronizationObjectCoalescing::FindRedundancies() {
           }
           SynchronizationCaseMask referenceSyncCaseMask = GetStrictSynchronizationMask(pInst);
           bool isObligatory = (syncCaseMask & referenceSyncCaseMask) != 0;
-          isObligatory |= IsRequiredForAtomicOperationsOrdering(pInst, true /*onlyGlobalAtomics*/);
+
+          std::vector<llvm::Instruction *> atomicInstToBeSourced;
+          if (!isObligatory) {
+            isObligatory =
+                IsRequiredForAtomicOperationsOrdering(pInst, atomicInstToBeSourced, true /*onlyGlobalAtomics*/);
+          }
+
           bool verifyUnsynchronizedInstructions = IsFenceOperation(pInst);
           verifyUnsynchronizedInstructions &= (!isObligatory || syncCaseMask == ReadSyncWrite);
 
@@ -767,6 +881,9 @@ bool SynchronizationObjectCoalescing::FindRedundancies() {
 #if _DEBUG
             RegisterRedundancyExplanation(pInst, ExplanationEntry::GlobalMemoryRedundancy);
 #endif // _DEBUG
+            if (IGC_IS_FLAG_ENABLED(ReplaceAtomicFenceWithSourceValue) && atomicInstToBeSourced.size() > 0) {
+              CreateSourceValueInst(atomicInstToBeSourced, const_cast<Instruction *>(pInst));
+            }
             EraseRedundantGlobalScope(pInst);
             isModified = true;
             SetLocalMemoryInstructionMask();
@@ -831,7 +948,12 @@ bool SynchronizationObjectCoalescing::FindRedundancies() {
           GetSynchronizationMaskForAllResources(localForwardMemoryInstructionMask, localBackwardMemoryInstructionMask);
       SynchronizationCaseMask referenceSyncCaseMask = GetStrictSynchronizationMask(pInst);
       bool isObligatory = (syncCaseMask & referenceSyncCaseMask) != 0;
-      isObligatory |= IsRequiredForAtomicOperationsOrdering(pInst);
+
+      std::vector<llvm::Instruction *> atomicInstToBeSourced;
+      if (!isObligatory) {
+        isObligatory = IsRequiredForAtomicOperationsOrdering(pInst, atomicInstToBeSourced);
+      }
+
       bool verifyUnsynchronizedInstructions = IsFenceOperation(pInst);
       verifyUnsynchronizedInstructions &= (!isObligatory || syncCaseMask == ReadSyncWrite);
 
@@ -847,6 +969,9 @@ bool SynchronizationObjectCoalescing::FindRedundancies() {
 #if _DEBUG
         RegisterRedundancyExplanation(pInst, ExplanationEntry::StrictRedundancy);
 #endif // _DEBUG
+        if (IGC_IS_FLAG_ENABLED(ReplaceAtomicFenceWithSourceValue) && atomicInstToBeSourced.size() > 0) {
+          CreateSourceValueInst(atomicInstToBeSourced, const_cast<Instruction *>(pInst));
+        }
         EraseRedundantInst(pInst);
         isModified = true;
       }
@@ -1731,8 +1856,9 @@ SynchronizationObjectCoalescing::GetUnsynchronizedForwardInstructionMask(const l
 /// operations present before the fence (in program order)
 /// @param pSourceInst the source synchronization instruction
 /// @param onlyGlobalAtomics check only TGM and UGM atomic operations
-bool SynchronizationObjectCoalescing::IsRequiredForAtomicOperationsOrdering(const llvm::Instruction *pSourceInst,
-                                                                            bool onlyGlobalAtomics /*= false*/) const {
+bool SynchronizationObjectCoalescing::IsRequiredForAtomicOperationsOrdering(
+    const llvm::Instruction *pSourceInst, std::vector<llvm::Instruction *> &pAtomicInstToBeSourced,
+    bool onlyGlobalAtomics /*= false*/) const {
   if (!IsFenceOperation(pSourceInst)) {
     // Not a fence, nothing to check
     return false;
@@ -1782,6 +1908,10 @@ bool SynchronizationObjectCoalescing::IsRequiredForAtomicOperationsOrdering(cons
     {
       isPotentiallyUnsynchronizedAtomic = false;
       // Lambda that checks if a fence operation synchronizes the atomic operation.
+      // This can be improved to detect the users of atomic instruction and end the search for fences once we find the
+      // user. This user is essentially same as Source Value Intrinsic, however it can be reordered in visa affecting
+      // the execution order of atomic instructions. If we can find a way to treat this user as a special instruction
+      // and avoid reordering, we can skip creating new source value instruction.
       std::function<bool(const llvm::Instruction *)> IsBoundaryInst = [this, &atomicPointerMemoryInstructionMask,
                                                                        &isPotentiallyUnsynchronizedAtomic,
                                                                        pSourceInst](const llvm::Instruction *pInst) {
@@ -1840,7 +1970,22 @@ bool SynchronizationObjectCoalescing::IsRequiredForAtomicOperationsOrdering(cons
       if (!substituteFenceFound) {
         // Found an atomic operation that requires the source fence
         // instruction for correct memory ordering.
-        return true;
+
+        // If ReplaceAtomicFenceWithSourceValue is true, we can replace this fence with GenISA_source_value.
+        // This will source the atomic instruction and still maintains the order of atomic instructions.
+        // Else return true marking the fence instruction as Obligatory.
+
+        if (IGC_IS_FLAG_ENABLED(ReplaceAtomicFenceWithSourceValue)) {
+          // If a previous fence was replaced with source value intrinsic, GetVisibleMemoryInstructions will add the
+          // same atomic instruction again for the next fence resulting in multiple source value intrinsics but we need
+          // it to be sourced only once. Hence we check if it was already sourced previously. Continues to check all
+          // valid atomic Instructions to be sourced.
+          if (m_SourcedAtomicInstructions.find(const_cast<Instruction *>(pInst)) == m_SourcedAtomicInstructions.end()) {
+            pAtomicInstToBeSourced.push_back(const_cast<Instruction *>(pInst));
+          }
+        } else {
+          return true;
+        }
       }
     }
   }
@@ -2002,6 +2147,7 @@ void SynchronizationObjectCoalescing::InvalidateMembers() {
   m_OrderedFenceInstructionsInBasicBlockCache.clear();
   m_OrderedBarrierInstructionsInBasicBlockCache.clear();
   m_BasicBlockMemoryInstructionMaskCache.clear();
+  m_SourcedAtomicInstructions.clear();
 #if _DEBUG
   m_ExplanationEntries.clear();
 #endif // _DEBUG

IGC/common/igc_flags.h

@@ -433,6 +433,9 @@ DECLARE_IGC_REGKEY(
     "The mask is casted to IGC::SyncInstMask and informs which synchronization objects should not be coalesced. Note "
     "that synchronization objects classified in multiple types are not disabled if any bit describing them is off.",
     true)
+DECLARE_IGC_REGKEY(bool, ReplaceAtomicFenceWithSourceValue, true,
+    "Fences are required to maintain the order of atomic memory instructions. This flag will replace the fence with "
+    "GenISA_source_value intrinsic which sources the result of atomic operation and still maintains the order.", true)
 DECLARE_IGC_REGKEY(bool, UnrollLoopForCodeSizeOnly, false,
                    "Only unroll the loop if it can reduce program size/register pressure. Ignore all other threshold "
                    "setting but still enable EnablePromoteLoopUnrollwithAlloca due to high likelyhood to reduce size.",