[PATCH] D123751: Implement support for __llvm_addrsig for MachO in llvm-mc

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added a comment.

Placing code gen test/MC



================
Comment at: llvm/include/llvm/MC/MCObjectWriter.h:37
+  bool EmitAddrsigSection = false;
+  std::vector<const MCSymbol *> AddrsigSyms;
+
----------------
Moving the small boolean variable after the vector. If someone adds more smaller variables to the class in the future, it will make the class size smaller.


================
Comment at: llvm/lib/MC/MachObjectWriter.cpp:767
+  MCFragment *pFragment = &*fragmentList.begin();
+  assert(pFragment);
+  MCDataFragment *pDataFragment = dyn_cast_or_null<MCDataFragment>(pFragment);
----------------
`*fragmentList.begin()` the dereference has implied that it can't be nullptr, so assert can be removed.


================
Comment at: llvm/lib/MC/MachObjectWriter.cpp:773
+  for (const MCSymbol *sym : this->getAddrsigSyms()) {
+    encodeULEB128(sym->getIndex(), OS);
+  }
----------------
drop brace


================
Comment at: llvm/test/MC/MachO/addrsig-macho.ll:55
+
+attributes #0 = { noinline }
----------------
CodeGen/X86/addrsig.ll has some interesting IR objects which may be worth adding here. It seems to me that CodeGen may be a better directory.


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

https://reviews.llvm.org/D123751