legalizing gvload users which's further optimizations by standard LLVM passes may result in potential clobbering.

VLoad's value usage was later optimized by mem2reg in a way that resulted in genx_volatile value clobbering.
Current solution to avoid this is to forbid direct usage of genx.vload(@GENX_VOLATILE_GLOBAL*) values by non-genx intrinsic.
Until all the frontends are fixed, we fix-up such cases (emitting warning message in Debug build), since they are trivial to fix-up.

This fix will be followed up by removal of previous fix to this issue which was based on convertion of a non-genx_volatile global
used "in the context of" a genx_volatile global to genx_volatile global. This fix results in implicit conversion of "ordinary globals"
to genx_volatile which is not expected by the user and increases register pressure.

Author: lsatanov

IGC/VectorCompiler/lib/GenXCodeGen/CMakeLists.txt

@@ -43,6 +43,7 @@ set(CODEGEN_SOURCES
   GenXEmulate.cpp
   GenXExtractVectorizer.cpp
   GenXFixInvalidFuncName.cpp
+  GenXLegalizeGVLoadUses.cpp
   GenXGASCastAnalyzer.cpp
   GenXGASDynamicResolution.cpp
   GenXGEPLowering.cpp

IGC/VectorCompiler/lib/GenXCodeGen/GenX.h

@@ -148,6 +148,7 @@ ModulePass *createGenXStructSplitterPass();
 FunctionPass *createGenXPredRegionLoweringPass();
 FunctionPass *createGenXDebugLegalizationPass();
 FunctionPass *createGenXFixInvalidFuncNamePass();
+FunctionPass *createGenXLegalizeGVLoadUsesPass();
 ModulePass *createGenXGASCastWrapperPass();
 FunctionPass *createGenXGASDynamicResolutionPass();
 ModulePass *createGenXInitBiFConstantsPass();

IGC/VectorCompiler/lib/GenXCodeGen/GenXBaling.cpp

@@ -330,7 +330,7 @@ bool GenXBaling::isRegionOKForIntrinsic(unsigned ArgInfoBits,
  * I operation is load-like, we don't sink it only through store-like operations
  */
 bool GenXBaling::isSafeToMove(Instruction *Op, Instruction *From, Instruction *To) {
-  if (!genx::isSafeToMoveInstCheckAVLoadKill(Op, To, this))
+  if (!genx::isSafeToSink_CheckAVLoadKill(Op, To, this))
     return false;
 
   if (DisableMemOrderCheck || !Op->mayReadOrWriteMemory())
@@ -532,7 +532,7 @@ bool GenXBaling::operandCanBeBaled(
     // where there are no region restrictions.)
     Region RdR = makeRegionFromBaleInfo(Opnd, BaleInfo());
     if (!isRegionOKForIntrinsic(AI.Info, RdR, canSplitBale(Inst)) ||
-        !genx::isSafeToMoveInstCheckAVLoadKill(Opnd, Inst, this))
+        !genx::isSafeToSink_CheckAVLoadKill(Opnd, Inst, this))
       return false;
 
     // Do not bale in a region read with multiple uses if
@@ -657,7 +657,7 @@ void GenXBaling::processWrRegion(Instruction *Inst)
         GenXIntrinsic::GenXRegion::OldValueOperandNum));
     auto *Src1 = genx::getAVLoadSrcOrNull(L1);
     auto *Src2 = genx::getAVLoadSrcOrNull(L2);
-    if (Src1 && Src1 == Src2 && !genx::loadingSameValue(L1, L2, DT))
+    if (Src1 && Src1 == Src2 && !genx::vloadsReadSameValue(L1, L2, DT))
       V = nullptr;
   }
 
