added verifier to FunctionGroupAnalysis

Current FGA implementation may produce erroneous results.
Added verification and debugging facilities to detect errors.

Author: aparshin-intel

IGC/VectorCompiler/lib/GenXCodeGen/FunctionGroup.cpp

@@ -31,15 +31,55 @@ SPDX-License-Identifier: MIT
 #include "llvm/Support/raw_ostream.h"
 #include "llvm/Transforms/Utils/Cloning.h"
 #include "llvm/Transforms/Utils/ValueMapper.h"
-using namespace llvm;
+
+#include "GenXUtil.h"
 
 #include "llvmWrapper/IR/LegacyPassManagers.h"
 #include "llvmWrapper/IR/PassTimingInfo.h"
-#include "Probe/Assertion.h"
 
+#include "Probe/Assertion.h"
 
 #define DEBUG_TYPE "functiongroup-passmgr"
 
+using namespace llvm;
+
+bool FunctionGroup::verify() const {
+  for (auto I = Functions.begin(), E = Functions.end(); I != E; ++I) {
+    Function *F = &(**I);
+    for (auto *U : F->users()) {
+      auto *CI = genx::checkFunctionCall(U, F);
+      if (!CI)
+        continue;
+
+      // Note: it is expected that all users of any function from
+      // Functions array belong to the same FG
+      Function *Caller = CI->getFunction();
+      auto *OtherGroup = FGA->getAnyGroup(Caller);
+      IGC_ASSERT_MESSAGE(OtherGroup == this,
+                         "inconsisten function group detected!");
+      if (OtherGroup != this)
+        return false;
+    }
+  }
+  return true;
+}
+
+#if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
+void FunctionGroup::print(raw_ostream &OS) const {
+  OS << "{{{" << getName() << "}}}\n";
+  for (const Function *F : Functions) {
+    OS << "  " << F->getName() << "\n";
+  }
+
+  for (const auto &EnItem : enumerate(Subgroups)) {
+    OS << "--SGR[" << EnItem.index() << "]: ";
+    OS << "<" << EnItem.value()->getHead()->getName() << ">]\n";
+  }
+}
+
+void FunctionGroup::dump() const { print(dbgs()); }
+#endif // if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
+
 /***********************************************************************
  * FunctionGroupAnalysis implementation
  */
@@ -57,13 +97,9 @@ void FunctionGroupAnalysis::clear() {
   for (auto T : TypesToProcess)
     GroupMap[T].clear();
 
-  for (auto i = begin(), e = end(); i != e; ++i)
-    delete *i;
-  for (auto i = NonMainGroups.begin(), e = NonMainGroups.end(); i != e; ++i)
-    delete *i;
-
   Groups.clear();
   NonMainGroups.clear();
+  Visited.clear();
   M = nullptr;
 }
 
