[PATCH] D154020: [BOLT] Introduce Rewriter interface
Job Noorman via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Thu Jun 29 00:13:56 PDT 2023
jobnoorman added inline comments.
================
Comment at: bolt/include/bolt/Rewrite/RewriteManager.h:48
+
+ /// Use this static method to access global RewriteManager instance.
+ static RewriteManager *getManager();
----------------
Amir wrote:
> Why a global instance and not a RewriteInstance member variable?
I would also recommend to not make this a singleton. It is (in theory) possible to have multiple `RewriteInstance` objects but having a global `RewriteManager` would make this (more) difficult.
================
Comment at: bolt/include/bolt/Rewrite/RewriterBase.h:23
+/// Base class for updating sections containing metadata.
+class RewriterBase {
+ StringRef Name;
----------------
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 :))
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D154020/new/
https://reviews.llvm.org/D154020
More information about the llvm-commits
mailing list