Force private memory to global buffer when generic load/store are present (off for now)

weiyu-chen · igcbot · commit 4bff40d4d0e6 · 2021-04-07T08:23:50.000+02:00
diff --git a/IGC/Compiler/CISACodeGen/CheckInstrTypes.cpp b/IGC/Compiler/CISACodeGen/CheckInstrTypes.cpp
@@ -103,6 +103,7 @@ CheckInstrTypes::CheckInstrTypes(IGC::SInstrTypes* instrList) : FunctionPass(ID)
     instrList->sampleCmpToDiscardOptimizationPossible = false;
     instrList->sampleCmpToDiscardOptimizationSlot = 0;
     instrList->hasPullBary = false;
+    instrList->hasDynamicGenericLoadStore = false;
 }
 
 void CheckInstrTypes::SetLoopFlags(Function& F)
diff --git a/IGC/Compiler/CISACodeGen/Platform.hpp b/IGC/Compiler/CISACodeGen/Platform.hpp
@@ -461,6 +461,11 @@ unsigned getSlmSizePerSsOrDss() const
     return slmSizePerSsOrDss;
 }
 
+bool canForcePrivateToGlobal() const
+{
+    return m_platformInfo.eRenderCoreFamily >= IGFX_GEN9_CORE && IGC_IS_FLAG_ENABLED(ForcePrivateMemoryToGlobalOnGeneric);
+}
+
 bool hasNoFullI64Support() const
 {
     return hasNoInt64Inst();
diff --git a/IGC/Compiler/CodeGenPublic.h b/IGC/Compiler/CodeGenPublic.h
@@ -284,6 +284,7 @@ namespace IGC
         unsigned int numInsts;    //<! measured after optimization, used as a compiler heuristic
         unsigned int numAllocaInsts;
         unsigned int numPsInputs;
+        bool hasDynamicGenericLoadStore;
     };
 
     struct SSimplePushInfo
diff --git a/IGC/Compiler/Optimizer/OpenCLPasses/GenericAddressResolution/GenericAddressDynamicResolution.cpp b/IGC/Compiler/Optimizer/OpenCLPasses/GenericAddressResolution/GenericAddressDynamicResolution.cpp
@@ -225,8 +225,10 @@ bool GenericAddressDynamicResolution::visitLoadStoreInst(Instruction& I)
         IGC_ASSERT_EXIT_MESSAGE(0, "Unable to resolve generic address space pointer");
     }
 
