[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