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

Amara Emerson via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Feb 11 16:38:59 PST 2019

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.




More information about the llvm-commits mailing list