Corrections to genx_volatile-related API.

- Added comments
- Fixed forbidden users genx.vload checks to apply only for genx_volatile-related ones.
- In "non-strict" mode of GenXGVClobberChecker besides function calls that are proved to cause a kill
  also consider any indirect calls or calls to functions with no definitions in current module.
- Corrected genx::peelBitCastsWhileSingleUserChain(...)
- Added vc::isAnyVcIntrinsic(...)

IGC/VectorCompiler/include/vc/Utils/GenX/IntrinsicsWrapper.h

@@ -26,6 +26,11 @@ bool isOverloadedRet(unsigned ID);
 // Note: input of LLVM intrinsic not expected
 bool isOverloadedArg(unsigned ID, unsigned ArgumentNum);
 
+// Is any of GenX or Internal VC BE intrinsic not equal to not_any_intrinsic
+bool isAnyVcIntrinsic(unsigned ID);
+bool isAnyVcIntrinsic(const llvm::Function *F);
+bool isAnyVcIntrinsic(const llvm::Value *V);
+
 /// isAnyNonTrivialIntrinsic(id) - Is Internal, GenX or LLVM intrinsic, which is
 /// not equal to not_any_intrinsic
 bool isAnyNonTrivialIntrinsic(unsigned ID);

IGC/VectorCompiler/lib/GenXCodeGen/GenXGVClobberChecker.cpp

@@ -301,7 +301,7 @@ bool GenXGVClobberChecker::checkGVClobberingByInterveningStore(
     Detected |= CheckUserInst(
         CheckGVClobbOpt_ChkWithBales
             ? Baling->getBaleHead(dyn_cast<Instruction>(U))
-            : dyn_cast<Instruction>(genx::peelBitCastsInSingleUseChain(U)));
+            : dyn_cast<Instruction>(genx::peelBitCastsWhileSingleUserChain(U)));
 
   return Detected;
 };

IGC/VectorCompiler/lib/GenXCodeGen/GenXLegalizeGVLoadUses.cpp

@@ -42,7 +42,7 @@ bool GenXLegalizeGVLoadUses::runOnModule(Module &M) {
       continue;
     for (auto UI = GV.user_begin(), E = GV.user_end(); UI != E;)
       if (auto *I = dyn_cast<Instruction>(*UI++); I && genx::isAVLoad(I))
-        Changed |= genx::legalizeGVLoadForbiddenUses(I);
+        Changed |= genx::legalizeGVLoadForbiddenUsers(I);
   }
   return Changed;
 }

IGC/VectorCompiler/lib/GenXCodeGen/GenXRegionCollapsing.cpp

