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

[Chandler Carruth via Phabricator via llvm-commits]

chandlerc accepted this revision.
chandlerc added a comment.
This revision is now accepted and ready to land.

LGTM with whatever you decide about the allocator stuff.



================
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) {
----------------
rnk wrote:
> rnk wrote:
> > chandlerc wrote:
> > > Why malloc?
> > Is there a way to use 'new' to allocate uninitialized memory suitable for this kind of placement new construction? I can do `new char[sizeof(Argument)*NumArgs]`, but it didn't seem prettier. Regular array new wants a default ctor, which Argument doesn't have, and adding one would be wasteful. I just want to allocate some memory and initialize some complex, non-value type objects into it. How hard can that be? ¯\_(ツ)_/¯
> Maybe you're suggesting `::operator new(...)`? That'd work.
Is `std::allocator<Argument>().allocate(NumArgs)` too verbose/arcane? It has the semantics I think we want here. (And `.deallocate(...)' below.)

Anyways, I don't feel particularly strongly about this.


================
Comment at: lib/IR/Function.cpp:245-247
+static MutableArrayRef<Argument> makeArgArray(Argument *Args, size_t Count) {
+  return MutableArrayRef<Argument>(Args, Count);
+}
----------------
We should either have `makeArrayRef` produce a MutableArrayRef for non-const pointers or we should add a `makeMutableArrayRef` that accepts non-const pointers. Then I don't think you'll need this.

That can bea  follow-up cleanup.


https://reviews.llvm.org/D31058