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

[Jez Ng via Phabricator via llvm-commits]

int3 added inline comments.


================
Comment at: lld/MachO/SyntheticSections.cpp:280
 };
+struct BindOpcodeOpt {
+  uint8_t opcode = 0xF0;
----------------
how about `BindIR`?


================
Comment at: lld/MachO/SyntheticSections.cpp:282
+  uint8_t opcode = 0xF0;
+  uint64_t delta = 0; // Placeholder for offset or addend
+};
----------------
how about renaming it to `data` so that there's less confusion about why we're using `delta` to refer to an offset or an addend?

(alternatively we could use a union here)


================
Comment at: lld/MachO/SyntheticSections.cpp:299-300
+    BindOpcodeOpt op = {
+        static_cast<uint8_t>(BIND_OPCODE_SET_SEGMENT_AND_OFFSET_ULEB |
+                             seg->index), // opcode
+        offset                            // delta
----------------
cast shouldn't be necessary any more I think. I used it for writing to the stream because we would otherwise call the integer overload of the stream operator. But here there's no overload to worry about


================
Comment at: lld/MachO/SyntheticSections.cpp:303
+    };
+    opcodes.push_back(op);
     lastBinding.segment = seg;
----------------



================
Comment at: lld/MachO/SyntheticSections.cpp:334
+static void flushOpcodes(BindOpcodeOpt &op, raw_svector_ostream &os) {
+  uint8_t opcode = op.opcode & 0xF0;
+  switch (opcode) {
----------------
I don't think there's anything to mask here, since we are explicitly setting `op.opcode` to one of the opcode values minus any extra data. But we could do an assert to be on the safe side :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105866