Refactor FunctionPropertiesAnalysis

[tarinduj]

Summary

this separates analyze logic from FunctionPropertiesAnalysis

[mtrofin]

llvm/include/llvm/Analysis/ML/FunctionPropertiesAnalysis.h

44 ↗

(On Diff #273230)

I think (D82270) we were saying we would have a static factory method in FunctionPropertiesInfo, instead of a member. We'd then (maybe later CL) try to treat a FunctionPropertiesInfo instance as immutable.

[tarinduj]

added a static factory method getFunctionPropertiesInfo.

[mtrofin]

lgtm

[tarinduj]

[rebased] can someone upstream this? I do not have commit access.

llvm/include/llvm/Analysis/FunctionPropertiesAnalysis.h

-//=- FunctionPropertiesAnalysis.h - Function Properties extraction  -*- C++ -=//
+//==- FunctionPropertiesAnalysis.h - Function Properties Analysis -*-C++ -*-==//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
+//
+// This file defines the FunctionPropertiesInfo and FunctionPropertiesAnalysis
+// classes used to extract function properties.
+//
+//===----------------------------------------------------------------------===//
 
 #ifndef LLVM_FUNCTIONPROPERTIESANALYSIS_H_
 #define LLVM_FUNCTIONPROPERTIESANALYSIS_H_

 namespace llvm {
 class Function;
 
+class FunctionPropertiesInfo {
+public:
+  /// Number of basic blocks
+  int64_t BasicBlockCount = 0;
+
+  /// Number of blocks reached from a conditional instruction, or that are
+  /// 'cases' of a SwitchInstr.
+  // FIXME: We may want to replace this with a more meaningful metric, like
+  // number of conditionally executed blocks:
+  // 'if (a) s();' would be counted here as 2 blocks, just like
+  // 'if (a) s(); else s2(); s3();' would.
+  int64_t BlocksReachedFromConditionalInstruction = 0;
+
+  /// Number of uses of this function, plus 1 if the function is callable
+  /// outside the module.
+  int64_t Uses = 0;
+
+  /// Number of direct calls made from this function to other functions
+  /// defined in this module.
+  int64_t DirectCallsToDefinedFunctions = 0;
+
+  static FunctionPropertiesInfo getFunctionPropertiesInfo(const Function &F);
+};
+
+// Analysis pass
 class FunctionPropertiesAnalysis
     : public AnalysisInfoMixin<FunctionPropertiesAnalysis> {
+
 public:
   static AnalysisKey Key;
-  struct Result {
-    /// Number of basic blocks
-    int64_t BasicBlockCount = 0;
-
-    /// Number of blocks reached from a conditional instruction, or that are
-    /// 'cases' of a SwitchInstr.
-    // FIXME: We may want to replace this with a more meaningful metric, like
-    // number of conditionally executed blocks:
-    // 'if (a) s();' would be counted here as 2 blocks, just like
-    // 'if (a) s(); else s2(); s3();' would.
-    int64_t BlocksReachedFromConditionalInstruction = 0;
-
-    /// Number of uses of this function, plus 1 if the function is callable
-    /// outside the module.
-    int64_t Uses = 0;
-
-    /// Number of direct calls made from this function to other functions
-    /// defined in this module.
-    int64_t DirectCallsToDefinedFunctions = 0;
-  };
-  Result run(const Function &F, FunctionAnalysisManager &FAM);
+
+  using Result = FunctionPropertiesInfo;
+
+  Result run(Function &F, FunctionAnalysisManager &FAM);
 };
 
 } // namespace llvm

llvm/lib/Analysis/FunctionPropertiesAnalysis.cpp

 //
 //===----------------------------------------------------------------------===//
 //
-// This file implements an analysis extracting function features, which may be
+// This file implements an analysis extracting function properties, which may be
 // used by ML-driven policies, for example.
 //
 //===----------------------------------------------------------------------===//

 
 using namespace llvm;
 
-AnalysisKey FunctionPropertiesAnalysis::Key;
+FunctionPropertiesInfo
+FunctionPropertiesInfo::getFunctionPropertiesInfo(const Function &F) {
+
+  FunctionPropertiesInfo FPI;
+
+  FPI.Uses = ((!F.hasLocalLinkage()) ? 1 : 0) + F.getNumUses();
 
-FunctionPropertiesAnalysis::Result
-FunctionPropertiesAnalysis::run(const Function &F,
-                                FunctionAnalysisManager &FAM) {
-  Result Ret;
-  Ret.Uses = ((!F.hasLocalLinkage()) ? 1 : 0) + F.getNumUses();
   for (const auto &BB : F) {
-    ++Ret.BasicBlockCount;
+    ++FPI.BasicBlockCount;
+
     if (const auto *BI = dyn_cast<BranchInst>(BB.getTerminator())) {
       if (BI->isConditional())
-        Ret.BlocksReachedFromConditionalInstruction += BI->getNumSuccessors();
-    } else if (const auto *SI = dyn_cast<SwitchInst>(BB.getTerminator()))
-      Ret.BlocksReachedFromConditionalInstruction +=
+        FPI.BlocksReachedFromConditionalInstruction += BI->getNumSuccessors();
+    } else if (const auto *SI = dyn_cast<SwitchInst>(BB.getTerminator())) {
+      FPI.BlocksReachedFromConditionalInstruction +=
           (SI->getNumCases() + (nullptr != SI->getDefaultDest()));
-    for (const auto &I : BB)
+    }
+
+    for (const auto &I : BB) {
       if (auto *CS = dyn_cast<CallBase>(&I)) {
         const auto *Callee = CS->getCalledFunction();
         if (Callee && !Callee->isIntrinsic() && !Callee->isDeclaration())
-          ++Ret.DirectCallsToDefinedFunctions;
+          ++FPI.DirectCallsToDefinedFunctions;
       }
+    }
   }
-  return Ret;
+  return FPI;
+}
+
+AnalysisKey FunctionPropertiesAnalysis::Key;
+
+FunctionPropertiesInfo
+FunctionPropertiesAnalysis::run(Function &F, FunctionAnalysisManager &FAM) {
+  return FunctionPropertiesInfo::getFunctionPropertiesInfo(F);
 }
 No newline at end of file

llvm/unittests/Analysis/FunctionPropertiesAnalysisTest.cpp

-//===- FunctionPropertiesAnalysisTest.cpp - function properties unit tests-===//
+//===- FunctionPropertiesAnalysisTest.cpp - Function Properties Unit Tests-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.