[PATCH] D31058: Store Arguments in a flat array instead of an iplist

Chandler Carruth via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Mar 16 15:29:54 PDT 2017


chandlerc added inline comments.


================
Comment at: include/llvm/IR/Function.h:51
 public:
-  typedef SymbolTableList<Argument> ArgumentListType;
+  typedef OwningArrayRef<Argument> ArgumentListType;
   typedef SymbolTableList<BasicBlock> BasicBlockListType;
----------------
You're not really using this in the way I would expect -- instead we use a raw pointer for the storage. We just return a const reference to this. That doesn't make a lot of sense.

I think you want to change the getArgumentList methods to operate in terms of ArrayRef, and either use this for the storage itself or not have it at all.


================
Comment at: include/llvm/IR/Function.h:107-110
+  /// Internal wrapper for conveniently iterating the argument list.
+  MutableArrayRef<Argument> getArgList() {
+    return MutableArrayRef<Argument>(Arguments, NumArgs);
+  }
----------------
We already have getArgumentList?


================
Comment at: lib/IR/Function.cpp:230-231
+  if (NumArgs > 0) {
+    Arguments =
+        reinterpret_cast<Argument *>(malloc(sizeof(Argument) * NumArgs));
+    for (unsigned i = 0, e = NumArgs; i != e; ++i) {
----------------
Why malloc?


https://reviews.llvm.org/D31058





More information about the llvm-commits mailing list