[PATCH] D125998: [Metadata] Add a resize capability to MDNodes and add a push_back interface to MDNodes.
Duncan P. N. Exon Smith via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Thu May 26 07:47:43 PDT 2022
dexonsmith accepted this revision.
dexonsmith added a comment.
This revision is now accepted and ready to land.
LGTM, thanks! I left a couple of nits inline for you to consider.
================
Comment at: llvm/include/llvm/IR/Metadata.h:955-956
struct Header {
- unsigned NumOperands;
+ bool Resizable : 1;
+ bool Large : 1;
+ size_t SmallSize : 4;
----------------
Nit: I suggest naming these `Is{Resizable,Large}`.
================
Comment at: llvm/include/llvm/IR/Metadata.h:992-997
+ bool isResizable() const { return Resizable; }
+ bool isLarge() const { return Large; }
+ size_t getSmallSize() const { return SmallSize; }
+ void setSmallSize(size_t NumOps) { SmallSize = NumOps; }
+ size_t getSmallNumOps() const { return SmallNumOps; }
+ void setSmallNumOps(size_t NumOps) { SmallNumOps = NumOps; }
----------------
Nit: I'm not sure these accessors/mutators carry their weight, given that fields are already public in `Header` and `Header` is an implementation detail of `MDOperand` (no other users). I'd just as soon skip them and access the fields directly.
(If you have a short-term plan for a follow up that makes one of them non-trivial, maybe it'd be worth leaving that one behind, but otherwise...)
================
Comment at: llvm/include/llvm/IR/Metadata.h:998
+ void setSmallNumOps(size_t NumOps) { SmallNumOps = NumOps; }
+ size_t getNumOperands() const { return operands().size(); }
+
----------------
(Might skip this one too...)
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D125998/new/
https://reviews.llvm.org/D125998
More information about the llvm-commits
mailing list