[PATCH] D58073: [GlobalISel][NFC]: Add interface to reserve memory in GISelWorklist

[Amara Emerson via Phabricator via llvm-commits]

aemerson added inline comments.


================
Comment at: lib/CodeGen/GlobalISel/Combiner.cpp:115
+    unsigned MF_Size = 0;
+    for (auto &MBB : MF)
+      MF_Size += MBB.size();
----------------
aditya_nandakumar wrote:
> arsenm wrote:
> > aditya_nandakumar wrote:
> > > aditya_nandakumar wrote:
> > > > paquette wrote:
> > > > > arsenm wrote:
> > > > > > It seems weird to me to scan over the function just to estimate this. Can you start with some large constant and increase with each block as it's seen?
> > > > > `MF.getInstructionCount()` does this
> > > > What according to you is a large constant? Also could you elaborate on what  you mean by increase with each block as it's seen? IIUC are you suggesting NumBlocks * some_large_constant?
> > > Didn't know it existed. Will change to it.
> > Well both the number of blocks and count of instructions in the block are scans of the entire linked lists IIRC. You could update the estimate at the end of the loop when you can know how many instructions are around. I don't know if it matters. If I were to randomly pick an initial number I would guess thousands?
> I just realized that in the combiner it's silly that we're recomputing this for every loop - I'm not sure if your original comment was addressing that or in general about traversing linked lists even once (I assumed it was the latter). I also don't think it matters if we care about the exact size during each iteration - if we're doing the full list traversal, I'll definitely move this out of the main loop and use an old estimate.
> Alternatively I can use something like 4096 * getNumBlocks() if that works better.. I can run some numbers to see how this performs.
I think that ilist's size() is a linear time linked list walk.


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D58073/new/

https://reviews.llvm.org/D58073