[PATCH] D53990: [MC] Support labels as offsets in .reloc directive

Simon Atanasyan via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Nov 8 12:39:12 PST 2018


atanasyan added a reviewer: echristo.
atanasyan added a comment.

LGTM, nit below.

Please delay the commit a bit to get a chance other reviewers to take a look at the patch.



================
Comment at: include/llvm/MC/MCObjectStreamer.h:43
+  class PendingMCFixup {
+  public:
+    const MCSymbol *Sym;
----------------
The `class` can be replaced by `struct`.


================
Comment at: include/llvm/MC/MCObjectStreamer.h:47
+    MCDataFragment *DF;
+    PendingMCFixup(const MCSymbol *MCSym, MCDataFragment *F, MCFixup McFixup) {
+      Sym = MCSym;
----------------
  - Why don't use c++ style class fields initialization?
  - `MCSym` but `McFixup`. I think it's better to name either `MCSym` / `MCFixup` or `McSym` / `McFixup`.


================
Comment at: lib/MC/MCObjectStreamer.cpp:658
+  PendingFixups.push_back(PendingMCFixup(&SRE.getSymbol(), DF,
+                                         MCFixup::create(-1, Expr, Kind, Loc)));
   return false;
----------------
That can be written a bit more briefly:
```
PendingFixups.emplace_back(&SRE.getSymbol(), DF,
                           MCFixup::create(-1, Expr, Kind, Loc));
```


Repository:
  rL LLVM

https://reviews.llvm.org/D53990





More information about the llvm-commits mailing list