+    m_ctx->m_instrTypes.hasDynamicGenericLoadStore = true;
+
     if (pointerAddressSpace == ADDRESS_SPACE_GENERIC) {
-        if (m_ctx->forceGlobalMemoryAllocation() && m_ctx->hasNoLocalToGenericCast())
+        if ((m_ctx->forceGlobalMemoryAllocation() || m_ctx->platform.canForcePrivateToGlobal()) && m_ctx->hasNoLocalToGenericCast())
         {
             resolveGASWithoutBranches(I, pointerOperand);
         }
@@ -271,27 +273,32 @@ void GenericAddressDynamicResolution::resolveGAS(Instruction& I, Value* pointerO
     Value* privateLoad = nullptr;
     Value* globalLoad = nullptr;
 
+    bool hasPrivate = !m_ctx->platform.canForcePrivateToGlobal();
+    bool hasLocal = !m_ctx->hasNoLocalToGenericCast();
 
     // Private branch
-    privateBlock = BasicBlock::Create(I.getContext(), "PrivateBlock", convergeBlock->getParent(), convergeBlock);
+    if (hasPrivate)
     {
-        IRBuilder<> privateBuilder(privateBlock);
-        PointerType* ptrType = pointerType->getElementType()->getPointerTo(ADDRESS_SPACE_PRIVATE);
-        Value* privatePtr = privateBuilder.CreateAddrSpaceCast(pointerOperand, ptrType);
-
-        if (LoadInst* LI = dyn_cast<LoadInst>(&I))
+        privateBlock = BasicBlock::Create(I.getContext(), "PrivateBlock", convergeBlock->getParent(), convergeBlock);
         {
-            privateLoad = privateBuilder.CreateAlignedLoad(privatePtr, getAlign(LI->getAlignment()), LI->isVolatile(), "privateLoad");
-        }
-        else if (StoreInst* SI = dyn_cast<StoreInst>(&I))
-        {
-            privateBuilder.CreateAlignedStore(I.getOperand(0), privatePtr, getAlign(SI->getAlignment()), SI->isVolatile());
+            IRBuilder<> privateBuilder(privateBlock);
+            PointerType* ptrType = pointerType->getElementType()->getPointerTo(ADDRESS_SPACE_PRIVATE);
+            Value* privatePtr = privateBuilder.CreateAddrSpaceCast(pointerOperand, ptrType);
+
+            if (LoadInst* LI = dyn_cast<LoadInst>(&I))
+            {
+                privateLoad = privateBuilder.CreateAlignedLoad(privatePtr, getAlign(LI->getAlignment()), LI->isVolatile(), "privateLoad");
+            }
+            else if (StoreInst* SI = dyn_cast<StoreInst>(&I))
+            {
+                privateBuilder.CreateAlignedStore(I.getOperand(0), privatePtr, getAlign(SI->getAlignment()), SI->isVolatile());
+            }
+            privateBuilder.CreateBr(convergeBlock);
         }
-        privateBuilder.CreateBr(convergeBlock);
     }
 
     // Local Branch
-    if (!m_ctx->hasNoLocalToGenericCast())
+    if (hasLocal)
     {
         localBlock = BasicBlock::Create(I.getContext(), "LocalBlock", convergeBlock->getParent(), convergeBlock);
         // Local
@@ -332,35 +339,33 @@ void GenericAddressDynamicResolution::resolveGAS(Instruction& I, Value* pointerO
     currentBlock->getTerminator()->eraseFromParent();
     builder.SetInsertPoint(currentBlock);
 
-    // Local branch can be saved if there are no local to generic casts
-    if (m_ctx->hasNoLocalToGenericCast())
-    {
-        SwitchInst* switchTag = builder.CreateSwitch(tag, globalBlock, 1);
-        // Based on tag there are two cases 001: private, 000/111: global
-        switchTag->addCase(privateTag, privateBlock);
+    int numPrivateLocal = (hasPrivate && hasLocal) ? 2 : ((hasPrivate || hasLocal) ? 1 : 0);
+    assert(numPrivateLocal > 0);
 
-        if ((privateLoad != nullptr) && (globalLoad != nullptr))
-        {
-            IRBuilder<> phiBuilder(&(*convergeBlock->begin()));
-            PHINode* phi = phiBuilder.CreatePHI(I.getType(), 2, I.getName());
-            phi->addIncoming(privateLoad, privateBlock);
-            phi->addIncoming(globalLoad, globalBlock);
-            I.replaceAllUsesWith(phi);
-        }
-    }
-    else
     {
-        SwitchInst* switchTag = builder.CreateSwitch(tag, globalBlock, 2);
+        SwitchInst* switchTag = builder.CreateSwitch(tag, globalBlock, numPrivateLocal);
         // Based on tag there are two cases 001: private, 010: local, 000/111: global
-        switchTag->addCase(privateTag, privateBlock);
-        switchTag->addCase(localTag, localBlock);
+        if (hasPrivate)
+        {
+            switchTag->addCase(privateTag, privateBlock);
+        }
+        if (hasLocal)
+        {
+            switchTag->addCase(localTag, localBlock);
+        }
 
-        if ((privateLoad != nullptr) && (localLoad != nullptr) && (globalLoad != nullptr))
+        if (isa<LoadInst>(&I))
         {
             IRBuilder<> phiBuilder(&(*convergeBlock->begin()));
-            PHINode* phi = phiBuilder.CreatePHI(I.getType(), 3, I.getName());
-            phi->addIncoming(privateLoad, privateBlock);
-            phi->addIncoming(localLoad, localBlock);
+            PHINode* phi = phiBuilder.CreatePHI(I.getType(), numPrivateLocal + 1, I.getName());
+            if (privateLoad)
+            {
+                phi->addIncoming(privateLoad, privateBlock);
+            }
+            if (localLoad)
+            {
+                phi->addIncoming(localLoad, localBlock);
+            }
             phi->addIncoming(globalLoad, globalBlock);
             I.replaceAllUsesWith(phi);
         }
diff --git a/IGC/Compiler/Optimizer/OpenCLPasses/PrivateMemory/PrivateMemoryResolution.cpp b/IGC/Compiler/Optimizer/OpenCLPasses/PrivateMemory/PrivateMemoryResolution.cpp
@@ -266,6 +266,11 @@ bool PrivateMemoryResolution::safeToUseScratchSpace(llvm::Module& M) const
     bool supportsStatelessSpacePrivateMemory = Ctx.m_DriverInfo.supportsStatelessSpacePrivateMemory();
     bool bOCLLegacyStatelessCheck = true;
 
+    if (Ctx.m_instrTypes.hasDynamicGenericLoadStore && Ctx.platform.canForcePrivateToGlobal())
+    {
+        return false;
+    }
+
     if ((modMD.compOpt.OptDisable && bOCLLegacyStatelessCheck) || !supportsScratchSpacePrivateMemory
         || Ctx.forceGlobalMemoryAllocation()) {
         return false;
diff --git a/IGC/common/igc_flags.def b/IGC/common/igc_flags.def
@@ -467,6 +467,7 @@ DECLARE_IGC_REGKEY(DWORD, OverrideOCLMaxParamSize, 0,  "Override the value impos
 DECLARE_IGC_REGKEY(bool, EnableOptReportPrivateMemoryToSLM, false, "[POC] Generate opt report file for moving private memory allocations to SLM.", false)
 DECLARE_IGC_REGKEY(bool, ForceAllPrivateMemoryToSLM, false, "[POC] Force moving all private memory allocations to SLM.", false)
 DECLARE_IGC_REGKEY(debugString, ForcePrivateMemoryToSLMOnBuffers, 0, "[POC] Force moving private memory allocations to SLM, semicolon-separated list of buffers.", false)
+DECLARE_IGC_REGKEY(bool, ForcePrivateMemoryToGlobalOnGeneric, false, "Force moving private memory allocations to global buffer when generic pointer is present", true)
 
 DECLARE_IGC_GROUP("Generating precompiled headers")
 DECLARE_IGC_REGKEY(bool, ApplyConservativeRastWAHeader, true, "Apply WaConservativeRasterization for the platforms enabled", false)

Original file line number	Diff line number	Diff line change
`@@ -103,6 +103,7 @@ CheckInstrTypes::CheckInstrTypes(IGC::SInstrTypes* instrList) : FunctionPass(ID)`
`103`	`103`	`instrList->sampleCmpToDiscardOptimizationPossible = false;`
`104`	`104`	`instrList->sampleCmpToDiscardOptimizationSlot = 0;`
`105`	`105`	`instrList->hasPullBary = false;`
	`106`	`+ instrList->hasDynamicGenericLoadStore = false;`
`106`	`107`	`}`
`107`	`108`
`108`	`109`	`void CheckInstrTypes::SetLoopFlags(Function& F)`
Original file line number	Diff line number	Diff line change
`@@ -461,6 +461,11 @@ unsigned getSlmSizePerSsOrDss() const`
`461`	`461`	`return slmSizePerSsOrDss;`
`462`	`462`	`}`
`463`	`463`
	`464`	`+bool canForcePrivateToGlobal() const`
	`465`	`+{`
	`466`	`+ return m_platformInfo.eRenderCoreFamily >= IGFX_GEN9_CORE && IGC_IS_FLAG_ENABLED(ForcePrivateMemoryToGlobalOnGeneric);`
	`467`	`+}`
	`468`	`+`
`464`	`469`	`bool hasNoFullI64Support() const`
`465`	`470`	`{`
`466`	`471`	`return hasNoInt64Inst();`