@@ -140,11 +176,12 @@ void FunctionGroupAnalysis::addToFunctionGroup(FunctionGroup *FG, Function *F,
 // createFunctionGroup : create new FunctionGroup for which F is the head
 FunctionGroup *FunctionGroupAnalysis::createFunctionGroup(Function *F,
                                                           FGType Type) {
-  auto FG = new FunctionGroup(this);
+  auto *FG = new FunctionGroup(this);
+  auto FGOwner = std::unique_ptr<FunctionGroup>(FG);
   if (Type == FGType::GROUP)
-    Groups.push_back(FG);
+    Groups.push_back(std::move(FGOwner));
   else
-    NonMainGroups.push_back(FG);
+    NonMainGroups.push_back(std::move(FGOwner));
   addToFunctionGroup(FG, F, Type);
   return FG;
 }
@@ -211,7 +248,7 @@ bool FunctionGroupAnalysis::buildGroup(CallGraph &Callees, Function *F,
       }
     }
   } else if (!Visited.count(F)) {
-    Visited[F] = true;
+    Visited.insert(F);
     // group is created either on a function with a corresponding attribute
     // or on a root of a whole function tree that is kernel (genx_main)
     if (F->hasFnAttribute(TypeToAttr(Type)) ||
@@ -240,6 +277,30 @@ bool FunctionGroupAnalysis::buildGroup(CallGraph &Callees, Function *F,
   return result;
 }
 
+bool FunctionGroupAnalysis::verify() const {
+  return std::all_of(Groups.begin(), Groups.end(),
+                     [](const auto &GR) { return GR->verify(); });
+}
+
+#if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
+void FunctionGroupAnalysis::print(raw_ostream &OS) const {
+  OS << "Number of Groups = " << Groups.size() << "\n";
+  for (const auto &X : enumerate(Groups)) {
+    OS << "GR[" << X.index() << "] = <\n";
+    X.value()->print(OS);
+    OS << ">\n";
+  }
+  OS << "Number of SubGroups = " << NonMainGroups.size() << "\n";
+  for (const auto &X : enumerate(NonMainGroups)) {
+    OS << "SGR[" << X.index() << "] = <\n";
+    X.value()->print(OS);
+    OS << ">\n";
+  }
+}
+
+void FunctionGroupAnalysis::dump() const { print(dbgs()); }
+#endif // if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
+
 //===----------------------------------------------------------------------===//
 // FGPassManager
 //

IGC/VectorCompiler/lib/GenXCodeGen/FunctionGroup.h

@@ -24,17 +24,20 @@ SPDX-License-Identifier: MIT
 /// is used. It could be moved somewhere more general.
 ///
 //===----------------------------------------------------------------------===//
-#ifndef FUNCTIONGROUP_H
-#define FUNCTIONGROUP_H
+#ifndef LIB_GENXCODEGEN_FUNCTIONGROUP_H
+#define LIB_GENXCODEGEN_FUNCTIONGROUP_H
 
 #include "vc/GenXOpts/Utils/KernelInfo.h"
-#include "llvm/ADT/SetVector.h"
-#include "llvm/ADT/SmallVector.h"
-#include "llvm/IR/Module.h"
-#include "llvm/IR/ValueHandle.h"
-#include "llvm/Pass.h"
+
+#include <llvm/ADT/SetVector.h>
+#include <llvm/ADT/SmallVector.h>
+#include <llvm/IR/Module.h>
+#include <llvm/IR/ValueHandle.h>
+#include <llvm/Pass.h>
 
 #include <list>
+#include <unordered_set>
+
 #include "Probe/Assertion.h"
 
 namespace llvm {
@@ -70,7 +73,7 @@ class FunctionGroup {
 
 public:
   FunctionGroup(FunctionGroupAnalysis *FGA) : FGA(FGA) {}
-  FunctionGroupAnalysis *getParent() { return FGA; }
+  FunctionGroupAnalysis *getParent() const { return FGA; }
   // push_back : push a Function into the group. The first time this is done,
   // the Function is the head Function.
   void push_back(Function *F) { Functions.push_back(AssertingVH<Function>(F)); }
@@ -97,7 +100,7 @@ class FunctionGroup {
     IGC_ASSERT(size());
     return *begin();
   }
-  StringRef getName() { return getHead()->getName(); }
+  StringRef getName() const { return getHead()->getName(); }
   LLVMContext &getContext() { return getHead()->getContext(); }
   Module *getModule() { return getHead()->getParent(); }
   void addSubgroup(FunctionGroup *FG) {
@@ -119,6 +122,13 @@ class FunctionGroup {
   iterator_range<const_subgroup_iterator> subgroups() const {
     return make_range(begin_subgroup(), end_subgroup());
   }
+
+  bool verify() const;
+
+#if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
+  void print(raw_ostream &OS) const;
+  void dump() const;
+#endif // if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
 };
 
 //----------------------------------------------------------------------
@@ -140,12 +150,12 @@ class FunctionGroupAnalysis : public ModulePass {
 
 private:
   Module *M = nullptr;
-  SmallVector<FunctionGroup *, 8> Groups;
+  SmallVector<std::unique_ptr<FunctionGroup>, 8> Groups;
 
   // storage for FunctionGroups that aren't of type GROUP,
   // i.e. not necessarily GENX_MAIN headed
   // TODO: mb increase 8 as there can be many stack funcs hence may subgroups
-  SmallVector<FunctionGroup *, 8> NonMainGroups;
+  SmallVector<std::unique_ptr<FunctionGroup>, 8> NonMainGroups;
 
   class FGMap {
     using ElementType = std::map<const Function *, FunctionGroup *>;
@@ -164,7 +174,9 @@ class FunctionGroupAnalysis : public ModulePass {
   };
 
   FGMap GroupMap;
-  std::map<Function *, bool> Visited;
+  // TODO: "Visited" should not be part of FGA state - it should be an external
+  // entity
+  std::unordered_set<Function *> Visited;
   using CallGraph = std::map<Function *, std::list<Function *>>;
 
 public:
@@ -244,7 +256,12 @@ class FunctionGroupAnalysis : public ModulePass {
   bool buildGroup(CallGraph &callees, Function *F,
                   FunctionGroup *curGr = nullptr, FGType Type = FGType::GROUP);
 
-  void clearVisited() { Visited.clear(); }
+  bool verify() const;
+
+#if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
+  void print(raw_ostream &OS) const;
+  void dump() const;
+#endif // if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
 };
 
 ModulePass *createFunctionGroupAnalysisPass();
@@ -381,6 +398,9 @@ class LoopInfoGroupWrapperPass : public FunctionGroupPass {
 
   void print(raw_ostream &OS, const Module *M = nullptr) const override;
 };
+
 void initializeLoopInfoGroupWrapperPassPass(PassRegistry &);
+
 } // end namespace llvm
-#endif // ndef FUNCTIONGROUP_H
+
+#endif // LIB_GENXCODEGEN_FUNCTIONGROUP_H

IGC/VectorCompiler/lib/GenXCodeGen/GenXModule.cpp

@@ -159,6 +159,7 @@ bool GenXModule::runOnModule(Module &M) {
     }
   }
 
+  IGC_ASSERT(FGA->verify());
   return ModuleModified;
 }
 void GenXModule::updateVisaMapping(const Function *K, const Instruction *Inst,