[PATCH] D153927: Resubmit: [Analysis] Refactor MBB hotness/coldness into templated PSI functions

[Fangrui Song via Phabricator via llvm-commits]

MaskRay accepted this revision.
MaskRay added a comment.

Thanks. This change should support Bazel builds.

> The origin CL D152758 <https://reviews.llvm.org/D152758> caused gcc-12 build failures, which is caused by a gcc bug, and won't be fixed in version 12.

"CL" means https://opensource.google/documentation/reference/glossary#changelist , but our term is Differential, or just "patch" :)

In fact, I'd suggest that you remove

> Below is origin CL description.
>
> [NFC] Refactor MBB hotness/coldness into templated PSI functions.

Just start with the original description. You can add information in last paragraph about what's changed from the reverted (two?) commits.

> In D152399 <https://reviews.llvm.org/D152399>, we calculate BPI->BFI in MachineFunctionSplit pass just to use PSI->isFunctionHotInCallGraph, which is expensive. Instead, we can implement this directly with MBFI.

Since this patch will land before D152399 <https://reviews.llvm.org/D152399> , describing the situation with D152399 <https://reviews.llvm.org/D152399> as the subject may be confusing. I suggest that you describe the code structure issue for the current state, pretending that D152399 <https://reviews.llvm.org/D152399> does not exist.
Then, if you think appropriate, describe that this refactoring will make D152399 <https://reviews.llvm.org/D152399> more focused.



================
Comment at: llvm/lib/CodeGen/MachineFunction.cpp:1504
+//===----------------------------------------------------------------------===//
+namespace llvm {
+template <>
----------------
Remove `namespace llvm` (see https://llvm.org/docs/CodingStandards.html#use-namespace-qualifiers-to-implement-previously-declared-functions).

`ProfileSummaryInfo` is in the namespace llvm, so `ProfileSummaryInfo::getEntryCount` is sufficient.

Ideally `getEntryCount` should be a const reference to indicate that it cannot be null, but the API uses a pointer, so it seems fine to not change it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153927