[PATCH] D116610: [ADCE][NFC] Batch DT updates together

Jakub Kuderski via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Jan 4 19:36:38 PST 2022


kuhar added inline comments.


================
Comment at: llvm/lib/Transforms/Scalar/ADCE.cpp:582
   bool Changed = false;
+  SmallVector<DominatorTree::UpdateType, 16> DeletedEdges;
 
----------------
qcolombet wrote:
> qcolombet wrote:
> > kuhar wrote:
> > > qcolombet wrote:
> > > > kuhar wrote:
> > > > > How did you pick this constant? 
> > > > Randomly to be honest.
> > > > 
> > > > It was 4 for the incremental updates and I figured we need to increase this size while batching more updates together while still hitting the fast path of the container.
> > > Could you either generate some statistics based on some real-world code bases and the pathological test cases mentioned in the patch description, or leave the constant out and let it SmallVector pick the small size automatically?
> > > 
> > > If you need test bitcode, I have some old whole-program bitcode laying around here: https://drive.google.com/drive/folders/1VJpym19cW-8BVgdtl2MsD3zB4CoEQ93O. You can just feed it to opt.
> > > Could you either generate some statistics based on some real-world code bases
> > 
> > I'll do that thanks for the pointer.
> > 
> > > ... the pathological test cases
> > 
> > The pathological test cases are less interesting because they involve thousands of edges and that's not a small vector at this point :).
> Here are some statistics based on the bitcode files you've shared. We delete 733 edges total on the whole suite.
> Ignoring the cases where we don't remove anything, the number of edges to delete:
> - Average at 1.5
> - Median is 1
> - The max is 36 (happens once) and the next max is 10 (happens 5 times)
> 
> All in all, if we leave the size at 4, with that suite, we hit the fast storage in ~97.3% of the cases.
> If we bump it to 16 (like I did here), we hit the fast storage in ~99.9% of the cases.
Thanks! I'm surprised the max is so small. <= 10 or the default size sound good to me then.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116610



More information about the llvm-commits mailing list