[PATCH] D105866: [lld-macho] Use intermediate arrays to store opcodes

[Vincent Lee via Phabricator via llvm-commits]

thevinster marked an inline comment as done.
thevinster added inline comments.


================
Comment at: lld/MachO/SyntheticSections.cpp:281-282
+struct BindIR {
+  uint8_t opcode = 0xF0;
+  uint64_t data = 0;
+};
----------------
int3 wrote:
> do we need these default values?
Probably not, since we always construct these values explicitly. But, the reason for having it as default is 1/ follow the convention of `Binding` struct and 2/ in the event that something goes awry, `0xF0` is not valid opcode and should make the program scream instead of accidentally writing "valid" values. 

Plus, I think it'll debugging easier if we see these specific values when debugging.


================
Comment at: lld/MachO/SyntheticSections.cpp:298-299
   if (lastBinding.segment != seg) {
-    os << static_cast<uint8_t>(BIND_OPCODE_SET_SEGMENT_AND_OFFSET_ULEB |
-                               seg->index);
-    encodeULEB128(offset, os);
+    opcodes.push_back(
+        {BIND_OPCODE_SET_SEGMENT_AND_OFFSET_ULEB | seg->index, offset});
     lastBinding.segment = seg;
----------------
int3 wrote:
> ah I guess we need the cast
Fwiw, the tests did pass. I'll revert to the old implementation. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105866