[PATCH] D154020: [BOLT] Introduce Rewriter interface

[Job Noorman via Phabricator via llvm-commits]

jobnoorman added inline comments.


================
Comment at: bolt/include/bolt/Rewrite/RewriterBase.h:23
+/// Base class for updating sections containing metadata.
+class RewriterBase {
+  StringRef Name;
----------------
maksfb wrote:
> jobnoorman wrote:
> > Is this only supposed to be used for rewriting metadata? If so, maybe this needs a more specific name?
> > 
> > For example, at some point I was considering to rewrite instructions pre-CFG to support RISC-V linker relaxation. Would this interface have been suitable for that? (Let's ignore for now whether that's actually a good idea to do :))
> How about `MetadataRewriterBase`? I'm thinking the instances/implementations should be generic enough, but not modify the instructions directly. I.e. they can annotate instructions and introduce pseudo instructions to the stream, but otherwise keep the disassembly intact.
> 
> The pre-CFG modification you are talking about is better suited for a `BinaryPass` that modifies `BinaryFunction` code. We just have to teach the `BinaryPassManager` to operate on pre-CFG state.
> How about `MetadataRewriterBase`?

I'd say that's better. It would make me think twice about using it to rewrite instructions :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154020