@@ -1456,7 +1456,7 @@ void GenXBaling::processMainInst(Instruction *Inst, int IntrinID) {
             break;
           case GenXIntrinsicInfo::RAW:
             if (isa<Instruction>(Inst->getOperand(ArgIdx)) &&
-                !genx::isSafeToMoveInstCheckAVLoadKill(
+                !genx::isSafeToSink_CheckAVLoadKill(
                     cast<Instruction>(Inst->getOperand(ArgIdx)), Inst, this))
               continue;
             // Rdregion can be baled in to a raw operand as long as it is

IGC/VectorCompiler/lib/GenXCodeGen/GenXDepressurizer.cpp

@@ -1121,7 +1121,7 @@ bool GenXDepressurizer::sinkOnce(Instruction *InsertBefore, Superbale *SB,
   for (auto *I : SB->Bales) {
     Bale B;
     Baling->buildBale(I, &B);
-    if (!genx::isSafeToMoveBaleCheckAVLoadKill(B, InsertBefore)) {
+    if (!genx::isSafeToSink_CheckAVLoadKill(B, InsertBefore)) {
       LLVM_DEBUG(
           dbgs() << "Will not move this superbale to position of "
                  << *InsertBefore

IGC/VectorCompiler/lib/GenXCodeGen/GenXGVClobberChecker.cpp

@@ -241,7 +241,7 @@ bool GenXGVClobberChecker::checkGVClobberingByInterveningStore(
     }
 
     const auto *SI = genx::getAVLoadKillOrNull(
-        LI, UI, true, nullptr,
+        LI, UI, false, true, nullptr,
         isa<PHINode>(UI) ? &PhiUserExcludeBlocksOnCfgTraversal : nullptr, SIs);
 
     if (!SI)

IGC/VectorCompiler/lib/GenXCodeGen/GenXIMadPostLegalization.cpp

@@ -180,8 +180,8 @@ bool GenXIMadPostLegalization::runOnFunction(Function &F) {
             MadSumOpndInst->mayHaveSideEffects() ||
             isa<PHINode>(MadSumOpndInst) ||
             MadSumOpndInst->getNextNode() == MadInst ||
-            !genx::isSafeToMoveInstCheckAVLoadKill(MadSumOpndInst, MadInst,
-                                                   Baling))
+            !genx::isSafeToSink_CheckAVLoadKill(MadSumOpndInst, MadInst,
+                                                Baling))
           continue;
         MadSumOpndInst->moveBefore(MadInst);
         Changed = true;
@@ -203,8 +203,8 @@ bool GenXIMadPostLegalization::runOnFunction(Function &F) {
         auto *UserInst = cast<Instruction>(MadSumOpndUse.getUser());
         if (isIntegerMadIntrinsic(UserInst) &&
             MadSumOpndUse.getOperandNo() == 2 &&
-            genx::isSafeToMoveInstCheckAVLoadKill(MadSumOpndInst, UserInst,
-                                                  Baling)) {
+            genx::isSafeToSink_CheckAVLoadKill(MadSumOpndInst, UserInst,
+                                               Baling)) {
           auto *NewMadSumOpndInst = MadSumOpndInst->clone();
           NewMadSumOpndInst->setName(MadSumOpndInst->getName() + ".postimad");
           NewMadSumOpndInst->insertBefore(UserInst);
@@ -220,8 +220,8 @@ bool GenXIMadPostLegalization::runOnFunction(Function &F) {
         Instruction *InsertPt = nullptr;
         std::tie(NBB, InsertPt) = findNearestInsertPt(DT, OtherUseCases);
         InsertPt = InsertPt ? InsertPt : NBB->getTerminator();
-        if (genx::isSafeToMoveInstCheckAVLoadKill(MadSumOpndInst, InsertPt,
-                                                  Baling))
+        if (genx::isSafeToSink_CheckAVLoadKill(MadSumOpndInst, InsertPt,
+                                               Baling))
           MadSumOpndInst->moveBefore(InsertPt);
       } else
         OrphanedMadSumOpndInst.push_back(MadSumOpndInst);
@@ -384,7 +384,7 @@ bool GenXIMadPostLegalization::fixMadChain(BasicBlock *BB) {
           // Skip movement of a bale sourcing a global volatile load predecessor
           // if it can result in potential clobbering by an intervening vstore.
           // Skip phi which is not movable.
-          if (!genx::isSafeToMoveBaleCheckAVLoadKill(*Bale, Pos) ||
+          if (!genx::isSafeToSink_CheckAVLoadKill(*Bale, Pos) ||
               isa<PHINode>(BaleInst->Inst))
             break;
           BaleInst->Inst->moveBefore(Pos);

IGC/VectorCompiler/lib/GenXCodeGen/GenXLegalizeGVLoadUses.cpp

@@ -0,0 +1,57 @@
+/*========================== begin_copyright_notice ============================
+
+Copyright (C) 2023 Intel Corporation
+
+SPDX-License-Identifier: MIT
+
+============================= end_copyright_notice ===========================*/
+
+//
+/// GenXLegalizeGVLoadUses
+///
+//===----------------------------------------------------------------------===//
+
+#include "GenX.h"
+#include "GenXUtil.h"
+
+#include "llvm/IR/Function.h"
+#include "llvm/InitializePasses.h"
+#include "llvm/Pass.h"
+#include "llvm/Support/Debug.h"
+
+#include "GenXRegionUtils.h"
+
+#define DEBUG_TYPE "GENX_LEGALIZE_GVLOAD_USERS"
+
+using namespace llvm;
+
+class GenXLegalizeGVLoadUses : public FunctionPass
+{
+public:
+  static char ID;
+  explicit GenXLegalizeGVLoadUses() : FunctionPass(ID) {}
+  llvm::StringRef getPassName() const override { return "Fix illegal gvload users."; }
+  bool runOnFunction(Function& F) override;
+};
+
+bool GenXLegalizeGVLoadUses::runOnFunction(Function& F) {
+  bool Changed = false;
+  for (auto &BB : F)
+    for (auto II = BB.begin(); II != BB.end();) {
+      auto *Inst = &*II++;
+      if (genx::isAGVLoad(Inst))
+        Changed |= genx::legalizeGVLoadForbiddenUses(Inst);
+    }
+  return Changed;
+}
+
+char GenXLegalizeGVLoadUses::ID = 0;
+namespace llvm {
+void initializeGenXLegalizeGVLoadUsesPass(PassRegistry &);
+}
+INITIALIZE_PASS_BEGIN(GenXLegalizeGVLoadUses, "GenXLegalizeGVLoadUses", "GenXLegalizeGVLoadUses", false, false)
+INITIALIZE_PASS_END(GenXLegalizeGVLoadUses, "GenXLegalizeGVLoadUses", "GenXLegalizeGVLoadUses", false, false)
+FunctionPass *llvm::createGenXLegalizeGVLoadUsesPass() {
+  initializeGenXLegalizeGVLoadUsesPass(*PassRegistry::getPassRegistry());
+  return new GenXLegalizeGVLoadUses;
+}

IGC/VectorCompiler/lib/GenXCodeGen/GenXRegionCollapsing.cpp

@@ -230,9 +230,6 @@ void GenXRegionCollapsing::runOnBasicBlock(BasicBlock *BB) {
     }
 
     if (Value *V = simplifyRegionInst(Inst, DL)) {
-      if (isa<Instruction>(V) &&
-          !genx::isSafeToReplaceInstCheckAVLoadKill(Inst, cast<Instruction>(V)))
-        continue;
       Inst->replaceAllUsesWith(V);
       Modified = true;
       continue;
@@ -530,7 +527,7 @@ void GenXRegionCollapsing::processRdRegion(Instruction *InnerRd)
   for (;;) {
     Instruction *OuterRd = dyn_cast<Instruction>(InnerRd->getOperand(0));
 
-    if (OuterRd && !genx::isSafeToMoveInstCheckAVLoadKill(OuterRd, InnerRd))
+    if (OuterRd && !genx::isSafeToMove_CheckAVLoadKill(OuterRd, InnerRd, DT))
       break;
 
     // Go through any bitcasts and up to one sext/zext if necessary to find the
@@ -767,7 +764,7 @@ void GenXRegionCollapsing::processWrRegionElim(Instruction *OuterWr)
       InnerWr->getOperand(GenXIntrinsic::GenXRegion::OldValueOperandNum);
 
   if (auto *I = dyn_cast<Instruction>(InnerWrOldV);
-      I && !genx::isSafeToUseAtPosCheckAVLoadKill(I, OuterWr))
+      I && !genx::isSafeToUse_CheckAVLoadKill(I, OuterWr, DT))
     return;
 
   Region InnerR = genx::makeRegionFromBaleInfo(InnerWr, BaleInfo(),
@@ -917,7 +914,7 @@ Instruction *GenXRegionCollapsing::processWrRegion(Instruction *OuterWr)
       dyn_cast<Instruction>(genx::getBitCastedValue(cast<Value>(InnerWr)));
 
   if (!GenXIntrinsic::isWrRegion(InnerWr) ||
-      !genx::isSafeToMoveInstCheckAVLoadKill(InnerWr, OuterWr))
+      !genx::isSafeToMove_CheckAVLoadKill(InnerWr, OuterWr, DT))
     return OuterWr;
 
   // Process inner wrregions first, recursively.
@@ -1033,7 +1030,7 @@ Instruction *GenXRegionCollapsing::processWrRegionSplat(Instruction *OuterWr)
       dyn_cast<Instruction>(genx::getBitCastedValue(cast<Value>(InnerWr)));
 
   if (!GenXIntrinsic::isWrRegion(InnerWr) ||
-      !genx::isSafeToMoveInstCheckAVLoadKill(InnerWr, OuterWr))
+      !genx::isSafeToMove_CheckAVLoadKill(InnerWr, OuterWr, DT))
     return OuterWr;
 
   // Process inner wrregions first, recursively.