[PATCH] D113424: ext-tsp basic block layout

[Hongtao Yu via Phabricator via llvm-commits]

hoy accepted this revision.
hoy added a comment.

lgtm except for some nits. Thanks.



================
Comment at: llvm/lib/CodeGen/MachineBlockPlacement.cpp:3571
+  MachineBasicBlock *HeadBB = &F->front();
+  BlockChain *FunctionChain =
+      new (ChainAllocator.Allocate()) BlockChain(BlockToChain, HeadBB);
----------------
spupyrev wrote:
> hoy wrote:
> > The object is created but not used anywhere. Is that expected?
> What object do you refer to?
nvm, the object pointed by `FunctionChain` is used in `FunctionChain->merge`. 


================
Comment at: llvm/lib/Transforms/Utils/CodeLayout.cpp:60
+    "ext-tsp-forward-distance", cl::Hidden, cl::init(1024),
+    cl::desc("The maximum distance (in bytes) of a forward jump for ExtTSP"));
+
----------------
nit: the distance in use isn't really in bytes? It is number of MR instructions?


================
Comment at: llvm/lib/Transforms/Utils/CodeLayout.cpp:273
+  // Unique chain identifier.
+  size_t Id;
+  // Cached ext-tsp score for the chain.
----------------
nit: we usually use `uint64_t` for id type explicitly.


================
Comment at: llvm/lib/Transforms/Utils/CodeLayout.cpp:897
+
+#ifndef NDEBUG
+  // Verify correctness of the input data.
----------------
nit: no need to use #ifndef NDEBUG


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113424