Fix memory overwriting in TPM

Author: Konstantin Vladimirov

IGC/VectorCompiler/include/vc/GenXOpts/Utils/InternalMetadata.h

@@ -24,7 +24,13 @@ inline constexpr const char VCEmulationRoutine[] = "VC.Emulation.Routine";
 }
 
 namespace InstMD {
+// SVMBlockType metadata serves interesting purpose:
+//   Finalizer now don't support properly SVM gathers/scatters less then dword
+//   So we are extending everything to 32 but preserving real type in metadata
+//   To use it later in CISA builder when we are creating gather/scatter
 inline constexpr const char SVMBlockType[] = "SVMBlockType";
+
+// These two are used in prologue/epilogue insertion
 inline constexpr const char FuncArgSize[] = "FuncArgSize";
 inline constexpr const char FuncRetSize[] = "FuncRetSize";
 }

IGC/VectorCompiler/lib/GenXCodeGen/GenXCisaBuilder.cpp

@@ -3710,21 +3710,31 @@ void GenXKernelBuilder::buildIntrinsic(CallInst *CI, unsigned IntrinID,
     return Byte;
   };
 
-  auto GetSvmGatherBlockSize = [&](II::ArgInfo AI) {
-    LLVM_DEBUG(dbgs() << "GetSvmGatherBlockSize\n");
+  auto GetSvmBlockSizeNum = [&](II::ArgInfo Sz, II::ArgInfo Num) {
+    LLVM_DEBUG(dbgs() << "SVM gather/scatter element size and num blocks\n");
     // svm gather/scatter "block size" field, set to reflect the element
     // type of the data
     Value *V = CI;
-    if (!AI.isRet())
-      V = CI->getArgOperand(AI.getArgIdx());
+    if (!Sz.isRet())
+      V = CI->getArgOperand(Sz.getArgIdx());
     auto *EltType = V->getType()->getScalarType();
     if (auto *MDType = CI->getMetadata(InstMD::SVMBlockType))
       EltType = cast<ValueAsMetadata>(MDType->getOperand(0).get())->getType();
+    ConstantInt *LogOp = cast<ConstantInt>(CI->getArgOperand(Num.getArgIdx()));
+    unsigned LogNum = LogOp->getZExtValue();
     unsigned ElBytes = getResultedTypeSize(EltType, DL);
     switch (ElBytes) {
-      // For N = 2 byte data type, use block size 1 and block count 2.
-      // Otherwise, use block size N and block count 1.
+      // For N = 2 byte data type, use block size 1 and block count x2
+      // Otherwise, use block size N and original block count.
     case 2:
+      ElBytes = 0;
+      IGC_ASSERT(LogNum < 4);
+      // This is correct but I can not merge this in while ISPC not fixed
+      // LogNum += 1;
+
+      // this is incorrect temporary solution
+      LogNum = 1;
+      break;
     case 1:
       ElBytes = 0;
       break;
@@ -3739,7 +3749,7 @@ void GenXKernelBuilder::buildIntrinsic(CallInst *CI, unsigned IntrinID,
                                   DS_Error};
       getContext().diagnose(Err);
     }
-    return ElBytes;
+    return std::make_pair(ElBytes, LogNum);
   };
 
   auto CreateOpndPredefinedSrc = [&](PreDefined_Vars RegId, unsigned ROffset,

IGC/VectorCompiler/lib/GenXCodeGen/GenXThreadPrivateMemory.cpp

@@ -620,11 +620,8 @@ bool GenXThreadPrivateMemory::replaceLoad(LoadInst *LdI) {
 
   Value *EltsOffset = FormEltsOffsetVector(NumEltsToLoad, ValueEltSz, LdI);
 
-  unsigned NumBlocks = m_DL->getTypeSizeInBits(LdEltTy) / genx::ByteBits;
-  // This logic is aligned with the on in CisaBuilder and GenXLowering
-  // The reason behind check for == 2 is that svm intrinsics don't support
-  // BlockSize of 2, so for ops with i16s we have to use BlockSize == 1 and NumBlocks == 2
-  Value *logNumBlocks = ConstantInt::get(I32Ty, genx::log2(NumBlocks == 2 ? NumBlocks : 1));
+  // always one element for one channel
+  Value *logNumBlocks = ConstantInt::get(I32Ty, 0);
   Value *Scale = ConstantInt::get(Type::getInt16Ty(*m_ctx), 0);
   Value *Surface = ConstantInt::get(I32Ty,
                                     visa::getReservedSurfaceIndex(m_stack));
@@ -715,9 +712,9 @@ bool GenXThreadPrivateMemory::replaceStore(StoreInst *StI) {
       {Pred->getType(),
        (m_useGlobalMem ? Offset : EltsOffset)->getType(),
        ValueOp->getType()});
-  unsigned NumBlocks = m_DL->getTypeSizeInBits(ValueOpTy->getScalarType()) / genx::ByteBits;
-  // see the comment in replaceLoad above
-  Value *logNumBlocks = ConstantInt::get(I32Ty, genx::log2(NumBlocks == 2 ? NumBlocks : 1));
+
+  // always one element for one channel
+  Value *logNumBlocks = ConstantInt::get(I32Ty, 0);
   Value *Scale = ConstantInt::get(Type::getInt16Ty(*m_ctx), 0);
   Value *Surface = ConstantInt::get(I32Ty,
                                     visa::getReservedSurfaceIndex(m_stack));

IGC/VectorCompiler/lib/GenXCodeGen/Utils/cisa_gen_intrinsics.json

@@ -2308,7 +2308,6 @@
       "sub_opc": [ "LITERAL", "SVM_GATHER" ],
       "exec_size": [ "EXECSIZE_FROM_ARG", 1 ],
       "pred": [ "PREDICATION", 1 ],
-      "block_size_inferred_from_dst": [ "SVMGATHERBLOCKSIZE", 0 ],
       "log2_num_blocks": [ "BYTE", 2 ],
       "address": [ "URAW", 3 ],
       "twoaddr": [ "TWOADDR", 4 ],
@@ -2333,7 +2332,6 @@
       "sub_opc": [ "LITERAL", "SVM_SCATTER" ],
       "exec_size": [ "EXECSIZE_FROM_ARG", 1 ],
       "pred": [ "PREDICATION", 1 ],
-      "block_size_inferred_from_dst": [ "SVMGATHERBLOCKSIZE", 4 ],
       "log2_num_blocks": [ "BYTE", 2 ],
       "address": [ "URAW", 3 ],
       "src": [ "RAW", 4 ]
@@ -3809,13 +3807,15 @@
       ]
     ],
     "ISA_SVM_SVM_GATHER": [
+      "unsigned BlockType, BlockNum",
+      "std::tie(BlockType, BlockNum) = GetSvmBlockSizeNum(II::ArgInfo(II::GENERAL | 0), II::ArgInfo(II::GENERAL | 2))",
       [ "CISA_CALL",
         [ "Kernel->AppendVISASvmGatherInst",
           "pred",
           "exec_mask",
           "exec_size",
-          "(VISA_SVM_Block_Type)block_size_inferred_from_dst",
-          "(VISA_SVM_Block_Num)log2_num_blocks",
+          "(VISA_SVM_Block_Type)BlockType",
+          "(VISA_SVM_Block_Num)BlockNum",
           "address",
           "dst"
         ]
@@ -3835,13 +3835,15 @@
       ]
     ],
     "ISA_SVM_SVM_SCATTER": [
+      "unsigned BlockType, BlockNum",
+      "std::tie(BlockType, BlockNum) = GetSvmBlockSizeNum(II::ArgInfo(II::GENERAL | 4), II::ArgInfo(II::GENERAL | 2))",
       [ "CISA_CALL",
         [ "Kernel->AppendVISASvmScatterInst",
           "pred",
           "exec_mask",
           "exec_size",
-          "(VISA_SVM_Block_Type)block_size_inferred_from_dst",
-          "(VISA_SVM_Block_Num)log2_num_blocks",
+          "(VISA_SVM_Block_Type)BlockType",
+          "(VISA_SVM_Block_Num)BlockNum",
           "address",
           "src"
         ]
@@ -3901,7 +3903,6 @@
     "INT":          "GetUnsignedValue(II::ArgInfo({args}))",
     "LOG2OWORDS":   "GetOwords(II::ArgInfo({args}))",
     "LOG2OWORDS_PLUS_8": "GetOwords(II::ArgInfo({args})) + 8",
-    "SVMGATHERBLOCKSIZE": "GetSvmGatherBlockSize(II::ArgInfo({args}))",
     "TWOADDR":      "ProcessTwoAddr(II::ArgInfo({args}))",
     "CONSTVI1ASI32": "ConstVi1Asi32(II::ArgInfo({args}))",
     "ARGCOUNT":     "GetArgCount(II::ArgInfo({args}))",