[PATCH] D145791: [MC] Always encode instruction into SmallVector

Fri Mar 10 11:30:52 PST 2023

MaskRay accepted this revision.
MaskRay added a comment.
This revision is now accepted and ready to land.

LGTM, but make sure @RKSimon is happy as well.

In D145791#4185462 <https://reviews.llvm.org/D145791#4185462>, @aengelke wrote:

> I don't think there are "obviously missed" problems; IMHO the main problem is the abstraction itself, which for every write involves a function call (`raw_ostream::write`, unlikely to be inlined due to size and unlikely to be optimizable as the buffer pointers/mode are mutable members) and an indirect function call (`write_impl`, extremely unlikely to be devirtualized due to general code complexity). The impact of the latter could //probably// be reduced by buffering in `raw_svector_ostream`, but that'd be a substantial and massively breaking change with unknown (and hardly quantifiable) benefits. If you have any suggestions for more generally applicable approaches, I'd be happy to try them; but right now getting rid of the (apparently completely unneeded) abstraction appears to be the easiest, least intrusive, and most performant change.

The change makes sense to me. It removes an unneeded abstraction and simplifies many call sites.
`raw_ostream::write` =(inlinable)=> `flush_tied_then_write` (unneeded `TiedStream` check) =(virtual function call)=> `raw_svector_ostream::write_impl` ==> SmallVector `append(ItTy in_start, ItTy in_end)` (range; less efficient then `push_back`).

Every step adds some overhead.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145791