@@ -764,7 +764,7 @@ void GenXRegionCollapsing::processWrRegionElim(Instruction *OuterWr)
       InnerWr->getOperand(GenXIntrinsic::GenXRegion::OldValueOperandNum);
 
   if (auto *I = dyn_cast<Instruction>(InnerWrOldV);
-      I && !genx::isSafeToUse_CheckAVLoadKill(I, OuterWr, DT))
+      I && !genx::isSafeToUse_CheckAVLoadKillOrForbiddenUser(I, OuterWr, DT))
     return;
 
   Region InnerR = genx::makeRegionFromBaleInfo(InnerWr, BaleInfo(),

IGC/VectorCompiler/lib/GenXCodeGen/GenXUtil.cpp

@@ -2370,11 +2370,27 @@ Instruction *genx::getAVLoadKillOrNull(
   IGC_ASSERT(FromVLoad->getFunction() == ToPosI->getFunction());
 
   auto isAKill = [VLoadSrc, KillCallSites](Instruction &I) {
-    return isAVStore(&I, VLoadSrc) ||
-           (KillCallSites
-                ? llvm::find(*KillCallSites, &I) != KillCallSites->end()
-                : isa<CallInst>(I) &&
-                      !vc::isAnyNonTrivialIntrinsic(vc::getAnyIntrinsicID(&I)));
+    if (isAVStore(&I, VLoadSrc))
+      return true;
+
+    const auto *Call = dyn_cast<CallInst>(&I);
+    if (!Call)
+      return false;
+
+    const bool IsAnIntrinsic =
+        vc::isAnyNonTrivialIntrinsic(vc::getAnyIntrinsicID(Call));
+
+    if (LLVM_LIKELY(!KillCallSites))
+      return !IsAnIntrinsic;
+
+    const bool KillCallSiteMatch =
+        llvm::find(*KillCallSites, Call) != KillCallSites->end();
+    const auto *CalledFunction = Call->getCalledFunction();
+    const bool IsAnIndirectCall = !CalledFunction;
+    const bool IsDeclaration =
+        CalledFunction && CalledFunction->isDeclaration();
+    return KillCallSiteMatch || IsAnIndirectCall ||
+           (!IsAnIntrinsic && IsDeclaration);
   };
 
   if (DT && ChangeSearchDirectionBasedOnDominance &&
@@ -2493,18 +2509,19 @@ bool genx::isSafeToSink_CheckAVLoadKill(Instruction *I, Instruction *To,
   return isSafeToSink_CheckAVLoadKill(Bale, To, DT);
 }
 
-bool genx::isSafeToUse_CheckAVLoadKill(Instruction *UseSrc,
-                                       Instruction *UseTarget,
-                                       const DominatorTree *DT) {
+bool genx::isSafeToUse_CheckAVLoadKillOrForbiddenUser(Instruction *UseSrc,
+                                                      Instruction *UseTarget,
+                                                      const DominatorTree *DT) {
   IGC_ASSERT(UseSrc != UseTarget);
   IGC_ASSERT(DT->dominates(UseSrc, UseTarget));
-  if (genx::isAVLoad(UseSrc))
-    return !getAVLoadKillOrNull(UseSrc, UseTarget, false, false);
-  return GenXIntrinsic::isGenXNonTrivialIntrinsic(UseTarget) ||
-         !hasVLoadSource(UseSrc) ||
-         llvm::all_of(getSrcVLoads(UseSrc), [UseTarget](Instruction *VLoad) {
-           return !getAVLoadKillOrNull(VLoad, UseTarget, false, false);
-         });
+  if (isAVLoad(UseSrc)) {
+    if (!isAGVLoad(UseSrc)) // Non genx_volatile-related genx.vloads are allowed
+                            // to have any users.
+      return true;
+    return !isAGVLoadForbiddenUser(UseTarget) &&
+           !getAVLoadKillOrNull(UseSrc, UseTarget, false, false);
+  }
+  return true;
 }
 
 bool genx::isSafeToMove_CheckAVLoadKill(Instruction *I, Instruction *To,
@@ -2527,43 +2544,54 @@ bool genx::isSafeToReplace_CheckAVLoadKillOrForbiddenUser(
                      "both instructions must be placed in IR.");
   return isSafeToMove_CheckAVLoadKill(NewI, OldI, DT) &&
          (!genx::isAVLoad(NewI) ||
-          llvm::all_of(OldI->users(), [NewI, DT](User *U) {
-            return !isAGVLoadForbiddenUser(U) &&
-                   !getAVLoadKillOrNull(NewI, cast<Instruction>(U));
-          }));
+          llvm::all_of(
+              OldI->users(), [NewI, DT, IsAGVLoad = isAGVLoad(NewI)](User *U) {
+                return (!IsAGVLoad || !isAGVLoadForbiddenUser(U)) &&
+                       !getAVLoadKillOrNull(NewI, cast<Instruction>(U));
+              }));
 }
 
-bool genx::legalizeGVLoadForbiddenUses(Instruction *GVLoad) {
+bool genx::legalizeGVLoadForbiddenUsers(Instruction *GVLoad) {
   IGC_ASSERT(isAGVLoad(GVLoad));
 
   bool Changed = false;
-  for (auto &Use_ : GVLoad->uses()) {
-    User *User_ = genx::peelBitCastsInSingleUseChain(Use_.getUser());
-    IGC_ASSERT(isa<Instruction>(User_));
-    IGC_ASSERT(
-        !genx::isABitCast(User_)); // Only single-use-chain is expected, but can
-                                   // support the tree if such use case arises.
-    if (genx::isAGVLoadForbiddenUser(User_)) {
+
+  Instruction *Rdr = nullptr;
+
+  for (auto UserI = GVLoad->user_begin(), E = GVLoad->user_end(); UserI != E;) {
+    auto *User_ = *UserI++;
+
+    User_ = genx::peelBitCastsWhileSingleUserChain(User_);
+
+    if (!genx::isAGVLoadForbiddenUser(User_))
+      continue;
+
+    if (!Rdr) {
       const auto *GVLoadMaybeVT =
           dyn_cast<IGCLLVM::FixedVectorType>(GVLoad->getType());
       const unsigned ElsCount =
           GVLoadMaybeVT ? GVLoadMaybeVT->getNumElements() : 1;
       genx::Region R(GVLoad);
       R.getSubregion(0, ElsCount);
-      auto *Rdr =
+      Rdr =
           R.createRdRegion(GVLoad, GVLoad->getName() + "._gvload_legalized_rdr",
                            GVLoad->getNextNode(), GVLoad->getDebugLoc(), true);
-      IGCLLVM::replaceUsesWithIf(GVLoad, Rdr, [&](Use &U) {
-        return cast<Instruction>(U.getUser()) != Rdr;
-      });
-      Use_.set(Rdr);
-      LLVM_DEBUG(vc::diagnose(GVLoad->getContext(),
-                              "genx::legalizeGVLoadForbiddenUses: ",
-                              "illegal genx.vload user found, "
-                              "fixing it by inserting rdregion in between.",
-                              DS_Warning, vc::WarningName::Generic, GVLoad););
-      Changed |= true;
     }
+
+    IGC_ASSERT(Rdr);
+
+    IGCLLVM::replaceUsesWithIf(GVLoad, Rdr, [&](Use &U) {
+      return U.getUser() != Rdr && U.getUser() == User_;
+    });
+
+#ifndef NDEBUG
+    vc::diagnose(GVLoad->getContext(), "genx::legalizeGVLoadForbiddenUsers: ",
+                 "illegal genx.vload user found, fixing it by inserting "
+                 "rdregion in between.",
+                 DS_Warning, vc::WarningName::Generic, GVLoad);
+#endif
+
+    Changed |= true;
   }
   return Changed;
 }

IGC/VectorCompiler/lib/GenXCodeGen/GenXUtil.h

@@ -135,8 +135,11 @@ inline bool isABitCast(Value *V) {
           cast<ConstantExpr>(V)->getOpcode() == CastInst::BitCast);
 }
 
-inline User *peelBitCastsInSingleUseChain(User *U) {
-  while (isABitCast(U) && U->hasOneUse())
+// Peels off bitcasts in single-user chain until a non-bitcast user is found or
+// a bitcast user has multiple users. Hence, if a bitcast gets returned from
+// this function, this means it has multiple users.
+inline User *peelBitCastsWhileSingleUserChain(User *U) {
+  while (isABitCast(U) && std::distance(U->user_begin(), U->user_end()) == 1)
     U = *U->user_begin();
   return U;
 }
@@ -776,18 +779,25 @@ bool isRdRWithOldValueVLoadSrc(Value *V);
 bool isWrRWithOldValueVLoadSrc(Value *V);
 
 //----------------------------------------------------------
-// Check that a Store potentially clobbers Load's Use.
-//----------------------------------------------------------
-// G = global value, L = load(G), S = store(G), U = use(L)
-// S clobbers L if there's a path from S to U not through
-// L: { S -> U } != { S -> L -> U }
-//----------------------------------------------------------
-// NB: This function may return a call to a user function as a potential
+// If there's a genx.vload kill on the path (if this path exists) from
+// LI to PosI, return it, otherwize returns nullptr.
+// + NB: In VC BE semantics genx.vload value is not translated to any VISA
+// instruction, rather it signifies either a reference to an object pinned in
+// the register file (if genx.vload loads from a global value with genx_volatile
+// attribute) or a kernel parameter passed by reference (in which case
+// genx.vload is lowered early in pipeline to simple load and later converted to
+// ssa form by mem2reg).
+// + NB: This function may return a call to a user function as a potential
 //     intervening store. If CallSites is supplied, we'll limit our lookup to
 //     the call instructions mentioned in this list.
-// NB: There may be multiple intervening stores, for now we don't have
+// + NB/TBD: There may be multiple intervening stores, for now we don't have
 //     a use case where we want the whole list, hence this variant
-//     only returns the first found.
+//     only returns the first one found.
+//----------------------------------------------------------
+// P = value pointer, L = genx.vload(P), S = genx.vstore(P), U = use(L)
+// S clobbers L if there's a path from S to U not through L:
+// { S -> U } != { S -> L -> U }
+//----------------------------------------------------------
 Instruction *getAVLoadKillOrNull(
     Instruction *LI, Instruction *PosI,
     bool ChangeSearchDirectionBasedOnDominance = false,
@@ -804,14 +814,33 @@ llvm::SmallPtrSet<Instruction *, 1> getSrcVLoads(Instruction *I);
 Instruction *getSrcVLoadOrNull(Instruction *I);
 bool hasVLoadSource(Instruction *I);
 
-// VLoad's users sanity check. In current design to avoid genx_volatile-related
-// clobbering we have to allow only GenX intrinsics to use (ignoring
-// intermediate bitcasts) a VLoad's value.
+// genx_volatile genx.vload's users sanity check.
+// + A genx_volatile genx.vload's "forbidden use" is defined as a use on IR
+// instruction whos optimization can not be controlled by VC backend-owned
+// codebase and can result in clobbering after standard LLVM optimizations.
+// Example:
+// ------ mem2reg can optimize this sequence ------
+// L=genx.vload(@genx_volatile_global*)
+// store(L,%simple_global.localized*)
+// <... no stores into %simple_global.localized* ...>
+// <... possible genx.vstore(Y, @genx_volatile_global*) ...>
+// L2 = load(%simple_global.localized*)
+// use(L2)
+//-------- into --------------
+// L=genx.vload(@genx_volatile_global*)
+// <... possible genx.vstore(Y, @genx_volatile_global*) ...>
+// use(L)
+// ---------------------------
+// Which is clobbering situation, since L is not a VISA instruction in VC BE
+// semantics, but only signifies a pointer to an object pinned in the register
+// file.
 inline bool isAGVLoadForbiddenUser(User *U) {
-  U = genx::peelBitCastsInSingleUseChain(U);
-  IGC_ASSERT(
-      !isABitCast(U)); // Only single-use chain, not a use tree is expected.
-  return !GenXIntrinsic::isGenXNonTrivialIntrinsic(U);
+  U = genx::peelBitCastsWhileSingleUserChain(
+      U); // For simplicity a bitcast having multiple users is considered
+          // forbidden, but bitcasts in single-use chain are not.
+  return !vc::isAnyVcIntrinsic(U) &&
+         !isa<PHINode>(U); // Currently PHI nodes are handeled by GenXCoalescing
+                           // by inserting phicopy
 }
 
 // Safety checks for vloads clobbering when sinking a bale of instructions.
@@ -821,6 +850,7 @@ bool isSafeToSink_CheckAVLoadKill(const Bale &B, Instruction *To,
 bool isSafeToSink_CheckAVLoadKill(Instruction *I, Instruction *To,
                                   GenXBaling *Baling_,
                                   const DominatorTree *DT = nullptr);
+
 // Safety checks for vloads clobbering when looking to hoist or sink an
 // instruction.
 // + NB: Is also reused by isSafeToReplace_CheckAVLoadKillOrForbiddenUser(...).
@@ -832,24 +862,29 @@ bool isSafeToSink_CheckAVLoadKill(Instruction *I, Instruction *To,
 // potential VLoad sources of "I" dominate "To".
 bool isSafeToMove_CheckAVLoadKill(Instruction *I, Instruction *To,
                                   const DominatorTree *DT);
+
 // Safety checks for vloads clobbering when replacing an instruction.
-// + NB: Make sure all the users are valid VLoad users
-// (!isAGVLoadForbiddenUser(...))
 // + NB: When used for checks on hoisting, const DominatorTree* is mandatory and
 // must not be nullptr.
 bool isSafeToReplace_CheckAVLoadKillOrForbiddenUser(Instruction *OldI,
                                                     Instruction *NewI,
                                                     const DominatorTree *DT);
-// Safety checks for vloads clobbering when targeting to modify a use of an
-// instruction.
-// + WARNING: Only checking for VLoad clobbering aspect in the context of value
-// usage. If moving/merging/splitting an instruction or checking in a
-// bale context use isSafeTo(Sink|Move)<...>CheckAVLoadKill variants.
-// + NB: UseSrc must dominate UseTarget.
-bool isSafeToUse_CheckAVLoadKill(Instruction *UseSrc, Instruction *UseTarget,
-                                 const DominatorTree *DT);
 
-bool legalizeGVLoadForbiddenUses(Instruction *GVLoad);
+// Safety checks for genx_volatile genx.vloads clobbering when
+// targeting to modify a use on a UseTarget instruction.
+// + NB: UseSrc must dominate UseTarget.
+// + NB: This API can be augmented to optionally avoid the "forbidden user"
+// check on certain pipeline stages when we know that no further standard LLVM
+// optimizations shall be run.
+// + NB: This API can be augmented by returning metadata
+// differentiating between "kill" and "forbidden user" cases,
+// returning "forbidden user" to make further legalization on the caller side.
+bool isSafeToUse_CheckAVLoadKillOrForbiddenUser(Instruction *UseSrc,
+                                                Instruction *UseTarget,
+                                                const DominatorTree *DT);
+
+// See isAGVLoadForbiddenUser(...) description.
+bool legalizeGVLoadForbiddenUsers(Instruction *GVLoad);
 
 bool vloadsReadSameValue(Instruction *L1, Instruction *L2,
                          const DominatorTree *DT);

IGC/VectorCompiler/lib/Utils/GenX/IntrinsicsWrapper.cpp

@@ -44,6 +44,21 @@ unsigned vc::getAnyIntrinsicID(const llvm::Value *V) {
   return getAnyIntrinsicID(Callee);
 }
 
+bool vc::isAnyVcIntrinsic(unsigned ID) {
+  return InternalIntrinsic::isInternalNonTrivialIntrinsic(ID) ||
+         GenXIntrinsic::isGenXNonTrivialIntrinsic(ID);
+}
+
+bool vc::isAnyVcIntrinsic(const Function *F) {
+  return InternalIntrinsic::isInternalNonTrivialIntrinsic(F) ||
+         GenXIntrinsic::isGenXNonTrivialIntrinsic(F);
+}
+
+bool vc::isAnyVcIntrinsic(const Value *V) {
+  return InternalIntrinsic::isInternalNonTrivialIntrinsic(V) ||
+         GenXIntrinsic::isGenXNonTrivialIntrinsic(V);
+}
+
 bool vc::isAnyNonTrivialIntrinsic(unsigned ID) {
   return InternalIntrinsic::isInternalNonTrivialIntrinsic(ID) ||
          GenXIntrinsic::isAnyNonTrivialIntrinsic(ID);