Replace Atomic Fence with GenISA_source_value try 3

Replace Atomic Fence with GenISA_source_value try 3

Author: pthammin

IGC/Compiler/CISACodeGen/EmitVISAPass.cpp

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

IGC/Compiler/CISACodeGen/Platform.hpp

@@ -1356,5 +1356,9 @@ class CPlatform {
   bool enableLscSamplerRouting() const {
       return isCoreChildOf(IGFX_XE3_CORE);
   }
+
+  bool enableReplaceAtomicFenceWithSourceValue() const {
+    return IGC_IS_FLAG_ENABLED(ReplaceAtomicFenceWithSourceValue);
+  }
 };
 } // namespace IGC

IGC/Compiler/Optimizer/SynchronizationObjectCoalescing.cpp

@@ -17,6 +17,9 @@ 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 "llvm/Analysis/LoopInfo.h"
 #include <utility>
 #include <map>
 
@@ -285,6 +288,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 +333,7 @@ class SynchronizationObjectCoalescing : public llvm::FunctionPass {
 
   ////////////////////////////////////////////////////////////////////////
   bool IsRequiredForAtomicOperationsOrdering(const llvm::Instruction *pSourceInst,
+                                             std::vector<llvm::Instruction *> &pAtomicInstToBeSourced,
                                              bool onlyGlobalAtomics = false) const;
 
   ////////////////////////////////////////////////////////////////////////
@@ -440,6 +447,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
@@ -466,6 +474,7 @@ class SynchronizationObjectCoalescing : public llvm::FunctionPass {
   InstMaskLookupTableT m_InstMaskLookupTable;
 
   llvm::Function *m_CurrentFunction = nullptr;
+  const CodeGenContext *ctx = nullptr;
   bool m_HasIndependentSharedMemoryFenceFunctionality = false;
   bool m_HasTypedMemoryFenceFunctionality = false;
   bool m_HasUrbFenceFunctionality = false;
@@ -507,7 +516,7 @@ SynchronizationObjectCoalescing::SynchronizationObjectCoalescing() : llvm::Funct
 ////////////////////////////////////////////////////////////////////////
 bool SynchronizationObjectCoalescing::runOnFunction(llvm::Function &F) {
   m_CurrentFunction = &F;
-  const CodeGenContext *const ctx = getAnalysis<CodeGenContextWrapper>().getCodeGenContext();
+  ctx = getAnalysis<CodeGenContextWrapper>().getCodeGenContext();
   const ModuleMetaData *const md = ctx->getModuleMetaData();
   m_HasIndependentSharedMemoryFenceFunctionality =
       !ctx->platform.hasSLMFence() ||
@@ -538,6 +547,131 @@ bool SynchronizationObjectCoalescing::ProcessFunction() {
   return FindRedundancies();
 }
 
+// Referenced from MemoryModelPass
+static 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) {
+  if (pAtomicInstToBeSourced.size() == 0) {
+    return;
+  }
+  // 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);
+
+  std::unordered_set<llvm::Value *> m_SourcedValues;
+
+  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;
+    }
+
+    // In few shaders, the atomic instructions were already sourced before unification. Skip creating one in this case
+    bool atomicSourced = false;
+    for (User *U : atomicInst->users()) {
+      if (GenIntrinsicInst *Inst = dyn_cast<GenIntrinsicInst>(U)) {
+        // TODO: If dominates fail, then either move the source_value to BB that dominates fence and post dominates
+        // atomic or delete them and let the below code create new ones. Requires further testing.
+        if (Inst->getIntrinsicID() == GenISAIntrinsic::GenISA_source_value &&
+            DT.dominates(Inst->getParent(), fenceBB)) {
+          atomicSourced = true;
+          break;
+        }
+      }
+    }
+    if (!atomicSourced) {
+      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();
+      }
+      // A Fence can synchronize a phi node which is coming from atomic instructions in different branches.
+      // Avoid creating a duplicate source value intrinsic in this case.
+      if (m_SourcedValues.find(sourceVal) == m_SourcedValues.end()) {
+        m_SourcedValues.insert(sourceVal);
+        // 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 +874,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 +907,9 @@ bool SynchronizationObjectCoalescing::FindRedundancies() {
 #if _DEBUG
             RegisterRedundancyExplanation(pInst, ExplanationEntry::GlobalMemoryRedundancy);
 #endif // _DEBUG
+            if (ctx->platform.enableReplaceAtomicFenceWithSourceValue()) {
+              CreateSourceValueInst(atomicInstToBeSourced, const_cast<Instruction *>(pInst));
+            }
             EraseRedundantGlobalScope(pInst);
             isModified = true;
             SetLocalMemoryInstructionMask();
@@ -831,7 +974,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 +995,9 @@ bool SynchronizationObjectCoalescing::FindRedundancies() {
 #if _DEBUG
         RegisterRedundancyExplanation(pInst, ExplanationEntry::StrictRedundancy);
 #endif // _DEBUG
+        if (ctx->platform.enableReplaceAtomicFenceWithSourceValue()) {
+          CreateSourceValueInst(atomicInstToBeSourced, const_cast<Instruction *>(pInst));
+        }
         EraseRedundantInst(pInst);
         isModified = true;
       }
@@ -1731,8 +1882,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 +1934,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) {
@@ -1832,6 +1988,11 @@ bool SynchronizationObjectCoalescing::IsRequiredForAtomicOperationsOrdering(cons
       for (llvm::BasicBlock::const_iterator it = ++pSourceInst->getIterator(); it != pSourceInst->getParent()->end();
            ++it) {
         const llvm::Instruction *pCurrInst = &(*it);
+        // If we encounter an atomic instruction after pSourceInst (Source Fence), then the fence is required to execute
+        // pInst (initial atomic) before pCurrInst (current atomic)
+        if (IsAtomicOperation(pCurrInst)) {
+          break;
+        }
         if (IsFenceOperation(pCurrInst) && IsSubstituteInstruction(pCurrInst, pSourceInst)) {
           substituteFenceFound = true;
           break;
@@ -1840,7 +2001,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 (ctx->platform.enableReplaceAtomicFenceWithSourceValue()) {
+          // 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;
+        }
       }
     }
   }
@@ -1866,6 +2042,9 @@ SynchronizationObjectCoalescing::GetInstructionMask(const std::vector<const llvm
 /// and it means that this instruction must be equal or weaker than the evaluated one.
 bool SynchronizationObjectCoalescing::IsSubstituteInstruction(const llvm::Instruction *pEvaluatedInst,
                                                              const llvm::Instruction *pReferenceInst) const {
+  if (pEvaluatedInst == pReferenceInst) {
+    return false;
+  }
   if (IsUntypedMemoryFenceOperation(pEvaluatedInst) && IsUntypedMemoryFenceOperation(pReferenceInst)) {
     const uint32_t commitEnableArg = 0;
     const uint32_t L3FlushRWDataArg = 1;
@@ -2002,6 +2181,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

@@ -440,6 +440,11 @@ 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, false,
+    "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 PromoteLoopUnrollwithAlloca due to high likelyhood to reduce size.",