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

[Vincent Lee via Phabricator via llvm-commits]

thevinster added inline comments.


================
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:
> thevinster wrote:
> > int3 wrote:
> > > ah I guess we need the cast
> > Fwiw, the tests did pass. I'll revert to the old implementation. 
> I get why we added the cast back, but the temporary seems unnecessary... we can construct it inline as part of the `push_back` statement.
> 
> Also, the cast is only needed for `BIND_OPCODE_SET_SEGMENT_AND_OFFSET_ULEB | seg->index` as `|` returns an `int` (regardless of the size of its operands). We don't need the cast for the other opcodes below
> 
> (I thought you meant 'revert' as in 'partial revert', didn't realize you were going to do a full one)
My bad. I completely misunderstood. I'll make a forward fix of this on D105867 and make sure things are still running smoothly. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105866