[PATCH] D107831: [llvm][Inline] Refactor out InlineOrder

Kazu Hirata via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Aug 10 10:57:02 PDT 2021


kazu added inline comments.


================
Comment at: llvm/include/llvm/Analysis/InlineOrder.h:1
+
+//===- InlineOrder.h - Inlining decision making abstraction -*- C++ ---*-===//
----------------
Please remove this empty line.


================
Comment at: llvm/include/llvm/Analysis/InlineOrder.h:13-17
+// #include "llvm/Analysis/InlineCost.h"
+// #include "llvm/Analysis/Utils/ImportedFunctionsInliningStatistics.h"
+// #include "llvm/IR/PassManager.h"
+// #include <memory>
+// #include <unordered_set>
----------------
Please include what you use like "llvm/ADT/SmallVector.h", <algorithm>, etc.

LLVM header files are supposed to be self contained, meaning that a C++ source file that does nothing but include a header file compiles without a problem.  While developing this patch, you could try to include this header file as the first one in Inliner.cpp to see if you get any errors from the compiler.


================
Comment at: llvm/include/llvm/Analysis/InlineOrder.h:20-24
+// class BasicBlock;
+// class CallBase;
+// class Function;
+// class Module;
+// class OptimizationRemarkEmitter;
----------------
Please remove these.


================
Comment at: llvm/include/llvm/Analysis/InlineOrder.h:76
+
+class Priority {
+public:
----------------
If we are moving this class outside a .cpp, I might rename it to InlinePriority or something to avoid potential conflicts.


================
Comment at: llvm/include/llvm/Analysis/InlineOrder.h:174
+#endif // LLVM_ANALYSIS_INLINEORDER_H
\ No newline at end of file

----------------
Please add a newline at the end.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D107831/new/

https://reviews.llvm.org/D107831



More information about the llvm-commits mailing list