[PATCH] D68062: Propeller lld framework for basicblock sections

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added inline comments.


================
Comment at: lld/ELF/PropellerELFCfg.cpp:1
+//===-------------------- PropellerELFCfg.cpp -----------------------------===//
+//
----------------
There are still open questions about the linker rewriting approach: https://lists.llvm.org/pipermail/llvm-dev/2019-October/135616.html

Let's see what conclusions people will reach.


================
Comment at: lld/ELF/PropellerELFCfg.cpp:19
+#include "llvm/Object/ObjectFile.h"
+// Needed by ELFSectionRef & ELFSymbolRef.
+#include "llvm/Object/ELFObjectFile.h"
----------------
Delete the comment.

I suspect a comment on its own line may interact badly with clang-format.


================
Comment at: lld/ELF/PropellerELFCfg.cpp:44
+bool ControlFlowGraph::writeAsDotGraph(const char *cfgOutName) {
+  FILE *fp = fopen(cfgOutName, "w");
+  if (!fp) {
----------------
Use StringRef. Avoid const char *

```
  std::error_code ec;
  raw_fd_ostream os(..., ec, sys::fs::OF_None);
```


================
Comment at: lld/ELF/PropellerELFCfg.cpp:223
+      StringRef symName = *s;
+      /*
+      lld::elf::Symbol *PSym =
----------------
Delete unused code.


================
Comment at: lld/ELF/PropellerELFCfg.h:95
+
+  const static uint64_t InvalidAddress = -1l;
+
----------------
delete the suffix l.


================
Comment at: lld/ELF/PropellerELFCfg.h:101
+      return BName.size();
+    else
+      return 0;
----------------
delete else


================
Comment at: lld/ELF/PropellerELFCfg.h:185
+  // See implementaion comments in .cpp.
+  void buildCFG(ControlFlowGraph &cfg, const SymbolRef &cfgSym,
+                std::map<uint64_t, std::unique_ptr<CFGNode>> &nodeMap);
----------------
If the ordered property is not required, map -> unordered_map


================
Comment at: lld/ELF/PropellerELFCfg.h:228
+std::ostream &operator<<(std::ostream &out, const ControlFlowGraph &cfg);
+}
+} // namespace lld
----------------
not clang-format'ed, i.e. i don't see

```
} // namespace propeller
```


Repository:
  rLLD LLVM Linker

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

https://reviews.llvm.org/D68062