[PATCH] D129893: extending code layout alg

[Hongtao Yu via Phabricator via llvm-commits]

hoy added inline comments.


================
Comment at: llvm/lib/CodeGen/MachineBlockPlacement.cpp:3491
   auto BlockCounts = std::vector<uint64_t>(F->size());
-  DenseMap<std::pair<uint64_t, uint64_t>, uint64_t> JumpCounts;
+  std::vector<std::pair<std::pair<uint64_t, uint64_t>, uint64_t>> JumpCounts;
   for (MachineBasicBlock &MBB : *F) {
----------------
nit: use `EdgeT` here instead of `std::pair<uint64_t, uint64_t>`?

Maybe also define a type, e.g, `EdgeCountT ` for `std::pair<std::pair<uint64_t, uint64_t>, uint64_t>`  ?


================
Comment at: llvm/lib/Transforms/Utils/CodeLayout.cpp:133
+    else
+      return jumpExtTSPScore(0, 1, Count, FallthroughWeightUncond);
   }
----------------
nit: `return jumpExtTSPScore(0, 1, Count, IsConditional ? FallthroughWeightCond : FallthroughWeightUncond);`


================
Comment at: llvm/lib/Transforms/Utils/CodeLayout.cpp:410
   for (auto EdgeIt : Other->Edges) {
-    const auto DstChain = EdgeIt.first;
-    const auto DstEdge = EdgeIt.second;
-    const auto TargetChain = DstChain == Other ? this : DstChain;
-    auto CurEdge = getEdge(TargetChain);
+    Chain *const DstChain = EdgeIt.first;
+    ChainEdge *const DstEdge = EdgeIt.second;
----------------
Why do you need 'const` here?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129893