Add intrinsic cache to KernelDebugInfo

Avoid need to iterate through all BBs to find if instruction is part of save restore vectors.
intel · Dec 18, 2024 · e4cdf12 · e4cdf12
1 parent fc155ef
commit e4cdf12
Show file tree

Hide file tree

Showing 2 changed files with 52 additions and 44 deletions.
diff --git a/visa/DebugInfo.cpp b/visa/DebugInfo.cpp
@@ -9,10 +9,10 @@ SPDX-License-Identifier: MIT
 #include "DebugInfo.h"
 
 #include "Assertions.h"
-#include "BitSet.h"
 #include "BuildIR.h"
 #include "Common_ISA_framework.h"
 #include "FlowGraph.h"
+#include "G4_BB.hpp"
 #include "G4_IR.hpp"
 #include "VISAKernel.h"
 
@@ -1933,76 +1933,76 @@ void KernelDebugInfo::updateExpandedIntrinsic(G4_InstIntrinsic *spillOrFill,
   // save/restore. These intrinsics are expanded after RA is
   // done. So this method gets invoked after RA is done and
   // when intrinsics are expanded.
-  for (auto &k : callerSaveRestore) {
-    for (auto it = k.second.first.begin(); it != k.second.first.end(); ++it) {
-      if ((*it) == spillOrFill) {
-        k.second.first.insert(it, insts.begin(), insts.end());
-        return;
-      }
-    }
 
-    for (auto it = k.second.second.begin(); it != k.second.second.end(); ++it) {
-      if ((*it) == spillOrFill) {
-        k.second.second.insert(it, insts.begin(), insts.end());
-        return;
-      }
-    }
-  }
+  auto cacheIt = isSaveRestoreInst.find(spillOrFill);
 
-  for (auto it = calleeSaveRestore.first.begin();
-       it != calleeSaveRestore.first.end(); ++it) {
-    if ((*it) == spillOrFill) {
-      calleeSaveRestore.first.insert(it, insts.begin(), insts.end());
-      return;
+  if (cacheIt == isSaveRestoreInst.end()) {
+    if (spillOrFill == getCallerBEFPRestoreInst()) {
+      // caller be fp restore is a fill intrinsic that reads from FDE. it is
+      // expanded to a regular fill instruction. so update the pointer to new
+      // instruction.
+      setCallerBEFPRestoreInst(insts.back());
     }
-  }
 
-  for (auto it = calleeSaveRestore.second.begin();
-       it != calleeSaveRestore.second.end(); ++it) {
-    if ((*it) == spillOrFill) {
-      calleeSaveRestore.second.insert(it, insts.begin(), insts.end());
-      return;
+    if (spillOrFill == getCallerSPRestoreInst()) {
+      setCallerSPRestoreInst(insts.back());
     }
-  }
 
-  if (spillOrFill == getCallerBEFPRestoreInst()) {
-    // caller be fp restore is a fill intrinsic that reads from FDE. it is
-    // expanded to a regular fill instruction. so update the pointer to new
-    // instruction.
-    setCallerBEFPRestoreInst(insts.back());
-  }
+    if (spillOrFill == getCallerBEFPSaveInst()) {
+      setCallerBEFPSaveInst(insts.back());
+    }
 
-  if (spillOrFill == getCallerSPRestoreInst()) {
-    setCallerSPRestoreInst(insts.back());
+    if (spillOrFill == getCESaveInst()) {
+      for (auto *inst : insts) {
+        if (inst->isSend()) {
+          setSaveCEInst(inst);
+        }
+      }
+    }
+    return;
   }
 
-  if (spillOrFill == getCallerBEFPSaveInst()) {
-    setCallerBEFPSaveInst(insts.back());
-  }
+  // We use isSaveRestoreInst map to determine if target spillOrFill instruction is part of
+  // callee/callerSaveRestore and is it in save or restore vector.
+  auto &[save, restore] = (cacheIt->second.bb == nullptr)
+                              ? calleeSaveRestore
+                              : callerSaveRestore[cacheIt->second.bb];
+  auto &target = (cacheIt->second.isSave) ? save : restore;
 
-  if (spillOrFill == getCESaveInst()) {
-    for (auto *inst: insts) {
-      if (inst->isSend()){
-        setSaveCEInst(inst);
-      }
+  for (auto it = target.begin(); it != target.end(); ++it) {
+    if ((*it) == spillOrFill) {
+      target.insert(it, insts.begin(), insts.end());
+      return;
     }
   }
 }
 
 void KernelDebugInfo::addCallerSaveInst(G4_BB *fcallBB, G4_INST *inst) {
   callerSaveRestore[fcallBB].first.push_back(inst);
+  if (inst->isIntrinsic()) {
+    isSaveRestoreInst.insert({inst, {true, fcallBB}});
+  }
 }
 
 void KernelDebugInfo::addCallerRestoreInst(G4_BB *fcallBB, G4_INST *inst) {
   callerSaveRestore[fcallBB].second.push_back(inst);
+  if (inst->isIntrinsic()) {
+    isSaveRestoreInst.insert({inst, {false, fcallBB}});
+  }
 }
 
 void KernelDebugInfo::addCalleeSaveInst(G4_INST *inst) {
   calleeSaveRestore.first.push_back(inst);
+  if (inst->isIntrinsic()) {
+    isSaveRestoreInst.insert({inst, {true, nullptr}});
+  }
 }
 
 void KernelDebugInfo::addCalleeRestoreInst(G4_INST *inst) {
   calleeSaveRestore.second.push_back(inst);
+  if (inst->isIntrinsic()) {
+    isSaveRestoreInst.insert({inst, {false, nullptr}});
+  }
 }
 
 std::vector<G4_INST *> &KernelDebugInfo::getCallerSaveInsts(G4_BB *fcallBB) {

diff --git a/visa/DebugInfo.h b/visa/DebugInfo.h
@@ -220,10 +220,18 @@ class KernelDebugInfo {
   // Caller save/restore
   // std::vector<std::pair<fcall inst BB, std::pair<first caller save, last
   // caller restore>>> One entry per fcall inst in current compilation unit
-  typedef std::pair<std::vector<G4_INST *>, std::vector<G4_INST *>> SaveRestore;
+  using SaveRestore = std::pair<std::vector<G4_INST *>, std::vector<G4_INST *>>;
   std::unordered_map<G4_BB *, SaveRestore> callerSaveRestore;
   SaveRestore calleeSaveRestore;
 
+  // Cache used to quickly check if intrinsic is in
+  // callerSaveRestore/calleeSaveRestore.
+  struct SaveRestoreIndex {
+    bool isSave;
+    G4_BB *bb;
+  };
+  std::unordered_map<vISA::G4_INST *, SaveRestoreIndex> isSaveRestoreInst;
+
   std::unordered_set<vISA::G4_INST *>  oldInsts;
 
   // Store pair of cisa byte offset and gen byte offset in vector