MaskRay added inline comments.
================
Comment at: clang/include/clang/Basic/CodeGenOptions.def:148
CODEGENOPT(MergeFunctions , 1, 0) ///< Set when -fmerge-functions is
enabled.
+CODEGENOPT(HeapProf , 1, 0) ///< Set when -fmemprof is enabled.
CODEGENOPT(MSVolatile , 1, 0) ///< Set when /volatile:ms is enabled.
----------------
There is a tab on this line. This makes the code not indented in `git diff`
output.
================
Comment at: llvm/lib/Transforms/Instrumentation/HeapProfiler.cpp:45
+
+#define LLVM_HEAP_PROFILER_VERSION 1
+
----------------
Consider `constexpr int LLVM_HEAP_PROFILER_VERSION = 1;`
================
Comment at: llvm/lib/Transforms/Instrumentation/HeapProfiler.cpp:48
+// Size of memory mapped to a single shadow location.
+static const uint64_t DefaultShadowGranularity = 64;
+
----------------
For a constant in the source file, consider `constexpr uint64_t
DefaultShadowGranularity = 64;` (constexpr implies const, which means internal
linkage in a namespace scope, so you can avoid `static`)
================
Comment at: llvm/lib/Transforms/Instrumentation/HeapProfiler.cpp:57
+static const uint64_t HeapProfEmscriptenCtorAndDtorPriority = 50;
+static const char *const HeapProfInitName = "__heapprof_init";
+static const char *const HeapProfVersionCheckNamePrefix =
----------------
`constexpr char HeapProfInitName[] = "__heapprof_init";`
================
Comment at: llvm/lib/Transforms/Instrumentation/HeapProfiler.cpp:100
+
+static cl::opt<int> ClMappingScale("heapprof-mapping-scale",
+ cl::desc("scale of heapprof shadow
mapping"),
----------------
Consider deleting `DefaultShadowScale`.
Alternatively, `cl::init(DefaultShadowScale)` and don't check
getNumOccurrences() below.
================
Comment at: llvm/lib/Transforms/Instrumentation/HeapProfiler.cpp:105
+static cl::opt<int>
+ ClMappingGranularity("heapprof-mapping-granularity",
+ cl::desc("granularity of heapprof shadow mapping"),
----------------
ditto
================
Comment at: llvm/lib/Transforms/Instrumentation/HeapProfiler.cpp:146
+
+uint64_t getCtorAndDtorPriority(Triple &TargetTriple) {
+ return TargetTriple.isOSEmscripten() ? HeapProfEmscriptenCtorAndDtorPriority
----------------
Add static.
https://llvm.org/docs/CodingStandards.html#anonymous-namespaces
================
Comment at: llvm/lib/Transforms/Instrumentation/HeapProfiler.cpp:159
+
+/// HeapProfiler: instrument the code in module to profile heap accesses.
+class HeapProfiler {
----------------
Don’t duplicate function or class name at the beginning of the comment.
https://llvm.org/docs/CodingStandards.html#doxygen-use-in-documentation-comments
================
Comment at: llvm/lib/Transforms/Instrumentation/HeapProfiler.cpp:203
+
+class HeapProfilerLegacyPass : public FunctionPass {
+public:
----------------
Since this is new. Perhaps not deal with legacy pass manager at all?
For example, recently there has been objection on porting CGProfile to the
legacy pass manager. For an entirely new thing, not handling legacy pass
managers can avoid many tests.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D85948/new/
https://reviews.llvm.org/D85948
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
